Keep pending output format on MediaCodec.flush()

When operating the MediaCodec in asynchronous mode, we are seeing
NPEs raised from the DefaultAudioSink because output buffers are pushed
to the DefaultAudioSink with the DefaultAudioSink not configured yet.
This is possible to happen if an output buffer is passed with
MediaCodecAudioRenderer.processOutputBuffer() before an output format
change.

One possible code path to trigger this is when an output format change
is pending and we flush MediaCodec (e.g. for a seek): the callback is
waiting in the looper's queue but we ignore all currently queued
callbacks after a flush().

This commit checks for a pending output format change during a flush():
if one exists, and the next MediaCodec output callback right after the
flush() is an output buffer (and not a new output format), then the
pending output format is propagated first.

The only adapter that needs to change is the
AsynchronousMediaCodecAdapter which previsouly deleted all pending
callbacks immediately on flush(). The AsynchronousMediaCodecAdapter now
needs to also handle every enqueued callback in order to identify is
there's a pending output format change.

Testing: added unit tests for the new code. I have verified that if we
pass an output buffer to the audio renderer before the output format, it
will result on the specific NPE, but I cannot reproduce the base
scenario (an output format change is pending when flushing) on a real
device because we can't know if an output format is indeed pending.
PiperOrigin-RevId: 310885283
This commit is contained in:
christosts 2020-05-11 12:15:48 +01:00 committed by Oliver Woodman
parent 6aab5e2e61
commit 159e3a80d5
4 changed files with 170 additions and 14 deletions

View File

@ -31,6 +31,7 @@ import java.util.ArrayDeque;
private final ArrayDeque<MediaCodec.BufferInfo> bufferInfos; private final ArrayDeque<MediaCodec.BufferInfo> bufferInfos;
private final ArrayDeque<MediaFormat> formats; private final ArrayDeque<MediaFormat> formats;
@Nullable private MediaFormat currentFormat; @Nullable private MediaFormat currentFormat;
@Nullable private MediaFormat pendingOutputFormat;
@Nullable private IllegalStateException mediaCodecException; @Nullable private IllegalStateException mediaCodecException;
/** Creates a new MediaCodecAsyncCallback. */ /** Creates a new MediaCodecAsyncCallback. */
@ -111,6 +112,7 @@ import java.util.ArrayDeque;
* and any error that was previously set. * and any error that was previously set.
*/ */
public void flush() { public void flush() {
pendingOutputFormat = formats.isEmpty() ? null : formats.getLast();
availableInputBuffers.clear(); availableInputBuffers.clear();
availableOutputBuffers.clear(); availableOutputBuffers.clear();
bufferInfos.clear(); bufferInfos.clear();
@ -119,14 +121,18 @@ import java.util.ArrayDeque;
} }
@Override @Override
public void onInputBufferAvailable(MediaCodec mediaCodec, int i) { public void onInputBufferAvailable(MediaCodec mediaCodec, int index) {
availableInputBuffers.add(i); availableInputBuffers.add(index);
} }
@Override @Override
public void onOutputBufferAvailable( public void onOutputBufferAvailable(
MediaCodec mediaCodec, int i, MediaCodec.BufferInfo bufferInfo) { MediaCodec mediaCodec, int index, MediaCodec.BufferInfo bufferInfo) {
availableOutputBuffers.add(i); if (pendingOutputFormat != null) {
addOutputFormat(pendingOutputFormat);
pendingOutputFormat = null;
}
availableOutputBuffers.add(index);
bufferInfos.add(bufferInfo); bufferInfos.add(bufferInfo);
} }
@ -137,12 +143,17 @@ import java.util.ArrayDeque;
@Override @Override
public void onOutputFormatChanged(MediaCodec mediaCodec, MediaFormat mediaFormat) { public void onOutputFormatChanged(MediaCodec mediaCodec, MediaFormat mediaFormat) {
availableOutputBuffers.add(MediaCodec.INFO_OUTPUT_FORMAT_CHANGED); addOutputFormat(mediaFormat);
formats.add(mediaFormat); pendingOutputFormat = null;
} }
@VisibleForTesting() @VisibleForTesting()
void onMediaCodecError(IllegalStateException e) { void onMediaCodecError(IllegalStateException e) {
mediaCodecException = e; mediaCodecException = e;
} }
private void addOutputFormat(MediaFormat mediaFormat) {
availableOutputBuffers.add(MediaCodec.INFO_OUTPUT_FORMAT_CHANGED);
formats.add(mediaFormat);
}
} }

View File

