Fix flaky unit tests

PiperOrigin-RevId: 334580007

Fix flaky unit tests
PiperOrigin-RevId: 334580007
a0d99a6a · christosts · kim-vde · c35787a0 · a0d99a6a · a0d99a6a
Commit a0d99a6a authored Sep 30, 2020 by christosts Committed by kim-vde Oct 06, 2020
Showing with 50 additions and 48 deletions
library/core/src/main/java/com/google/android/exoplayer2/mediacodec/AsynchronousMediaCodecAdapter.java
library/core/src/main/java/com/google/android/exoplayer2/mediacodec/AsynchronousMediaCodecBufferEnqueuer.java
library/core/src/test/java/com/google/android/exoplayer2/mediacodec/AsynchronousMediaCodecAdapterTest.java
--- a/library/core/src/main/java/com/google/android/exoplayer2/mediacodec/AsynchronousMediaCodecAdapter.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/mediacodec/AsynchronousMediaCodecAdapter.java
@@ -83,19 +83,24 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
   *     labelling the internal thread accordingly.
   */
  /* package */ AsynchronousMediaCodecAdapter(MediaCodec codec, int trackType) {
-    this(codec, trackType, new HandlerThread(createThreadLabel(trackType)));
+    this(
+        codec,
+        trackType,
+        new HandlerThread(createCallbackThreadLabel(trackType)),
+        new HandlerThread(createQueueingThreadLabel(trackType)));
  }
  @VisibleForTesting
  /* package */ AsynchronousMediaCodecAdapter(
      MediaCodec codec,
      int trackType,
-      HandlerThread handlerThread) {
+      HandlerThread callbackThread,
+      HandlerThread enqueueingThread) {
    this.lock = new Object();
    this.mediaCodecAsyncCallback = new MediaCodecAsyncCallback();
    this.codec = codec;
-    this.handlerThread = handlerThread;
+    this.handlerThread = callbackThread;
-    this.bufferEnqueuer = new AsynchronousMediaCodecBufferEnqueuer(codec, trackType);
+    this.bufferEnqueuer = new AsynchronousMediaCodecBufferEnqueuer(codec, enqueueingThread);
    this.state = STATE_CREATED;
  }
@@ -276,8 +281,16 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
    }
  }
