Skip to content

Commit

Permalink
Async buffer queueing: do not throw from flush/shutdown
Browse files Browse the repository at this point in the history
The asynchronous MediaCodec adapter queues input buffers in a
background thread. If a codec queueuing operation throws an exception,
the buffer enqueuer will store it as a pending exception and re-throw it
the next time the adapter will attempt to queue another input buffer.

The buffer enqueuer's flush() and shutdown() may throw an exception if
the pending error is set. This is subject to a race-condition in which
the pending error can be set while the adapter is flushing/shutting down
the enqueuer, e.g., if an input buffer is still being queued and the
codec throws an exception. As a result, the adapter cannot flush or
shutdown gracefully.

This change makes the buffer enqueuer to ignore any pending error
when flushing/shuttinf down so that the adapter can flush/release
gracefully even if a queueing error was detected.

PiperOrigin-RevId: 409113054
  • Loading branch information
christosts authored and icbaker committed Nov 19, 2021
1 parent 1c8d00a commit 0221e0e
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package androidx.media3.exoplayer.mediacodec;

import static androidx.annotation.VisibleForTesting.NONE;
import static androidx.media3.common.util.Assertions.checkNotNull;
import static androidx.media3.common.util.Util.castNonNull;

Expand Down Expand Up @@ -147,7 +148,7 @@ public void flush() {
}
}

/** Shut down the instance. Make sure to call this method to release its internal resources. */
/** Shuts down the instance. Make sure to call this method to release its internal resources. */
public void shutdown() {
if (started) {
flush();
Expand All @@ -173,26 +174,23 @@ private void maybeThrowException() {
* blocks until the {@link #handlerThread} is idle.
*/
private void flushHandlerThread() throws InterruptedException {
Handler handler = castNonNull(this.handler);
handler.removeCallbacksAndMessages(null);
checkNotNull(this.handler).removeCallbacksAndMessages(null);
blockUntilHandlerThreadIsIdle();
// Check if any exceptions happened during the last queueing action.
maybeThrowException();
}

private void blockUntilHandlerThreadIsIdle() throws InterruptedException {
conditionVariable.close();
castNonNull(handler).obtainMessage(MSG_OPEN_CV).sendToTarget();
checkNotNull(handler).obtainMessage(MSG_OPEN_CV).sendToTarget();
conditionVariable.block();
}

// Called from the handler thread

@VisibleForTesting
@VisibleForTesting(otherwise = NONE)
/* package */ void setPendingRuntimeException(RuntimeException exception) {
pendingRuntimeException.set(exception);
}

// Called from the handler thread

private void doHandleMessage(Message msg) {
@Nullable MessageParams params = null;
switch (msg.what) {
Expand All @@ -214,7 +212,8 @@ private void doHandleMessage(Message msg) {
conditionVariable.open();
break;
default:
setPendingRuntimeException(new IllegalStateException(String.valueOf(msg.what)));
pendingRuntimeException.compareAndSet(
null, new IllegalStateException(String.valueOf(msg.what)));
}
if (params != null) {
recycleMessageParams(params);
Expand All @@ -226,7 +225,7 @@ private void doQueueInputBuffer(
try {
codec.queueInputBuffer(index, offset, size, presentationTimeUs, flag);
} catch (RuntimeException e) {
setPendingRuntimeException(e);
pendingRuntimeException.compareAndSet(null, e);
}
}

Expand All @@ -240,7 +239,7 @@ private void doQueueSecureInputBuffer(
codec.queueSecureInputBuffer(index, offset, info, presentationTimeUs, flags);
}
} catch (RuntimeException e) {
setPendingRuntimeException(e);
pendingRuntimeException.compareAndSet(null, e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,25 @@ public void flush_multipleTimes_works() {
enqueuer.flush();
}

@Test
public void flush_withPendingError_doesNotResetError() {
enqueuer.start();
enqueuer.setPendingRuntimeException(
new MediaCodec.CryptoException(/* errorCode= */ 0, /* detailMessage= */ null));

enqueuer.flush();

assertThrows(
MediaCodec.CryptoException.class,
() ->
enqueuer.queueInputBuffer(
/* index= */ 0,
/* offset= */ 0,
/* size= */ 0,
/* presentationTimeUs= */ 0,
/* flags= */ 0));
}

@Test
public void shutdown_withoutStart_works() {
enqueuer.shutdown();
Expand Down Expand Up @@ -219,6 +238,16 @@ public void shutdown_onInterruptedException_throwsIllegalStateException()
assertThrows(IllegalStateException.class, () -> enqueuer.shutdown());
}

@Test
public void shutdown_withPendingError_doesNotThrow() {
enqueuer.start();
enqueuer.setPendingRuntimeException(
new MediaCodec.CryptoException(/* errorCode= */ 0, /* detailMessage= */ null));

// Shutting down with a pending error set should not throw .
enqueuer.shutdown();
}

private static CryptoInfo createCryptoInfo() {
CryptoInfo info = new CryptoInfo();
int numSubSamples = 5;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,24 @@ public void getOutputFormat_afterFlushWithPendingFormat_returnsPendingFormat() {
assertThat(asynchronousMediaCodecCallback.dequeueOutputBufferIndex(outInfo)).isEqualTo(1);
}

@Test
public void flush_withPendingError_resetsError() throws Exception {
asynchronousMediaCodecCallback.onError(codec, createCodecException());
// Calling flush should clear any pending error.
asynchronousMediaCodecCallback.flush(/* codec= */ null);

assertThat(asynchronousMediaCodecCallback.dequeueInputBufferIndex())
.isEqualTo(MediaCodec.INFO_TRY_AGAIN_LATER);
}

@Test
public void shutdown_withPendingError_doesNotThrow() throws Exception {
asynchronousMediaCodecCallback.onError(codec, createCodecException());

// Calling shutdown() should not throw.
asynchronousMediaCodecCallback.shutdown();
}

/** Reflectively create a {@link MediaCodec.CodecException}. */
private static MediaCodec.CodecException createCodecException() throws Exception {
Constructor<MediaCodec.CodecException> constructor =
Expand Down

0 comments on commit 0221e0e

Please sign in to comment.