From 1ff0965a10ac59cd0839956a7d7f38bd0b7997e5 Mon Sep 17 00:00:00 2001 From: christosts Date: Tue, 1 Dec 2020 16:20:44 +0000 Subject: [PATCH] Do not advance SystemClock manually in tests Changes MetadataRetriever and Transformer so that their respective tests don't need to manually control the SystemClock in order to execute taks posted with delay from Loader. PiperOrigin-RevId: 345024140 --- .../android/exoplayer2/MetadataRetriever.java | 37 ++++++++---- .../exoplayer2/util/HandlerWrapper.java | 3 + .../exoplayer2/util/SystemHandlerWrapper.java | 5 ++ .../exoplayer2/MetadataRetrieverTest.java | 58 +++++++++---------- .../testutil/AutoAdvancingFakeClock.java | 17 ++++-- .../exoplayer2/testutil/FakeClock.java | 5 ++ 6 files changed, 78 insertions(+), 47 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/MetadataRetriever.java b/library/core/src/main/java/com/google/android/exoplayer2/MetadataRetriever.java index 6deb792c1b..4c48cd3141 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/MetadataRetriever.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/MetadataRetriever.java @@ -22,6 +22,7 @@ import android.content.Context; import android.os.Handler; import android.os.HandlerThread; import android.os.Message; +import androidx.annotation.VisibleForTesting; import com.google.android.exoplayer2.extractor.DefaultExtractorsFactory; import com.google.android.exoplayer2.extractor.ExtractorsFactory; import com.google.android.exoplayer2.extractor.mp4.Mp4Extractor; @@ -32,7 +33,8 @@ import com.google.android.exoplayer2.source.MediaSourceFactory; import com.google.android.exoplayer2.source.TrackGroupArray; import com.google.android.exoplayer2.upstream.Allocator; import com.google.android.exoplayer2.upstream.DefaultAllocator; -import com.google.android.exoplayer2.util.Util; +import com.google.android.exoplayer2.util.Clock; +import com.google.android.exoplayer2.util.HandlerWrapper; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @@ -56,13 +58,7 @@ public final class MetadataRetriever { */ public static ListenableFuture retrieveMetadata( Context context, MediaItem mediaItem) { - ExtractorsFactory extractorsFactory = - new DefaultExtractorsFactory() - .setMp4ExtractorFlags( - Mp4Extractor.FLAG_READ_MOTION_PHOTO_METADATA | Mp4Extractor.FLAG_READ_SEF_DATA); - MediaSourceFactory mediaSourceFactory = - new DefaultMediaSourceFactory(context, extractorsFactory); - return retrieveMetadata(mediaSourceFactory, mediaItem); + return retrieveMetadata(context, mediaItem, Clock.DEFAULT); } /** @@ -77,9 +73,26 @@ public final class MetadataRetriever { */ public static ListenableFuture retrieveMetadata( MediaSourceFactory mediaSourceFactory, MediaItem mediaItem) { + return retrieveMetadata(mediaSourceFactory, mediaItem, Clock.DEFAULT); + } + + @VisibleForTesting + /* package */ static ListenableFuture retrieveMetadata( + Context context, MediaItem mediaItem, Clock clock) { + ExtractorsFactory extractorsFactory = + new DefaultExtractorsFactory() + .setMp4ExtractorFlags( + Mp4Extractor.FLAG_READ_MOTION_PHOTO_METADATA | Mp4Extractor.FLAG_READ_SEF_DATA); + MediaSourceFactory mediaSourceFactory = + new DefaultMediaSourceFactory(context, extractorsFactory); + return retrieveMetadata(mediaSourceFactory, mediaItem, clock); + } + + private static ListenableFuture retrieveMetadata( + MediaSourceFactory mediaSourceFactory, MediaItem mediaItem, Clock clock) { // Recreate thread and handler every time this method is called so that it can be used // concurrently. - return new MetadataRetrieverInternal(mediaSourceFactory).retrieveMetadata(mediaItem); + return new MetadataRetrieverInternal(mediaSourceFactory, clock).retrieveMetadata(mediaItem); } private static final class MetadataRetrieverInternal { @@ -91,15 +104,15 @@ public final class MetadataRetriever { private final MediaSourceFactory mediaSourceFactory; private final HandlerThread mediaSourceThread; - private final Handler mediaSourceHandler; + private final HandlerWrapper mediaSourceHandler; private final SettableFuture trackGroupsFuture; - public MetadataRetrieverInternal(MediaSourceFactory mediaSourceFactory) { + public MetadataRetrieverInternal(MediaSourceFactory mediaSourceFactory, Clock clock) { this.mediaSourceFactory = mediaSourceFactory; mediaSourceThread = new HandlerThread("ExoPlayer:MetadataRetriever"); mediaSourceThread.start(); mediaSourceHandler = - Util.createHandler(mediaSourceThread.getLooper(), new MediaSourceHandlerCallback()); + clock.createHandler(mediaSourceThread.getLooper(), new MediaSourceHandlerCallback()); trackGroupsFuture = SettableFuture.create(); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/util/HandlerWrapper.java b/library/core/src/main/java/com/google/android/exoplayer2/util/HandlerWrapper.java index 5b85b26c3f..8343d27f42 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/util/HandlerWrapper.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/util/HandlerWrapper.java @@ -44,6 +44,9 @@ public interface HandlerWrapper { /** @see Handler#sendEmptyMessage(int) */ boolean sendEmptyMessage(int what); + /** @see Handler#sendEmptyMessageDelayed(int, long) */ + boolean sendEmptyMessageDelayed(int what, int delayMs); + /** @see Handler#sendEmptyMessageAtTime(int, long) */ boolean sendEmptyMessageAtTime(int what, long uptimeMs); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/util/SystemHandlerWrapper.java b/library/core/src/main/java/com/google/android/exoplayer2/util/SystemHandlerWrapper.java index 1fbea2ed7e..dc0f93165a 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/util/SystemHandlerWrapper.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/util/SystemHandlerWrapper.java @@ -58,6 +58,11 @@ import androidx.annotation.Nullable; return handler.sendEmptyMessage(what); } + @Override + public boolean sendEmptyMessageDelayed(int what, int delayMs) { + return handler.sendEmptyMessageDelayed(what, delayMs); + } + @Override public boolean sendEmptyMessageAtTime(int what, long uptimeMs) { return handler.sendEmptyMessageAtTime(what, uptimeMs); diff --git a/library/core/src/test/java/com/google/android/exoplayer2/MetadataRetrieverTest.java b/library/core/src/test/java/com/google/android/exoplayer2/MetadataRetrieverTest.java index 87225bc6f3..e9a3e351d2 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/MetadataRetrieverTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/MetadataRetrieverTest.java @@ -22,17 +22,18 @@ import static org.junit.Assert.assertThrows; import android.content.Context; import android.net.Uri; -import android.os.SystemClock; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.metadata.mp4.MotionPhotoMetadata; import com.google.android.exoplayer2.metadata.mp4.SlowMotionData; import com.google.android.exoplayer2.source.TrackGroupArray; +import com.google.android.exoplayer2.testutil.AutoAdvancingFakeClock; import com.google.android.exoplayer2.util.MimeTypes; import com.google.common.util.concurrent.ListenableFuture; import java.util.ArrayList; import java.util.List; import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,11 +42,15 @@ import org.junit.runner.RunWith; @RunWith(AndroidJUnit4.class) public class MetadataRetrieverTest { + private static final long TEST_TIMEOUT_SEC = 10; + private Context context; + private AutoAdvancingFakeClock clock; @Before public void setUp() throws Exception { context = ApplicationProvider.getApplicationContext(); + clock = new AutoAdvancingFakeClock(); } @Test @@ -53,8 +58,9 @@ public class MetadataRetrieverTest { MediaItem mediaItem = MediaItem.fromUri(Uri.parse("asset://android_asset/media/mp4/sample.mp4")); - ListenableFuture trackGroupsFuture = retrieveMetadata(context, mediaItem); - TrackGroupArray trackGroups = waitAndGetTrackGroups(trackGroupsFuture); + ListenableFuture trackGroupsFuture = + retrieveMetadata(context, mediaItem, clock); + TrackGroupArray trackGroups = trackGroupsFuture.get(TEST_TIMEOUT_SEC, TimeUnit.SECONDS); assertThat(trackGroups.length).isEqualTo(2); // Video group. @@ -72,10 +78,12 @@ public class MetadataRetrieverTest { MediaItem mediaItem2 = MediaItem.fromUri(Uri.parse("asset://android_asset/media/mp3/bear-id3.mp3")); - ListenableFuture trackGroupsFuture1 = retrieveMetadata(context, mediaItem1); - ListenableFuture trackGroupsFuture2 = retrieveMetadata(context, mediaItem2); - TrackGroupArray trackGroups1 = waitAndGetTrackGroups(trackGroupsFuture1); - TrackGroupArray trackGroups2 = waitAndGetTrackGroups(trackGroupsFuture2); + ListenableFuture trackGroupsFuture1 = + retrieveMetadata(context, mediaItem1, clock); + ListenableFuture trackGroupsFuture2 = + retrieveMetadata(context, mediaItem2, clock); + TrackGroupArray trackGroups1 = trackGroupsFuture1.get(TEST_TIMEOUT_SEC, TimeUnit.SECONDS); + TrackGroupArray trackGroups2 = trackGroupsFuture2.get(TEST_TIMEOUT_SEC, TimeUnit.SECONDS); // First track group. assertThat(trackGroups1.length).isEqualTo(2); @@ -104,8 +112,9 @@ public class MetadataRetrieverTest { /* videoStartPosition= */ 28_869, /* videoSize= */ 28_803); - ListenableFuture trackGroupsFuture = retrieveMetadata(context, mediaItem); - TrackGroupArray trackGroups = waitAndGetTrackGroups(trackGroupsFuture); + ListenableFuture trackGroupsFuture = + retrieveMetadata(context, mediaItem, clock); + TrackGroupArray trackGroups = trackGroupsFuture.get(TEST_TIMEOUT_SEC, TimeUnit.SECONDS); assertThat(trackGroups.length).isEqualTo(1); assertThat(trackGroups.get(0).length).isEqualTo(1); @@ -119,8 +128,9 @@ public class MetadataRetrieverTest { MediaItem mediaItem = MediaItem.fromUri(Uri.parse("asset://android_asset/media/mp4/sample_still_photo.heic")); - ListenableFuture trackGroupsFuture = retrieveMetadata(context, mediaItem); - TrackGroupArray trackGroups = waitAndGetTrackGroups(trackGroupsFuture); + ListenableFuture trackGroupsFuture = + retrieveMetadata(context, mediaItem, clock); + TrackGroupArray trackGroups = trackGroupsFuture.get(TEST_TIMEOUT_SEC, TimeUnit.SECONDS); assertThat(trackGroups.length).isEqualTo(1); assertThat(trackGroups.get(0).length).isEqualTo(1); @@ -140,15 +150,14 @@ public class MetadataRetrieverTest { /* startTimeMs= */ 1255, /* endTimeMs= */ 1970, /* speedDivisor= */ 8)); SlowMotionData expectedSlowMotionData = new SlowMotionData(segments); - ListenableFuture trackGroupsFuture = retrieveMetadata(context, mediaItem); - TrackGroupArray trackGroups = waitAndGetTrackGroups(trackGroupsFuture); + ListenableFuture trackGroupsFuture = + retrieveMetadata(context, mediaItem, clock); + TrackGroupArray trackGroups = trackGroupsFuture.get(TEST_TIMEOUT_SEC, TimeUnit.SECONDS); assertThat(trackGroups.length).isEqualTo(2); // Video and audio - // Audio assertThat(trackGroups.get(0).getFormat(0).metadata.length()).isEqualTo(1); assertThat(trackGroups.get(0).getFormat(0).metadata.get(0)).isEqualTo(expectedSlowMotionData); - // Video assertThat(trackGroups.get(1).getFormat(0).metadata.length()) .isEqualTo(3); // 2 Mdta entries and 1 slow motion entry. @@ -160,21 +169,10 @@ public class MetadataRetrieverTest { MediaItem mediaItem = MediaItem.fromUri(Uri.parse("asset://android_asset/media/does_not_exist")); - ListenableFuture trackGroupsFuture = retrieveMetadata(context, mediaItem); + ListenableFuture trackGroupsFuture = + retrieveMetadata(context, mediaItem, clock); - assertThrows(ExecutionException.class, () -> waitAndGetTrackGroups(trackGroupsFuture)); - } - - private static TrackGroupArray waitAndGetTrackGroups( - ListenableFuture trackGroupsFuture) - throws InterruptedException, ExecutionException { - while (!trackGroupsFuture.isDone()) { - // TODO: update once [Internal: b/168084145] is implemented. - // Advance SystemClock so that messages that are sent with a delay to the MetadataRetriever - // looper are received. - SystemClock.setCurrentTimeMillis(SystemClock.uptimeMillis() + 100); - Thread.sleep(/* millis= */ 100); - } - return trackGroupsFuture.get(); + assertThrows( + ExecutionException.class, () -> trackGroupsFuture.get(TEST_TIMEOUT_SEC, TimeUnit.SECONDS)); } } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/AutoAdvancingFakeClock.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/AutoAdvancingFakeClock.java index 7bef854ba9..2507397ca0 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/AutoAdvancingFakeClock.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/AutoAdvancingFakeClock.java @@ -15,20 +15,22 @@ */ package com.google.android.exoplayer2.testutil; +import androidx.annotation.Nullable; import com.google.android.exoplayer2.util.HandlerWrapper; -import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** * {@link FakeClock} extension which automatically advances time whenever an empty message is * enqueued at a future time. * - *