-  private static String createThreadLabel(int trackType) {
+  private static String createCallbackThreadLabel(int trackType) {
-    StringBuilder labelBuilder = new StringBuilder("ExoPlayer:MediaCodecAsyncAdapter:");
+    return createThreadLabel(trackType, /* prefix= */ "ExoPlayer:MediaCodecAsyncAdapter:");
+  }
+  private static String createQueueingThreadLabel(int trackType) {
+    return createThreadLabel(trackType, /* prefix= */ "ExoPlayer:MediaCodecQueueingThread:");
+  }
+  private static String createThreadLabel(int trackType, String prefix) {
+    StringBuilder labelBuilder = new StringBuilder(prefix);
    if (trackType == C.TRACK_TYPE_AUDIO) {
      labelBuilder.append("Audio");
    } else if (trackType == C.TRACK_TYPE_VIDEO) {

--- a/library/core/src/main/java/com/google/android/exoplayer2/mediacodec/AsynchronousMediaCodecBufferEnqueuer.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/mediacodec/AsynchronousMediaCodecBufferEnqueuer.java
@@ -26,7 +26,6 @@ import androidx.annotation.GuardedBy;
 import androidx.annotation.Nullable;
 import androidx.annotation.RequiresApi;
 import androidx.annotation.VisibleForTesting;
-import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.decoder.CryptoInfo;
 import com.google.android.exoplayer2.util.ConditionVariable;
 import com.google.android.exoplayer2.util.Util;
@@ -66,13 +65,10 @@ class AsynchronousMediaCodecBufferEnqueuer {
   * Creates a new instance that submits input buffers on the specified {@link MediaCodec}.
   *
   * @param codec The {@link MediaCodec} to submit input buffers to.
-   * @param trackType The type of stream (used for debug logs).
+   * @param queueingThread The {@link HandlerThread} to use for queueing buffers.
   */
-  public AsynchronousMediaCodecBufferEnqueuer(MediaCodec codec, int trackType) {
+  public AsynchronousMediaCodecBufferEnqueuer(MediaCodec codec, HandlerThread queueingThread) {
-    this(
+    this(codec, queueingThread, /* conditionVariable= */ new ConditionVariable());
-        codec,
-        new HandlerThread(createThreadLabel(trackType)),
-        /* conditionVariable= */ new ConditionVariable());
  }
  @VisibleForTesting
@@ -291,18 +287,6 @@ class AsynchronousMediaCodecBufferEnqueuer {
    return manufacturer.contains("samsung") || manufacturer.contains("motorola");
  }
-  private static String createThreadLabel(int trackType) {
-    StringBuilder labelBuilder = new StringBuilder("ExoPlayer:MediaCodecBufferEnqueuer:");
-    if (trackType == C.TRACK_TYPE_AUDIO) {
-      labelBuilder.append("Audio");
-    } else if (trackType == C.TRACK_TYPE_VIDEO) {
-      labelBuilder.append("Video");
-    } else {
-      labelBuilder.append("Unknown(").append(trackType).append(")");
-    }
-    return labelBuilder.toString();
-  }
  /** Performs a deep copy of {@code cryptoInfo} to {@code frameworkCryptoInfo}. */
  private static void copy(
      CryptoInfo cryptoInfo, android.media.MediaCodec.CryptoInfo frameworkCryptoInfo) {

--- a/library/core/src/test/java/com/google/android/exoplayer2/mediacodec/AsynchronousMediaCodecAdapterTest.java
+++ b/library/core/src/test/java/com/google/android/exoplayer2/mediacodec/AsynchronousMediaCodecAdapterTest.java
@@ -38,26 +38,27 @@ import org.robolectric.shadows.ShadowLooper;
 public class AsynchronousMediaCodecAdapterTest {
  private AsynchronousMediaCodecAdapter adapter;
  private MediaCodec codec;
-  private TestHandlerThread handlerThread;
+  private TestHandlerThread callbackThread;
+  private HandlerThread queueingThread;
  private MediaCodec.BufferInfo bufferInfo;
  @Before
  public void setUp() throws IOException {
    codec = MediaCodec.createByCodecName("h264");
-    handlerThread = new TestHandlerThread("TestHandlerThread");
+    callbackThread = new TestHandlerThread("TestCallbackThread");
+    queueingThread = new HandlerThread("TestQueueingThread");
    adapter =
        new AsynchronousMediaCodecAdapter(
-            codec,
+            codec, /* trackType= */ C.TRACK_TYPE_VIDEO, callbackThread, queueingThread);
-            /* trackType= */ C.TRACK_TYPE_VIDEO,
-            handlerThread);
    bufferInfo = new MediaCodec.BufferInfo();
  }
  @After
  public void tearDown() {
    adapter.shutdown();
+    codec.release();
-    assertThat(handlerThread.hasQuit()).isTrue();
+    assertThat(callbackThread.hasQuit()).isTrue();
  }
  @Test
@@ -66,7 +67,7 @@ public class AsynchronousMediaCodecAdapterTest {
        createMediaFormat("foo"), /* surface= */ null, /* crypto= */ null, /* flags= */ 0);
    // After adapter.start(), the ShadowMediaCodec offers one input buffer. We pause the looper so
    // that the buffer is not propagated to the adapter.
-    shadowOf(handlerThread.getLooper()).pause();
+    shadowOf(callbackThread.getLooper()).pause();
    adapter.start();
    assertThat(adapter.dequeueInputBufferIndex()).isEqualTo(MediaCodec.INFO_TRY_AGAIN_LATER);
@@ -79,7 +80,7 @@ public class AsynchronousMediaCodecAdapterTest {
    adapter.start();
    // After start(), the ShadowMediaCodec offers input buffer 0. We advance the looper to make sure
    // and messages have been propagated to the adapter.
-    shadowOf(handlerThread.getLooper()).idle();
+    shadowOf(callbackThread.getLooper()).idle();
    assertThat(adapter.dequeueInputBufferIndex()).isEqualTo(0);
  }
@@ -92,7 +93,7 @@ public class AsynchronousMediaCodecAdapterTest {
    // After adapter.start(), the ShadowMediaCodec offers input buffer 0. We run all currently
    // enqueued messages and pause the looper so that flush is not completed.
-    ShadowLooper shadowLooper = shadowOf(handlerThread.getLooper());
+    ShadowLooper shadowLooper = shadowOf(callbackThread.getLooper());
    shadowLooper.idle();
    shadowLooper.pause();
    adapter.flush();
@@ -107,7 +108,7 @@ public class AsynchronousMediaCodecAdapterTest {
    adapter.start();
    // After adapter.start(), the ShadowMediaCodec offers input buffer 0. We advance the looper to
    // make sure all messages have been propagated to the adapter.
-    ShadowLooper shadowLooper = shadowOf(handlerThread.getLooper());
+    ShadowLooper shadowLooper = shadowOf(callbackThread.getLooper());
    shadowLooper.idle();
    adapter.flush();
@@ -123,7 +124,7 @@ public class AsynchronousMediaCodecAdapterTest {
    adapter.configure(
        createMediaFormat("foo"), /* surface= */ null, /* crypto= */ null, /* flags= */ 0);
    // Pause the looper so that we interact with the adapter from this thread only.
-    shadowOf(handlerThread.getLooper()).pause();
+    shadowOf(callbackThread.getLooper()).pause();
    adapter.start();
    // Set an error directly on the adapter (not through the looper).
@@ -140,7 +141,7 @@ public class AsynchronousMediaCodecAdapterTest {
    // After start(), the ShadowMediaCodec offers input buffer 0, which is available only if we
    // progress the adapter's looper. We progress the looper so that we call shutdown() on a
    // non-empty adapter.
-    shadowOf(handlerThread.getLooper()).idle();
+    shadowOf(callbackThread.getLooper()).idle();
    adapter.shutdown();
@@ -155,7 +156,7 @@ public class AsynchronousMediaCodecAdapterTest {
    adapter.start();
    // After start(), the ShadowMediaCodec offers an output format change. We progress the looper
    // so that the format change is propagated to the adapter.
-    shadowOf(handlerThread.getLooper()).idle();
+    shadowOf(callbackThread.getLooper()).idle();
    assertThat(adapter.dequeueOutputBufferIndex(bufferInfo))
        .isEqualTo(MediaCodec.INFO_OUTPUT_FORMAT_CHANGED);
@@ -171,13 +172,17 @@ public class AsynchronousMediaCodecAdapterTest {
    adapter.start();
    // After start(), the ShadowMediaCodec offers input buffer 0, which is available only if we
    // progress the adapter's looper.
-    ShadowLooper shadowLooper = shadowOf(handlerThread.getLooper());
+    ShadowLooper callbackShadowLooper = shadowOf(callbackThread.getLooper());
-    shadowLooper.idle();
+    callbackShadowLooper.idle();
    int index = adapter.dequeueInputBufferIndex();
    adapter.queueInputBuffer(index, 0, 0, 0, 0);
-    // Progress the looper so that the ShadowMediaCodec processes the input buffer.
+    // Progress the queueuing looper first so the asynchronous enqueuer submits the input buffer,
-    shadowLooper.idle();
+    // the ShadowMediaCodec processes the input buffer and produces an output buffer. Then, progress
+    // the callback looper so that the available output buffer callback is handled and the output
+    // buffer reaches the adapter.
+    shadowOf(queueingThread.getLooper()).idle();
+    callbackShadowLooper.idle();
    // The ShadowMediaCodec will first offer an output format and then the output buffer.
    assertThat(adapter.dequeueOutputBufferIndex(bufferInfo))
@@ -194,7 +199,7 @@ public class AsynchronousMediaCodecAdapterTest {
    adapter.start();
    // After start(), the ShadowMediaCodec offers input buffer 0, which is available only if we
    // progress the adapter's looper.
-    ShadowLooper shadowLooper = shadowOf(handlerThread.getLooper());
+    ShadowLooper shadowLooper = shadowOf(callbackThread.getLooper());
    shadowLooper.idle();
    // Flush enqueues a task in the looper, but we will pause the looper to leave flush()
@@ -211,7 +216,7 @@ public class AsynchronousMediaCodecAdapterTest {
    // Pause the looper so that we interact with the adapter from this thread only.
    adapter.configure(
        createMediaFormat("foo"), /* surface= */ null, /* crypto= */ null, /* flags= */ 0);
-    shadowOf(handlerThread.getLooper()).pause();
+    shadowOf(callbackThread.getLooper()).pause();
    adapter.start();
    // Set an error directly on the adapter.
@@ -227,7 +232,7 @@ public class AsynchronousMediaCodecAdapterTest {
    adapter.start();
    // After start(), the ShadowMediaCodec offers input buffer 0, which is available only if we
    // progress the adapter's looper.
-    ShadowLooper shadowLooper = shadowOf(handlerThread.getLooper());
+    ShadowLooper shadowLooper = shadowOf(callbackThread.getLooper());
    shadowLooper.idle();
    int index = adapter.dequeueInputBufferIndex();
@@ -246,7 +251,7 @@ public class AsynchronousMediaCodecAdapterTest {
        createMediaFormat("foo"), /* surface= */ null, /* crypto= */ null, /* flags= */ 0);
    // After start() the ShadowMediaCodec offers an output format change. Pause the looper so that
    // the format change is not propagated to the adapter.
-    shadowOf(handlerThread.getLooper()).pause();
+    shadowOf(callbackThread.getLooper()).pause();
    adapter.start();
    assertThrows(IllegalStateException.class, () -> adapter.getOutputFormat());
@@ -259,7 +264,7 @@ public class AsynchronousMediaCodecAdapterTest {
    adapter.start();
    // After start(), the ShadowMediaCodec offers an output format, which is available only if we
    // progress the adapter's looper.
-    shadowOf(handlerThread.getLooper()).idle();
+    shadowOf(callbackThread.getLooper()).idle();
    // Add another format directly on the adapter.
    adapter.onOutputFormatChanged(codec, createMediaFormat("format2"));
@@ -283,7 +288,7 @@ public class AsynchronousMediaCodecAdapterTest {
    adapter.start();
    // After start(), the ShadowMediaCodec offers an output format, which is available only if we
    // progress the adapter's looper.
-    ShadowLooper shadowLooper = shadowOf(handlerThread.getLooper());
+    ShadowLooper shadowLooper = shadowOf(callbackThread.getLooper());
    shadowLooper.idle();
    adapter.dequeueOutputBufferIndex(bufferInfo);