From 117db48907ea7427a724082e5e80e175b074784c Mon Sep 17 00:00:00 2001 From: bachinger Date: Tue, 15 Apr 2025 04:06:00 -0700 Subject: [PATCH] Assert preload looper is not the main looper Issue: androidx/media#2315 PiperOrigin-RevId: 747809915 --- .../androidx/media3/exoplayer/ExoPlayer.java | 5 +- .../source/preload/DefaultPreloadManager.java | 5 +- .../preload/DefaultPreloadManagerTest.java | 47 +++++++++++-------- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayer.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayer.java index 9543742d8b..9b5291c476 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayer.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayer.java @@ -1057,12 +1057,13 @@ public interface ExoPlayer extends Player { * * @param playbackLooper A {@link Looper}. * @return This builder. - * @throws IllegalStateException If {@link #build()} has already been called. + * @throws IllegalStateException If {@link #build()} has already been called, or when the + * {@linkplain Looper#getMainLooper() main looper} is passed in. */ @CanIgnoreReturnValue @UnstableApi public Builder setPlaybackLooper(Looper playbackLooper) { - checkState(!buildCalled); + checkState(!buildCalled && playbackLooper != Looper.getMainLooper()); this.playbackLooperProvider = new PlaybackLooperProvider(playbackLooper); return this; } diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/preload/DefaultPreloadManager.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/preload/DefaultPreloadManager.java index c9054ac71e..2ed2743555 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/preload/DefaultPreloadManager.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/preload/DefaultPreloadManager.java @@ -197,11 +197,12 @@ public final class DefaultPreloadManager extends BasePreloadManager { * @param preloadLooper A {@link Looper}. * @return This builder. * @throws IllegalStateException If {@link #build()}, {@link #buildExoPlayer()} or {@link - * #buildExoPlayer(ExoPlayer.Builder)} has already been called. + * #buildExoPlayer(ExoPlayer.Builder)} has already been called, or when the {@linkplain + * Looper#getMainLooper() main looper} is passed in. */ @CanIgnoreReturnValue public Builder setPreloadLooper(Looper preloadLooper) { - checkState(!buildCalled && !buildExoPlayerCalled); + checkState(!buildCalled && !buildExoPlayerCalled && preloadLooper != Looper.getMainLooper()); this.preloadLooperProvider = new PlaybackLooperProvider(preloadLooper); return this; } diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/preload/DefaultPreloadManagerTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/preload/DefaultPreloadManagerTest.java index 4d588c5736..928999e42d 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/preload/DefaultPreloadManagerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/preload/DefaultPreloadManagerTest.java @@ -30,7 +30,6 @@ import static org.robolectric.Shadows.shadowOf; import android.content.Context; import android.net.Uri; import android.os.HandlerThread; -import android.os.Looper; import androidx.annotation.Nullable; import androidx.media3.common.AdPlaybackState; import androidx.media3.common.C; @@ -68,7 +67,9 @@ import com.google.common.collect.Iterables; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -81,6 +82,7 @@ public class DefaultPreloadManagerTest { private Context context; @Mock private TargetPreloadStatusControl mockTargetPreloadStatusControl; private RenderersFactory renderersFactory; + private HandlerThread preloadThread; @Before public void setUp() { @@ -95,6 +97,13 @@ public class DefaultPreloadManagerTest { SystemClock.DEFAULT.createHandler(handler.getLooper(), /* callback= */ null), audioListener) }; + preloadThread = new HandlerThread("DefaultPreloadManagerTest"); + preloadThread.start(); + } + + @After + public void tearDown() { + preloadThread.quit(); } @Test @@ -102,7 +111,7 @@ public class DefaultPreloadManagerTest { DefaultPreloadManager preloadManager = new DefaultPreloadManager.Builder(context, mockTargetPreloadStatusControl) .setRenderersFactory(renderersFactory) - .setPreloadLooper(Util.getCurrentOrMainLooper()) + .setPreloadLooper(preloadThread.getLooper()) .build(); MediaItem.Builder mediaItemBuilder = new MediaItem.Builder(); MediaItem mediaItem1 = @@ -123,7 +132,7 @@ public class DefaultPreloadManagerTest { DefaultPreloadManager preloadManager = new DefaultPreloadManager.Builder(context, mockTargetPreloadStatusControl) .setRenderersFactory(renderersFactory) - .setPreloadLooper(Util.getCurrentOrMainLooper()) + .setPreloadLooper(preloadThread.getLooper()) .build(); MediaItem.Builder mediaItemBuilder = new MediaItem.Builder(); MediaItem mediaItem1 = @@ -148,7 +157,7 @@ public class DefaultPreloadManagerTest { DefaultPreloadManager preloadManager = new DefaultPreloadManager.Builder(context, mockTargetPreloadStatusControl) .setRenderersFactory(renderersFactory) - .setPreloadLooper(Util.getCurrentOrMainLooper()) + .setPreloadLooper(preloadThread.getLooper()) .build(); MediaItem mediaItem = new MediaItem.Builder() @@ -178,8 +187,6 @@ public class DefaultPreloadManagerTest { ProgressiveMediaSource.Factory mediaSourceFactory = new ProgressiveMediaSource.Factory( new DefaultDataSource.Factory(ApplicationProvider.getApplicationContext())); - HandlerThread preloadThread = new HandlerThread("preload"); - preloadThread.start(); DefaultPreloadManager preloadManager = new DefaultPreloadManager.Builder(context, targetPreloadStatusControl) .setMediaSourceFactory(mediaSourceFactory) @@ -216,8 +223,6 @@ public class DefaultPreloadManagerTest { assertThat(preloadManagerListener.onCompletedMediaItemRecords) .containsExactly(mediaItem0, mediaItem1, mediaItem2) .inOrder(); - - preloadThread.quit(); } @Test @@ -595,7 +600,7 @@ public class DefaultPreloadManagerTest { new DefaultPreloadManager.Builder(context, targetPreloadStatusControl) .setMediaSourceFactory(mockMediaSourceFactory) .setRenderersFactory(renderersFactory) - .setPreloadLooper(Util.getCurrentOrMainLooper()) + .setPreloadLooper(preloadThread.getLooper()) .build(); MediaItem.Builder mediaItemBuilder = new MediaItem.Builder(); MediaItem mediaItem0 = @@ -667,7 +672,7 @@ public class DefaultPreloadManagerTest { new DefaultPreloadManager.Builder(context, targetPreloadStatusControl) .setMediaSourceFactory(mockMediaSourceFactory) .setRenderersFactory(renderersFactory) - .setPreloadLooper(Util.getCurrentOrMainLooper()) + .setPreloadLooper(preloadThread.getLooper()) .build(); MediaItem.Builder mediaItemBuilder = new MediaItem.Builder(); MediaItem mediaItem1 = @@ -694,11 +699,11 @@ public class DefaultPreloadManagerTest { }); preloadManager.add(mediaItem1, /* rankingData= */ 1); preloadManager.invalidate(); - shadowOf(Looper.getMainLooper()).idle(); + shadowOf(preloadThread.getLooper()).idle(); boolean mediaItem1Removed = preloadManager.remove(mediaItem1); boolean mediaItem2Removed = preloadManager.remove(mediaItem2); - shadowOf(Looper.getMainLooper()).idle(); + shadowOf(preloadThread.getLooper()).idle(); assertThat(mediaItem1Removed).isTrue(); assertThat(mediaItem2Removed).isFalse(); @@ -715,7 +720,7 @@ public class DefaultPreloadManagerTest { new DefaultPreloadManager.Builder(context, targetPreloadStatusControl) .setMediaSourceFactory(mockMediaSourceFactory) .setRenderersFactory(renderersFactory) - .setPreloadLooper(Util.getCurrentOrMainLooper()) + .setPreloadLooper(preloadThread.getLooper()) .build(); MediaItem.Builder mediaItemBuilder = new MediaItem.Builder(); MediaItem mediaItem1 = @@ -742,7 +747,7 @@ public class DefaultPreloadManagerTest { }); preloadManager.add(mediaItem1, /* rankingData= */ 1); preloadManager.invalidate(); - shadowOf(Looper.getMainLooper()).idle(); + shadowOf(preloadThread.getLooper()).idle(); MediaSource mediaSource1 = preloadManager.getMediaSource(mediaItem1); DefaultMediaSourceFactory defaultMediaSourceFactory = new DefaultMediaSourceFactory((Context) ApplicationProvider.getApplicationContext()); @@ -752,7 +757,7 @@ public class DefaultPreloadManagerTest { boolean mediaSource1Removed = preloadManager.remove(mediaSource1); boolean mediaSource2Removed = preloadManager.remove(mediaSource2); boolean mediaSource3Removed = preloadManager.remove(mediaSource3); - shadowOf(Looper.getMainLooper()).idle(); + shadowOf(preloadThread.getLooper()).idle(); assertThat(mediaSource1Removed).isTrue(); assertThat(mediaSource2Removed).isFalse(); @@ -762,7 +767,8 @@ public class DefaultPreloadManagerTest { } @Test - public void reset_returnZeroCount_sourcesButNotRendererCapabilitiesListReleased() { + public void reset_returnZeroCount_sourcesButNotRendererCapabilitiesListReleased() + throws TimeoutException { TargetPreloadStatusControl targetPreloadStatusControl = rankingData -> new DefaultPreloadManager.Status(STAGE_SOURCE_PREPARED); MediaSource.Factory mockMediaSourceFactory = mock(MediaSource.Factory.class); @@ -789,7 +795,7 @@ public class DefaultPreloadManagerTest { new DefaultPreloadManager.Builder(context, targetPreloadStatusControl) .setMediaSourceFactory(mockMediaSourceFactory) .setRenderersFactory(renderersFactory) - .setPreloadLooper(Util.getCurrentOrMainLooper()) + .setPreloadLooper(preloadThread.getLooper()) .build(); MediaItem.Builder mediaItemBuilder = new MediaItem.Builder(); MediaItem mediaItem1 = @@ -817,10 +823,11 @@ public class DefaultPreloadManagerTest { preloadManager.add(mediaItem1, /* rankingData= */ 1); preloadManager.add(mediaItem2, /* rankingData= */ 2); preloadManager.invalidate(); - shadowOf(Looper.getMainLooper()).idle(); + shadowOf(preloadThread.getLooper()).idle(); + shadowOf(Util.getCurrentOrMainLooper()).idle(); preloadManager.reset(); - shadowOf(Looper.getMainLooper()).idle(); + shadowOf(preloadThread.getLooper()).idle(); assertThat(preloadManager.getSourceCount()).isEqualTo(0); assertThat(internalSourceToReleaseReferenceByMediaId).containsExactly("mediaId1", "mediaId2"); @@ -888,7 +895,7 @@ public class DefaultPreloadManagerTest { preloadManager.add(mediaItem2, /* rankingData= */ 2); preloadManager.invalidate(); shadowOf(preloadThread.getLooper()).idle(); - shadowOf(Looper.getMainLooper()).idle(); + shadowOf(Util.getCurrentOrMainLooper()).idle(); preloadManager.release(); shadowOf(preloadThread.getLooper()).idle();