@ -19,6 +19,7 @@ package com.google.android.exoplayer2.mediacodec;
import static com.google.android.exoplayer2.testutil.TestUtil.assertBufferInfosEqual; import static com.google.android.exoplayer2.testutil.TestUtil.assertBufferInfosEqual;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertThrows;
import static org.robolectric.Shadows.shadowOf;
import static org.robolectric.annotation.LooperMode.Mode.LEGACY; import static org.robolectric.annotation.LooperMode.Mode.LEGACY;
import android.media.MediaCodec; import android.media.MediaCodec;
@ -33,7 +34,6 @@ import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.robolectric.Shadows;
import org.robolectric.annotation.LooperMode; import org.robolectric.annotation.LooperMode;
/** Unit tests for {@link AsynchronousMediaCodecAdapter}. */ /** Unit tests for {@link AsynchronousMediaCodecAdapter}. */
@ -81,9 +81,12 @@ public class AsynchronousMediaCodecAdapterTest {
@Test @Test
public void dequeueInputBufferIndex_whileFlushing_returnsTryAgainLater() { public void dequeueInputBufferIndex_whileFlushing_returnsTryAgainLater() {
adapter.start(); adapter.start();
adapter.getMediaCodecCallback().onInputBufferAvailable(codec, /* index=*/ 0); adapter.getMediaCodecCallback().onInputBufferAvailable(codec, /* index=*/ 0);
// A callback that is pending.
new Handler(looper)
.post(() -> adapter.getMediaCodecCallback().onInputBufferAvailable(codec, /* index=*/ 1));
adapter.flush(); adapter.flush();
adapter.getMediaCodecCallback().onInputBufferAvailable(codec, /* index=*/ 1);
assertThat(adapter.dequeueInputBufferIndex()).isEqualTo(MediaCodec.INFO_TRY_AGAIN_LATER); assertThat(adapter.dequeueInputBufferIndex()).isEqualTo(MediaCodec.INFO_TRY_AGAIN_LATER);
} }
@ -99,7 +102,7 @@ public class AsynchronousMediaCodecAdapterTest {
() -> adapter.getMediaCodecCallback().onInputBufferAvailable(codec, /* index=*/ 1)); () -> adapter.getMediaCodecCallback().onInputBufferAvailable(codec, /* index=*/ 1));
// Wait until all tasks have been handled. // Wait until all tasks have been handled.
Shadows.shadowOf(looper).idle(); shadowOf(looper).idle();
assertThat(adapter.dequeueInputBufferIndex()).isEqualTo(1); assertThat(adapter.dequeueInputBufferIndex()).isEqualTo(1);
} }
@ -116,7 +119,7 @@ public class AsynchronousMediaCodecAdapterTest {
adapter.flush(); adapter.flush();
// Wait until all tasks have been handled. // Wait until all tasks have been handled.
Shadows.shadowOf(looper).idle(); shadowOf(looper).idle();
assertThrows( assertThrows(
IllegalStateException.class, IllegalStateException.class,
() -> { () -> {
@ -168,7 +171,7 @@ public class AsynchronousMediaCodecAdapterTest {
() -> adapter.getMediaCodecCallback().onOutputBufferAvailable(codec, /* index=*/ 1, info1)); () -> adapter.getMediaCodecCallback().onOutputBufferAvailable(codec, /* index=*/ 1, info1));
// Wait until all tasks have been handled. // Wait until all tasks have been handled.
Shadows.shadowOf(looper).idle(); shadowOf(looper).idle();
assertThat(adapter.dequeueOutputBufferIndex(bufferInfo)).isEqualTo(1); assertThat(adapter.dequeueOutputBufferIndex(bufferInfo)).isEqualTo(1);
assertBufferInfosEqual(info1, bufferInfo); assertBufferInfosEqual(info1, bufferInfo);
} }
@ -186,10 +189,68 @@ public class AsynchronousMediaCodecAdapterTest {
adapter.flush(); adapter.flush();
// Wait until all tasks have been handled. // Wait until all tasks have been handled.
Shadows.shadowOf(looper).idle(); shadowOf(looper).idle();
assertThrows(IllegalStateException.class, () -> adapter.dequeueOutputBufferIndex(bufferInfo)); assertThrows(IllegalStateException.class, () -> adapter.dequeueOutputBufferIndex(bufferInfo));
} }
@Test
public void dequeueOutputBufferIndex_withPendingOutputFormat_returnsPendingOutputFormat() {
MediaFormat pendingOutputFormat = new MediaFormat();
MediaCodec.BufferInfo outBufferInfo = new MediaCodec.BufferInfo();
MediaCodec.Callback mediaCodecCallback = adapter.getMediaCodecCallback();
Handler handler = new Handler(looper);
adapter.start();
// Enqueue callbacks
handler.post(() -> mediaCodecCallback.onOutputFormatChanged(codec, new MediaFormat()));
handler.post(
() ->
mediaCodecCallback.onOutputBufferAvailable(
codec, /* index= */ 0, new MediaCodec.BufferInfo()));
handler.post(() -> mediaCodecCallback.onOutputFormatChanged(codec, pendingOutputFormat));
handler.post(
() ->
mediaCodecCallback.onOutputBufferAvailable(
codec, /* index= */ 1, new MediaCodec.BufferInfo()));
adapter.flush();
// After flush is complete, MediaCodec sends on output buffer.
handler.post(
() ->
mediaCodecCallback.onOutputBufferAvailable(
codec, /* index= */ 2, new MediaCodec.BufferInfo()));
shadowOf(looper).idle();
assertThat(adapter.dequeueOutputBufferIndex(outBufferInfo))
.isEqualTo(MediaCodec.INFO_OUTPUT_FORMAT_CHANGED);
assertThat(adapter.getOutputFormat()).isEqualTo(pendingOutputFormat);
assertThat(adapter.dequeueOutputBufferIndex(outBufferInfo)).isEqualTo(2);
}
@Test
public void dequeueOutputBufferIndex_withPendingAndNewOutputFormat_returnsNewOutputFormat() {
MediaCodec.BufferInfo outBufferInfo = new MediaCodec.BufferInfo();
MediaCodec.Callback mediaCodecCallback = adapter.getMediaCodecCallback();
Handler handler = new Handler(looper);
adapter.start();
// Enqueue callbacks
handler.post(() -> mediaCodecCallback.onOutputFormatChanged(codec, new MediaFormat()));
handler.post(
() ->
mediaCodecCallback.onOutputBufferAvailable(
codec, /* index= */ 0, new MediaCodec.BufferInfo()));
adapter.flush();
// After flush is complete, MediaCodec sends an output format change, it should overwrite
// the pending format.
MediaFormat newMediaFormat = new MediaFormat();
handler.post(() -> mediaCodecCallback.onOutputFormatChanged(codec, newMediaFormat));
shadowOf(looper).idle();
assertThat(adapter.dequeueOutputBufferIndex(outBufferInfo))
.isEqualTo(MediaCodec.INFO_OUTPUT_FORMAT_CHANGED);
assertThat(adapter.getOutputFormat()).isEqualTo(newMediaFormat);
}
@Test @Test
public void getOutputFormat_withMultipleFormats_returnsFormatsInCorrectOrder() { public void getOutputFormat_withMultipleFormats_returnsFormatsInCorrectOrder() {
adapter.start(); adapter.start();
@ -223,7 +284,7 @@ public class AsynchronousMediaCodecAdapterTest {
adapter.flush(); adapter.flush();
// Wait until all tasks have been handled. // Wait until all tasks have been handled.
Shadows.shadowOf(looper).idle(); shadowOf(looper).idle();
assertThat(adapter.getOutputFormat()).isEqualTo(format); assertThat(adapter.getOutputFormat()).isEqualTo(format);
} }
@ -236,7 +297,7 @@ public class AsynchronousMediaCodecAdapterTest {
adapter.shutdown(); adapter.shutdown();
// Wait until all tasks have been handled. // Wait until all tasks have been handled.
Shadows.shadowOf(looper).idle(); shadowOf(looper).idle();
assertThat(onCodecStartCalled.get()).isEqualTo(1); assertThat(onCodecStartCalled.get()).isEqualTo(1);
} }
} }

