From adee4626d3373c5481730c23cf15d06f16f8e39f Mon Sep 17 00:00:00 2001 From: Alexey Rochev Date: Mon, 30 Oct 2023 19:21:49 +0300 Subject: [PATCH 1/6] ffmpeg: reallocate output buffer dynamically With FFmpeg we can't determine size of output buffer ahead of time for all codecs, so we need to reallocate it when needed instead of simply failing. --- .../decoder/SimpleDecoderOutputBuffer.java | 20 +++++++ libraries/decoder_ffmpeg/proguard-rules.txt | 5 ++ .../decoder/ffmpeg/FfmpegAudioDecoder.java | 28 ++++++---- .../decoder_ffmpeg/src/main/jni/ffmpeg_jni.cc | 56 ++++++++++++++++--- 4 files changed, 90 insertions(+), 19 deletions(-) diff --git a/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java b/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java index c9adf0b7d6..eb01d97c49 100644 --- a/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java +++ b/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java @@ -16,6 +16,7 @@ package androidx.media3.decoder; import androidx.annotation.Nullable; +import androidx.media3.common.util.Assertions; import androidx.media3.common.util.UnstableApi; import java.nio.ByteBuffer; import java.nio.ByteOrder; @@ -49,6 +50,25 @@ public class SimpleDecoderOutputBuffer extends DecoderOutputBuffer { return data; } + /** + * Reallocates the buffer with new size + * Existing data between beginning of the buffer and {@link ByteBuffer#limit} is copied to the new buffer, + * and {@link ByteBuffer#position} is preserved. {@link ByteBuffer#limit} is set to the new size. + * @param newSize New size of buffer. + * @return The {@link #data} buffer, for convenience. + */ + public ByteBuffer grow(int newSize) { + Assertions.checkNotNull(data); + final ByteBuffer newData = ByteBuffer.allocateDirect(newSize).order(ByteOrder.nativeOrder()); + final int restorePosition = data.position(); + data.position(0); + newData.put(data); + newData.position(restorePosition); + newData.limit(newSize); + data = newData; + return newData; + } + @Override public void clear() { super.clear(); diff --git a/libraries/decoder_ffmpeg/proguard-rules.txt b/libraries/decoder_ffmpeg/proguard-rules.txt index 7f62d2b7e7..3708f7dd54 100644 --- a/libraries/decoder_ffmpeg/proguard-rules.txt +++ b/libraries/decoder_ffmpeg/proguard-rules.txt @@ -4,3 +4,8 @@ -keepclasseswithmembernames class * { native ; } + +# This method is called from native code +-keep class androidx.media3.decoder.ffmpeg.FfmpegAudioDecoder { + private java.nio.ByteBuffer growOutputBuffer(androidx.media3.decoder.SimpleDecoderOutputBuffer, int); +} \ No newline at end of file diff --git a/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java b/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java index 6fc34e7191..37a2c6b448 100644 --- a/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java +++ b/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java @@ -32,9 +32,8 @@ import java.util.List; /* package */ final class FfmpegAudioDecoder extends SimpleDecoder { - // Output buffer sizes when decoding PCM mu-law streams, which is the maximum FFmpeg outputs. - private static final int OUTPUT_BUFFER_SIZE_16BIT = 65536; - private static final int OUTPUT_BUFFER_SIZE_32BIT = OUTPUT_BUFFER_SIZE_16BIT * 2; + private static final int INITIAL_OUTPUT_BUFFER_SIZE_16BIT = 65535; + private static final int INITIAL_OUTPUT_BUFFER_SIZE_32BIT = INITIAL_OUTPUT_BUFFER_SIZE_16BIT * 2; private static final int AUDIO_DECODER_ERROR_INVALID_DATA = -1; private static final int AUDIO_DECODER_ERROR_OTHER = -2; @@ -42,7 +41,7 @@ import java.util.List; private final String codecName; @Nullable private final byte[] extraData; private final @C.PcmEncoding int encoding; - private final int outputBufferSize; + private int outputBufferSize; private long nativeContext; // May be reassigned on resetting the codec. private boolean hasOutputFormat; @@ -64,7 +63,7 @@ import java.util.List; codecName = Assertions.checkNotNull(FfmpegLibrary.getCodecName(format.sampleMimeType)); extraData = getExtraData(format.sampleMimeType, format.initializationData); encoding = outputFloat ? C.ENCODING_PCM_FLOAT : C.ENCODING_PCM_16BIT; - outputBufferSize = outputFloat ? OUTPUT_BUFFER_SIZE_32BIT : OUTPUT_BUFFER_SIZE_16BIT; + outputBufferSize = outputFloat ? INITIAL_OUTPUT_BUFFER_SIZE_32BIT : INITIAL_OUTPUT_BUFFER_SIZE_16BIT; nativeContext = ffmpegInitialize(codecName, extraData, outputFloat, format.sampleRate, format.channelCount); if (nativeContext == 0) { @@ -107,8 +106,9 @@ import java.util.List; } ByteBuffer inputData = Util.castNonNull(inputBuffer.data); int inputSize = inputData.limit(); - ByteBuffer outputData = outputBuffer.init(inputBuffer.timeUs, outputBufferSize); - int result = ffmpegDecode(nativeContext, inputData, inputSize, outputData, outputBufferSize); + outputBuffer.init(inputBuffer.timeUs, outputBufferSize); + + int result = ffmpegDecode(nativeContext, inputData, inputSize, outputBuffer, outputBuffer.data, outputBufferSize); if (result == AUDIO_DECODER_ERROR_OTHER) { return new FfmpegDecoderException("Error decoding (see logcat)."); } else if (result == AUDIO_DECODER_ERROR_INVALID_DATA) { @@ -135,11 +135,19 @@ import java.util.List; } hasOutputFormat = true; } - outputData.position(0); - outputData.limit(result); + outputBuffer.data.position(0); + outputBuffer.data.limit(result); return null; } + // Called from native code + /** @noinspection unused*/ + private ByteBuffer growOutputBuffer(SimpleDecoderOutputBuffer outputBuffer, int requiredSize) { + // Use it for new buffer so that hopefully we won't need to reallocate again + outputBufferSize = requiredSize; + return outputBuffer.grow(requiredSize); + } + @Override public void release() { super.release(); @@ -221,7 +229,7 @@ import java.util.List; int rawChannelCount); private native int ffmpegDecode( - long context, ByteBuffer inputData, int inputSize, ByteBuffer outputData, int outputSize); + long context, ByteBuffer inputData, int inputSize, SimpleDecoderOutputBuffer decoderOutputBuffer, ByteBuffer outputData, int outputSize); private native int ffmpegGetChannelCount(long context); diff --git a/libraries/decoder_ffmpeg/src/main/jni/ffmpeg_jni.cc b/libraries/decoder_ffmpeg/src/main/jni/ffmpeg_jni.cc index eeab3d3795..a91cde8ce6 100644 --- a/libraries/decoder_ffmpeg/src/main/jni/ffmpeg_jni.cc +++ b/libraries/decoder_ffmpeg/src/main/jni/ffmpeg_jni.cc @@ -35,6 +35,8 @@ extern "C" { #define LOG_TAG "ffmpeg_jni" #define LOGE(...) \ ((void)__android_log_print(ANDROID_LOG_ERROR, LOG_TAG, __VA_ARGS__)) +#define LOGD(...) \ + ((void)__android_log_print(ANDROID_LOG_DEBUG, LOG_TAG, __VA_ARGS__)) #define LIBRARY_FUNC(RETURN_TYPE, NAME, ...) \ extern "C" { \ @@ -67,6 +69,8 @@ static const AVSampleFormat OUTPUT_FORMAT_PCM_FLOAT = AV_SAMPLE_FMT_FLT; static const int AUDIO_DECODER_ERROR_INVALID_DATA = -1; static const int AUDIO_DECODER_ERROR_OTHER = -2; +static jmethodID growOutputBufferMethod; + /** * Returns the AVCodec with the specified name, or NULL if it is not available. */ @@ -81,13 +85,21 @@ AVCodecContext *createContext(JNIEnv *env, AVCodec *codec, jbyteArray extraData, jboolean outputFloat, jint rawSampleRate, jint rawChannelCount); +struct GrowOutputBufferCallback { + uint8_t *operator()(int requiredSize) const; + + JNIEnv *env; + jobject thiz; + jobject decoderOutputBuffer; +}; + /** * Decodes the packet into the output buffer, returning the number of bytes * written, or a negative AUDIO_DECODER_ERROR constant value in the case of an * error. */ int decodePacket(AVCodecContext *context, AVPacket *packet, - uint8_t *outputBuffer, int outputSize); + uint8_t *outputBuffer, int outputSize, GrowOutputBufferCallback growBuffer); /** * Transforms ffmpeg AVERROR into a negative AUDIO_DECODER_ERROR constant value. @@ -107,6 +119,17 @@ void releaseContext(AVCodecContext *context); jint JNI_OnLoad(JavaVM *vm, void *reserved) { JNIEnv *env; if (vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6) != JNI_OK) { + LOGE("JNI_OnLoad: GetEnv failed"); + return -1; + } + jclass clazz = env->FindClass("androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder"); + if (!clazz) { + LOGE("JNI_OnLoad: FindClass failed"); + return -1; + } + growOutputBufferMethod = env->GetMethodID(clazz, "growOutputBuffer","(Landroidx/media3/decoder/SimpleDecoderOutputBuffer;I)Ljava/nio/ByteBuffer;"); + if (!growOutputBufferMethod) { + LOGE("JNI_OnLoad: GetMethodID failed"); return -1; } avcodec_register_all(); @@ -138,12 +161,12 @@ AUDIO_DECODER_FUNC(jlong, ffmpegInitialize, jstring codecName, } AUDIO_DECODER_FUNC(jint, ffmpegDecode, jlong context, jobject inputData, - jint inputSize, jobject outputData, jint outputSize) { + jint inputSize, jobject decoderOutputBuffer, jobject outputData, jint outputSize) { if (!context) { LOGE("Context must be non-NULL."); return -1; } - if (!inputData || !outputData) { + if (!inputData || !decoderOutputBuffer || !outputData) { LOGE("Input and output buffers must be non-NULL."); return -1; } @@ -162,7 +185,17 @@ AUDIO_DECODER_FUNC(jint, ffmpegDecode, jlong context, jobject inputData, packet.data = inputBuffer; packet.size = inputSize; return decodePacket((AVCodecContext *)context, &packet, outputBuffer, - outputSize); + outputSize, GrowOutputBufferCallback{env, thiz, decoderOutputBuffer}); +} + +uint8_t *GrowOutputBufferCallback::operator()(int requiredSize) const { + jobject newOutputData = env->CallObjectMethod(thiz, growOutputBufferMethod, decoderOutputBuffer, requiredSize); + if (env->ExceptionCheck()) { + LOGE("growOutputBuffer() failed"); + env->ExceptionDescribe(); + return nullptr; + } + return static_cast(env->GetDirectBufferAddress(newOutputData)); } AUDIO_DECODER_FUNC(jint, ffmpegGetChannelCount, jlong context) { @@ -264,7 +297,7 @@ AVCodecContext *createContext(JNIEnv *env, AVCodec *codec, jbyteArray extraData, } int decodePacket(AVCodecContext *context, AVPacket *packet, - uint8_t *outputBuffer, int outputSize) { + uint8_t *outputBuffer, int outputSize, GrowOutputBufferCallback growBuffer) { int result = 0; // Queue input data. result = avcodec_send_packet(context, packet); @@ -320,15 +353,20 @@ int decodePacket(AVCodecContext *context, AVPacket *packet, } context->opaque = resampleContext; } - int inSampleSize = av_get_bytes_per_sample(sampleFormat); + int outSampleSize = av_get_bytes_per_sample(context->request_sample_fmt); int outSamples = swr_get_out_samples(resampleContext, sampleCount); int bufferOutSize = outSampleSize * channelCount * outSamples; if (outSize + bufferOutSize > outputSize) { - LOGE("Output buffer size (%d) too small for output data (%d).", + LOGD("Output buffer size (%d) too small for output data (%d), reallocating buffer.", outputSize, outSize + bufferOutSize); - av_frame_free(&frame); - return AUDIO_DECODER_ERROR_INVALID_DATA; + outputSize = outSize + bufferOutSize; + outputBuffer = growBuffer(outputSize); + if (!outputBuffer) { + LOGE("Failed to reallocate output buffer."); + av_frame_free(&frame); + return AUDIO_DECODER_ERROR_OTHER; + } } result = swr_convert(resampleContext, &outputBuffer, bufferOutSize, (const uint8_t **)frame->data, frame->nb_samples); From 8750ed8de6469dc818007f2eb254df9ddbd52cc5 Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 31 Oct 2023 12:24:24 +0000 Subject: [PATCH 2/6] Format with google-java-format Also amended Javadoc and added assertion to grow method --- .../decoder/SimpleDecoderOutputBuffer.java | 10 +++++--- .../decoder/ffmpeg/FfmpegAudioDecoder.java | 25 +++++++++++++------ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java b/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java index eb01d97c49..b3b1a1562f 100644 --- a/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java +++ b/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java @@ -51,14 +51,16 @@ public class SimpleDecoderOutputBuffer extends DecoderOutputBuffer { } /** - * Reallocates the buffer with new size - * Existing data between beginning of the buffer and {@link ByteBuffer#limit} is copied to the new buffer, - * and {@link ByteBuffer#position} is preserved. {@link ByteBuffer#limit} is set to the new size. - * @param newSize New size of buffer. + * Grows the buffer with to a new size. + * + *

Existing data is copied to the new buffer, and {@link ByteBuffer#position} is preserved. + * + * @param newSize New size of the buffer. * @return The {@link #data} buffer, for convenience. */ public ByteBuffer grow(int newSize) { Assertions.checkNotNull(data); + Assertions.checkArgument(newSize >= data.limit()); final ByteBuffer newData = ByteBuffer.allocateDirect(newSize).order(ByteOrder.nativeOrder()); final int restorePosition = data.position(); data.position(0); diff --git a/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java b/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java index 37a2c6b448..42e54b5a7f 100644 --- a/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java +++ b/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java @@ -63,7 +63,8 @@ import java.util.List; codecName = Assertions.checkNotNull(FfmpegLibrary.getCodecName(format.sampleMimeType)); extraData = getExtraData(format.sampleMimeType, format.initializationData); encoding = outputFloat ? C.ENCODING_PCM_FLOAT : C.ENCODING_PCM_16BIT; - outputBufferSize = outputFloat ? INITIAL_OUTPUT_BUFFER_SIZE_32BIT : INITIAL_OUTPUT_BUFFER_SIZE_16BIT; + outputBufferSize = + outputFloat ? INITIAL_OUTPUT_BUFFER_SIZE_32BIT : INITIAL_OUTPUT_BUFFER_SIZE_16BIT; nativeContext = ffmpegInitialize(codecName, extraData, outputFloat, format.sampleRate, format.channelCount); if (nativeContext == 0) { @@ -106,9 +107,10 @@ import java.util.List; } ByteBuffer inputData = Util.castNonNull(inputBuffer.data); int inputSize = inputData.limit(); - outputBuffer.init(inputBuffer.timeUs, outputBufferSize); - - int result = ffmpegDecode(nativeContext, inputData, inputSize, outputBuffer, outputBuffer.data, outputBufferSize); + ByteBuffer outputData = outputBuffer.init(inputBuffer.timeUs, outputBufferSize); + int result = + ffmpegDecode( + nativeContext, inputData, inputSize, outputBuffer, outputData, outputBufferSize); if (result == AUDIO_DECODER_ERROR_OTHER) { return new FfmpegDecoderException("Error decoding (see logcat)."); } else if (result == AUDIO_DECODER_ERROR_INVALID_DATA) { @@ -135,13 +137,15 @@ import java.util.List; } hasOutputFormat = true; } - outputBuffer.data.position(0); - outputBuffer.data.limit(result); + outputData.position(0); + outputData.limit(result); return null; } // Called from native code - /** @noinspection unused*/ + /** + * @noinspection unused + */ private ByteBuffer growOutputBuffer(SimpleDecoderOutputBuffer outputBuffer, int requiredSize) { // Use it for new buffer so that hopefully we won't need to reallocate again outputBufferSize = requiredSize; @@ -229,7 +233,12 @@ import java.util.List; int rawChannelCount); private native int ffmpegDecode( - long context, ByteBuffer inputData, int inputSize, SimpleDecoderOutputBuffer decoderOutputBuffer, ByteBuffer outputData, int outputSize); + long context, + ByteBuffer inputData, + int inputSize, + SimpleDecoderOutputBuffer decoderOutputBuffer, + ByteBuffer outputData, + int outputSize); private native int ffmpegGetChannelCount(long context); From 36fb20719f88bb91c32626daf2a70f2ab122fa4e Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 31 Oct 2023 12:29:23 +0000 Subject: [PATCH 3/6] Change warning suppression style --- .../androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java b/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java index 42e54b5a7f..d3b876403b 100644 --- a/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java +++ b/libraries/decoder_ffmpeg/src/main/java/androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder.java @@ -143,9 +143,7 @@ import java.util.List; } // Called from native code - /** - * @noinspection unused - */ + @SuppressWarnings("unused") private ByteBuffer growOutputBuffer(SimpleDecoderOutputBuffer outputBuffer, int requiredSize) { // Use it for new buffer so that hopefully we won't need to reallocate again outputBufferSize = requiredSize; From 78142d3f5e96c9cd8d72a022ac10bd13c7fd268e Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 31 Oct 2023 12:32:28 +0000 Subject: [PATCH 4/6] Fix typo --- .../java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java b/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java index b3b1a1562f..40f14d5213 100644 --- a/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java +++ b/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java @@ -51,7 +51,7 @@ public class SimpleDecoderOutputBuffer extends DecoderOutputBuffer { } /** - * Grows the buffer with to a new size. + * Grows the buffer to a new size. * *

Existing data is copied to the new buffer, and {@link ByteBuffer#position} is preserved. * From c4c4ef9670e6377d2b66afe8c468fef175efd58b Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 31 Oct 2023 12:39:02 +0000 Subject: [PATCH 5/6] Add terminating newline --- libraries/decoder_ffmpeg/proguard-rules.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/decoder_ffmpeg/proguard-rules.txt b/libraries/decoder_ffmpeg/proguard-rules.txt index 3708f7dd54..67c5e861a8 100644 --- a/libraries/decoder_ffmpeg/proguard-rules.txt +++ b/libraries/decoder_ffmpeg/proguard-rules.txt @@ -8,4 +8,4 @@ # This method is called from native code -keep class androidx.media3.decoder.ffmpeg.FfmpegAudioDecoder { private java.nio.ByteBuffer growOutputBuffer(androidx.media3.decoder.SimpleDecoderOutputBuffer, int); -} \ No newline at end of file +} From 58d8850b21641f794a7a99be753497a692695a8a Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 31 Oct 2023 12:56:32 +0000 Subject: [PATCH 6/6] Satisfy nullness checker correctness by using local variable --- .../media3/decoder/SimpleDecoderOutputBuffer.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java b/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java index 40f14d5213..a694250d8a 100644 --- a/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java +++ b/libraries/decoder/src/main/java/androidx/media3/decoder/SimpleDecoderOutputBuffer.java @@ -59,15 +59,15 @@ public class SimpleDecoderOutputBuffer extends DecoderOutputBuffer { * @return The {@link #data} buffer, for convenience. */ public ByteBuffer grow(int newSize) { - Assertions.checkNotNull(data); - Assertions.checkArgument(newSize >= data.limit()); - final ByteBuffer newData = ByteBuffer.allocateDirect(newSize).order(ByteOrder.nativeOrder()); - final int restorePosition = data.position(); - data.position(0); - newData.put(data); + ByteBuffer oldData = Assertions.checkNotNull(this.data); + Assertions.checkArgument(newSize >= oldData.limit()); + ByteBuffer newData = ByteBuffer.allocateDirect(newSize).order(ByteOrder.nativeOrder()); + int restorePosition = oldData.position(); + oldData.position(0); + newData.put(oldData); newData.position(restorePosition); newData.limit(newSize); - data = newData; + this.data = newData; return newData; }