The clock time is advanced to the time of enqueued empty messages. Only the first Handler - * sending messages at a future time will be allowed to advance time to ensure there is only one - * primary time source. This should usually be the Handler of the internal playback loop. + *

The clock time is advanced to the time of enqueued empty messages. The first Handler sending + * messages at a future time will be allowed to advance time to ensure there is only one primary + * time source at a time. This should usually be the Handler of the internal playback loop. You can + * {@link #resetHandler() reset the handler} so that the next Handler that sends messages at a + * future time becomes the primary time source. */ public final class AutoAdvancingFakeClock extends FakeClock { - private @MonotonicNonNull HandlerWrapper autoAdvancingHandler; + @Nullable private HandlerWrapper autoAdvancingHandler; /** Creates the auto-advancing clock with an initial time of 0. */ public AutoAdvancingFakeClock() { @@ -57,4 +59,9 @@ public final class AutoAdvancingFakeClock extends FakeClock { } return result; } + + /** Resets the internal handler, so that this clock can later be used with another handler. */ + public void resetHandler() { + autoAdvancingHandler = null; + } } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeClock.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeClock.java index 077b690b34..64d6ceb0e2 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeClock.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeClock.java @@ -223,6 +223,11 @@ public class FakeClock implements Clock { return handler.sendEmptyMessage(what); } + @Override + public boolean sendEmptyMessageDelayed(int what, int delayMs) { + return addHandlerMessageAtTime(this, what, uptimeMillis() + delayMs); + } + @Override public boolean sendEmptyMessageAtTime(int what, long uptimeMs) { return addHandlerMessageAtTime(this, what, uptimeMs);