View File

@ -221,6 +221,49 @@ public class DedicatedThreadAsyncMediaCodecAdapterTest {
assertThrows(IllegalStateException.class, () -> adapter.dequeueOutputBufferIndex(bufferInfo)); assertThrows(IllegalStateException.class, () -> adapter.dequeueOutputBufferIndex(bufferInfo));
} }
@Test
public void dequeueOutputBufferIndex_withPendingOutputFormat_returnsPendingOutputFormat() {
adapter.start();
MediaCodec.BufferInfo outputBufferInfo = new MediaCodec.BufferInfo();
MediaFormat pendingMediaFormat = new MediaFormat();
adapter.onOutputFormatChanged(codec, new MediaFormat());
adapter.onOutputBufferAvailable(codec, /* index= */ 0, new MediaCodec.BufferInfo());
adapter.onOutputFormatChanged(codec, pendingMediaFormat);
adapter.onOutputBufferAvailable(codec, /* index= */ 1, new MediaCodec.BufferInfo());
// Flush should clear the output queue except from the last pending output format received.
adapter.flush();
shadowOf(handlerThread.getLooper()).idle();
adapter.onOutputBufferAvailable(codec, /* index= */ 2, new MediaCodec.BufferInfo());
assertThat(adapter.dequeueOutputBufferIndex(outputBufferInfo))
.isEqualTo(MediaCodec.INFO_OUTPUT_FORMAT_CHANGED);
assertThat(adapter.getOutputFormat()).isEqualTo(pendingMediaFormat);
assertThat(adapter.dequeueOutputBufferIndex(outputBufferInfo)).isEqualTo(2);
}
@Test
public void dequeueOutputBufferIndex_withPendingAndNewOutputFormat_returnsNewOutputFormat() {
adapter.start();
MediaCodec.BufferInfo outputBufferInfo = new MediaCodec.BufferInfo();
MediaFormat pendingMediaFormat = new MediaFormat();
MediaFormat latestOutputFormat = new MediaFormat();
adapter.onOutputFormatChanged(codec, new MediaFormat());
adapter.onOutputBufferAvailable(codec, /* index= */ 0, new MediaCodec.BufferInfo());
adapter.onOutputFormatChanged(codec, pendingMediaFormat);
adapter.onOutputBufferAvailable(codec, /* index= */ 1, new MediaCodec.BufferInfo());
// Flush should clear the output queue except from the last pending output format received.
adapter.flush();
shadowOf(handlerThread.getLooper()).idle();
// New output format should overwrite the pending format.
adapter.onOutputFormatChanged(codec, latestOutputFormat);
assertThat(adapter.dequeueOutputBufferIndex(outputBufferInfo))
.isEqualTo(MediaCodec.INFO_OUTPUT_FORMAT_CHANGED);
assertThat(adapter.getOutputFormat()).isEqualTo(latestOutputFormat);
}
@Test @Test
public void getOutputFormat_withoutFormatReceived_throwsException() { public void getOutputFormat_withoutFormatReceived_throwsException() {
adapter.start(); adapter.start();

View File

@ -135,6 +135,47 @@ public class MediaCodecAsyncCallbackTest {
assertThat(mediaCodecAsyncCallback.dequeueOutputBufferIndex(outBufferInfo)).isEqualTo(2); assertThat(mediaCodecAsyncCallback.dequeueOutputBufferIndex(outBufferInfo)).isEqualTo(2);
} }
@Test
public void dequeOutputBufferIndex_withPendingOutputFormat_returnsPendingOutputFormat() {
MediaCodec.BufferInfo outBufferInfo = new MediaCodec.BufferInfo();
mediaCodecAsyncCallback.onOutputFormatChanged(codec, new MediaFormat());
MediaCodec.BufferInfo bufferInfo = new MediaCodec.BufferInfo();
mediaCodecAsyncCallback.onOutputBufferAvailable(codec, /* index= */ 0, bufferInfo);
MediaFormat pendingMediaFormat = new MediaFormat();
mediaCodecAsyncCallback.onOutputFormatChanged(codec, pendingMediaFormat);
// Flush should not discard the last format.
mediaCodecAsyncCallback.flush();
// First callback after flush is an output buffer, pending output format should be pushed first.
mediaCodecAsyncCallback.onOutputBufferAvailable(codec, /* index= */ 1, bufferInfo);
assertThat(mediaCodecAsyncCallback.dequeueOutputBufferIndex(outBufferInfo))
.isEqualTo(MediaCodec.INFO_OUTPUT_FORMAT_CHANGED);
assertThat(mediaCodecAsyncCallback.getOutputFormat()).isEqualTo(pendingMediaFormat);
assertThat(mediaCodecAsyncCallback.dequeueOutputBufferIndex(outBufferInfo)).isEqualTo(1);
}
@Test
public void dequeOutputBufferIndex_withPendingOutputFormatAndNewFormat_returnsNewFormat() {
mediaCodecAsyncCallback.onOutputFormatChanged(codec, new MediaFormat());
MediaCodec.BufferInfo bufferInfo = new MediaCodec.BufferInfo();
mediaCodecAsyncCallback.onOutputBufferAvailable(codec, /* index= */ 0, bufferInfo);
MediaFormat pendingMediaFormat = new MediaFormat();
mediaCodecAsyncCallback.onOutputFormatChanged(codec, pendingMediaFormat);
// Flush should not discard the last format
mediaCodecAsyncCallback.flush();
// The first callback after flush is a new MediaFormat, it should overwrite the pending format.
MediaFormat newFormat = new MediaFormat();
mediaCodecAsyncCallback.onOutputFormatChanged(codec, newFormat);
mediaCodecAsyncCallback.onOutputBufferAvailable(codec, /* index= */ 1, bufferInfo);
MediaCodec.BufferInfo outBufferInfo = new MediaCodec.BufferInfo();
assertThat(mediaCodecAsyncCallback.dequeueOutputBufferIndex(outBufferInfo))
.isEqualTo(MediaCodec.INFO_OUTPUT_FORMAT_CHANGED);
assertThat(mediaCodecAsyncCallback.getOutputFormat()).isEqualTo(newFormat);
assertThat(mediaCodecAsyncCallback.dequeueOutputBufferIndex(outBufferInfo)).isEqualTo(1);
}
@Test @Test
public void getOutputFormat_onNewInstance_raisesException() { public void getOutputFormat_onNewInstance_raisesException() {
try { try {