Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround crash in atrac3+ decoding #66

Closed
wants to merge 3 commits into from
Closed

Workaround crash in atrac3+ decoding #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e6f9c78
Update the ARM64 android build script
hrydgard Sep 8, 2022
0e9e153
Give the decoder more memory to read from to workaround bad bit readi…
hrydgard Sep 8, 2022
5f93a31
Update ARM64 binaries
hrydgard Sep 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion android/arm64/include/libavutil/ffversion.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* Automatically generated by version.sh, do not manually edit! */
#ifndef AVUTIL_FFVERSION_H
#define AVUTIL_FFVERSION_H
#define FFMPEG_VERSION "43076c7"
#define FFMPEG_VERSION "3.0.2"
#endif /* AVUTIL_FFVERSION_H */
Binary file modified android/arm64/lib/libavcodec.a
View file
Open in desktop
Binary file not shown.
Binary file modified android/arm64/lib/libavformat.a
View file
Open in desktop
Binary file not shown.
Binary file modified android/arm64/lib/libavutil.a
View file
Open in desktop
Binary file not shown.
Binary file modified android/arm64/lib/libswresample.a
View file
Open in desktop
Binary file not shown.
Binary file modified android/arm64/lib/libswscale.a
View file
Open in desktop
Binary file not shown.
9 changes: 7 additions & 2 deletions android_arm64-v8a.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#!/bin/bash

# NOTE: If you want to do a debug build, use the --disable-optimization flag below, not -O0 or similar in cflags.

BUILD_ANDROID_PLATFORM="android-21"

# Change NDK to your Android NDK location if needed
Expand All @@ -24,6 +26,10 @@ GENERAL="\
--cross-prefix=$NDK_PREBUILT/bin/aarch64-linux-android- \
--ld=$NDK_PREBUILTLLVM/bin/clang \
--nm=$NDK_PREBUILT/bin/aarch64-linux-android-nm"
# --disable-optimizations"

EXTRA_CC="\
-DCONFIG_SAFE_BITSTREAM_READER=1"

MODULES="\
--disable-avdevice \
Expand Down Expand Up @@ -87,13 +93,12 @@ PARSERS="\

function build_arm64
{
# no-missing-prototypes because of a compile error seemingly unique to aarch64.
./configure --logfile=conflog.txt --target-os=linux \
--prefix=./android/arm64 \
--arch=aarch64 \
${GENERAL} \
--sysroot=$NDK/sysroot \
--extra-cflags=" --target=aarch64-none-linux-android21 -no-canonical-prefixes -fdata-sections -ffunction-sections -fno-limit-debug-info -funwind-tables -fPIC -O2 -DANDROID -DANDROID_PLATFORM=$BUILD_ANDROID_PLATFORM -Dipv6mr_interface=ipv6mr_ifindex -fasm -fno-short-enums -fno-strict-aliasing -Wno-missing-prototypes" \
--extra-cflags="$EXTRA_CC --target=aarch64-none-linux-android21 -no-canonical-prefixes -fdata-sections -ffunction-sections -fno-limit-debug-info -funwind-tables -g -O2 -DANDROID -DANDROID_PLATFORM=$BUILD_ANDROID_PLATFORM -Dipv6mr_interface=ipv6mr_ifindex -fasm -fno-short-enums -fno-strict-aliasing" \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional to leave -g here? Also, last time I tried the latest NDK, there were issues related to PIC. This stuff:

ld: error: relocation R_AARCH64_ADR_PREL_PG_HI21 cannot be used against symbol ff_h264_idct_dc_add_neon; recompile with -fPIC
ld: error: can't create dynamic relocation R_AARCH64_ADD_ABS_LO12_NC against symbol: ff_h264_idct_dc_add_neon in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output

Not sure if this is just old binaries or something, though. I wonder if we need -pic or something else?

-[Unknown]

Copy link
Owner Author

@hrydgard hrydgard Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pic is enabled with --enable-pic in the GENERAL settings above. -g might not be neeed, I just wanted as much stack trace info as I can get... And it did compile and work with no issues.

--disable-shared \
--enable-static \
--extra-ldflags=" -B$NDK_PREBUILT/bin/aarch64-linux-android- --target=aarch64-linux-android -Wl,--rpath-link,$NDK_PLATFORM/usr/lib -L$NDK_PLATFORM/usr/lib -L$NDK_PREBUILT/aarch64-linux-android/lib64 -nostdlib -lc -lm -ldl -llog" \
Expand Down
21 changes: 19 additions & 2 deletions libavcodec/atrac3plusdec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,24 +339,38 @@ static int atrac3p_decode_frame(AVCodecContext *avctx, void *data,
if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
return ret;

if ((ret = init_get_bits8(&ctx->gb, avpkt->data, avpkt->size)) < 0)
// PPSSPP workaround: With bad/corrupt input, the atrac3plus decoder does not
// reliably stay inside the bounds of the buffer. Instead of carefully checking everything
// inside it, for now let's just give it more space to read from.
const int extra_bytes = 1024;

uint8_t *bigger_buffer = malloc(avpkt->size + extra_bytes);
memset(bigger_buffer + avpkt->size, 0, extra_bytes);
memcpy(bigger_buffer, avpkt->data, avpkt->size);
Comment on lines -342 to +349
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this directly the packet buffer we pass in via avcodec_decode_audio4? If so, we can control the buffer size there. Usually it points at PSP RAM, though... but it can also point to dataBuf_. Does padding that out help?

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was so happy to have found the workaround that I forgot to track outwards to see if we control the buffer. Will have a look later today.


if ((ret = init_get_bits8(&ctx->gb, bigger_buffer, avpkt->size)) < 0) {
free(bigger_buffer);
return ret;
}

if (get_bits1(&ctx->gb)) {
av_log(avctx, AV_LOG_ERROR, "Invalid start bit!\n");
free(bigger_buffer);
return AVERROR_INVALIDDATA;
}

while (get_bits_left(&ctx->gb) >= 2 &&
(ch_unit_id = get_bits(&ctx->gb, 2)) != CH_UNIT_TERMINATOR) {
if (ch_unit_id == CH_UNIT_EXTENSION) {
avpriv_report_missing_feature(avctx, "Channel unit extension");
free(bigger_buffer);
return AVERROR_PATCHWELCOME;
}
if (ch_block >= ctx->num_channel_blocks ||
ctx->channel_blocks[ch_block] != ch_unit_id) {
av_log(avctx, AV_LOG_ERROR,
"Frame data doesn't match channel configuration!\n");
free(bigger_buffer);
return AVERROR_INVALIDDATA;
}

Expand All @@ -366,8 +380,10 @@ static int atrac3p_decode_frame(AVCodecContext *avctx, void *data,
if ((ret = ff_atrac3p_decode_channel_unit(&ctx->gb,
&ctx->ch_units[ch_block],
channels_to_process,
avctx)) < 0)
avctx)) < 0) {
free(bigger_buffer);
return ret;
}

decode_residual_spectrum(&ctx->ch_units[ch_block], ctx->samples,
channels_to_process, avctx);
Expand All @@ -384,6 +400,7 @@ static int atrac3p_decode_frame(AVCodecContext *avctx, void *data,

*got_frame_ptr = 1;

free(bigger_buffer);
return FFMIN(avctx->block_align, avpkt->size);
}

Expand Down