Commit eb5ffeb0 by ibaker Committed by Tianyi Feng

Stop suppressing exceptions in `MediaCodec.Callback` during flush

Previously `AsynchronousMediaCodecCallback.mediaCodecException` was
cleared when flushing completed. This behaviour was changed in
https://github.com/google/ExoPlayer/commit/aeff51c50772a82e44045e7d80e8a13390e2bb36
so now the exception is not cleared.

The result after that commit was that we would **only** suppress/ignore
the expression if a flush was currently pending, and we would throw it
both before and after the flush. This doesn't really make sense, so this
commit changes the behaviour to also throw the exception during the
flush.

This commit also corrects the assertion in
`flush_withPendingError_resetsError` and deflakes it so that it
consistently passes. The previous version of this test, although the
assertion was incorrect, would often pass because the
`dequeueInputBuffer` call would happen while the `flush` was still
pending, so the exception was suppressed.

#minor-release

PiperOrigin-RevId: 540237228
(cherry picked from commit dd17e73629930628e5cb313301ceabf0c776bd18)
parent 1e6dd8a2
......@@ -134,10 +134,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
*/
public int dequeueInputBufferIndex() {
synchronized (lock) {
maybeThrowException();
if (isFlushingOrShutdown()) {
return MediaCodec.INFO_TRY_AGAIN_LATER;
} else {
maybeThrowException();
return availableInputBuffers.isEmpty()
? MediaCodec.INFO_TRY_AGAIN_LATER
: availableInputBuffers.remove();
......@@ -153,10 +153,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
*/
public int dequeueOutputBufferIndex(MediaCodec.BufferInfo bufferInfo) {
synchronized (lock) {
maybeThrowException();
if (isFlushingOrShutdown()) {
return MediaCodec.INFO_TRY_AGAIN_LATER;
} else {
maybeThrowException();
if (availableOutputBuffers.isEmpty()) {
return MediaCodec.INFO_TRY_AGAIN_LATER;
} else {
......
......@@ -30,6 +30,7 @@ import android.os.Looper;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import java.io.IOException;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.After;
......@@ -106,6 +107,36 @@ public class AsynchronousMediaCodecCallbackTest {
}
@Test
public void dequeInputBufferIndex_withPendingFlushAndError_throwsError() throws Exception {
AtomicBoolean beforeFlushCompletes = new AtomicBoolean();
AtomicBoolean flushCompleted = new AtomicBoolean();
Looper callbackThreadLooper = callbackThread.getLooper();
Handler callbackHandler = new Handler(callbackThreadLooper);
ShadowLooper shadowCallbackLooper = shadowOf(callbackThreadLooper);
// Pause the callback thread so that flush() never completes.
shadowCallbackLooper.pause();
// Send two input buffers to the callback, then an error, and then flush().
asynchronousMediaCodecCallback.onInputBufferAvailable(codec, 0);
asynchronousMediaCodecCallback.onInputBufferAvailable(codec, 1);
MediaCodec.CodecException expectedException = createCodecException();
asynchronousMediaCodecCallback.onError(codec, expectedException);
callbackHandler.post(() -> beforeFlushCompletes.set(true));
asynchronousMediaCodecCallback.flush();
callbackHandler.post(() -> flushCompleted.set(true));
while (!beforeFlushCompletes.get()) {
shadowCallbackLooper.runOneTask();
}
assertThat(flushCompleted.get()).isFalse();
MediaCodec.CodecException actualException =
assertThrows(
MediaCodec.CodecException.class,
() -> asynchronousMediaCodecCallback.dequeueInputBufferIndex());
assertThat(actualException).isSameInstanceAs(expectedException);
}
@Test
public void dequeInputBufferIndex_afterFlush_returnsTryAgain() {
Looper callbackThreadLooper = callbackThread.getLooper();
AtomicBoolean flushCompleted = new AtomicBoolean();
......@@ -219,6 +250,39 @@ public class AsynchronousMediaCodecCallbackTest {
}
@Test
public void dequeOutputBufferIndex_withPendingFlushAndError_throwsError() throws Exception {
AtomicBoolean beforeFlushCompletes = new AtomicBoolean();
AtomicBoolean flushCompleted = new AtomicBoolean();
Looper callbackThreadLooper = callbackThread.getLooper();
Handler callbackHandler = new Handler(callbackThreadLooper);
ShadowLooper shadowCallbackLooper = shadowOf(callbackThreadLooper);
// Pause the callback thread so that flush() never completes.
shadowCallbackLooper.pause();
// Send two output buffers to the callback, then an error, and then flush().
MediaCodec.BufferInfo bufferInfo = new MediaCodec.BufferInfo();
asynchronousMediaCodecCallback.onOutputBufferAvailable(codec, 0, bufferInfo);
asynchronousMediaCodecCallback.onOutputBufferAvailable(codec, 1, bufferInfo);
MediaCodec.CodecException expectedException = createCodecException();
asynchronousMediaCodecCallback.onError(codec, expectedException);
callbackHandler.post(() -> beforeFlushCompletes.set(true));
asynchronousMediaCodecCallback.flush();
callbackHandler.post(() -> flushCompleted.set(true));
while (beforeFlushCompletes.get()) {
shadowCallbackLooper.runOneTask();
}
assertThat(flushCompleted.get()).isFalse();
MediaCodec.CodecException actualException =
assertThrows(
MediaCodec.CodecException.class,
() ->
asynchronousMediaCodecCallback.dequeueOutputBufferIndex(
new MediaCodec.BufferInfo()));
assertThat(actualException).isSameInstanceAs(expectedException);
}
@Test
public void dequeOutputBufferIndex_afterFlush_returnsTryAgain() {
Looper callbackThreadLooper = callbackThread.getLooper();
AtomicBoolean flushCompleted = new AtomicBoolean();
......@@ -438,13 +502,24 @@ public class AsynchronousMediaCodecCallbackTest {
}
@Test
public void flush_withPendingError_resetsError() throws Exception {
asynchronousMediaCodecCallback.onError(codec, createCodecException());
// Calling flush should clear any pending error.
public void flush_withPendingError_doesntResetError() throws Exception {
AtomicBoolean flushCompleted = new AtomicBoolean();
Looper callbackThreadLooper = callbackThread.getLooper();
ShadowLooper shadowCallbackLooper = shadowOf(callbackThreadLooper);
MediaCodec.CodecException expectedException = createCodecException();
asynchronousMediaCodecCallback.onError(codec, expectedException);
// Flush and progress the looper so that flush is completed.
asynchronousMediaCodecCallback.flush();
new Handler(callbackThreadLooper).post(() -> flushCompleted.set(true));
shadowCallbackLooper.idle();
assertThat(asynchronousMediaCodecCallback.dequeueInputBufferIndex())
.isEqualTo(MediaCodec.INFO_TRY_AGAIN_LATER);
assertThat(flushCompleted.get()).isTrue();
MediaCodec.CodecException actualException =
assertThrows(
MediaCodec.CodecException.class,
() -> asynchronousMediaCodecCallback.dequeueInputBufferIndex());
assertThat(actualException).isSameInstanceAs(expectedException);
}
@Test
......@@ -456,7 +531,11 @@ public class AsynchronousMediaCodecCallbackTest {
}
/** Reflectively create a {@link MediaCodec.CodecException}. */
private static MediaCodec.CodecException createCodecException() throws Exception {
private static MediaCodec.CodecException createCodecException()
throws NoSuchMethodException,
InvocationTargetException,
IllegalAccessException,
InstantiationException {
Constructor<MediaCodec.CodecException> constructor =
MediaCodec.CodecException.class.getDeclaredConstructor(
Integer.TYPE, Integer.TYPE, String.class);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment