From 737fdd8693a4dbe56f629e40e4eddfa5c016bbc7 Mon Sep 17 00:00:00 2001 From: tianyifeng Date: Fri, 15 Nov 2024 08:20:00 -0800 Subject: [PATCH] Deflake the DefaultPreloadManagerTest From [ the last change in `DefaultPreloadManagerTest`](https://github.com/androidx/media/commit/2b54b1ebbeb6f13abb6944b7ff593c168c927192), the preloadManager began to use a separate `preloadThread` in `release_returnZeroCount_sourcesAndRendererCapabilitiesListReleased`, which unveils a bug in `PreloadMediaSource`. When `PreloadMediaSource.releasePreloadMediaSource` is called, `preloadHandler` will post a `Runnable` on the preload looper to release the internal resources. Before this `Runnable` is executed, it is possible that the [`stopPreloading`](https://github.com/androidx/media/blob/main/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/preload/PreloadMediaSource.java#L442) method is executed just as the result of preloading has completed. This is expected to remove the posted `Runnable`s for further preloading, however, the posted `Runnable` for releasing will also be removed from the message queue. Ideally we should use `postDelayed(runnable, token, delayMillis)` to post the runnables so that the token will be useful to identify which messages to remove in `removeCallbacksAndMessages(token)`, but that `postDelayed` method is only available from API 28. So in this change we are using a separate handler for releasing, and then the call of `preloadHandler.removeCallbacksAndMessages` won't impact the runnable for releasing. #cherrypick PiperOrigin-RevId: 696894483 (cherry picked from commit 0143884cd7ac06b042532e328ea69c5ac7da2cb3) --- .../media3/exoplayer/source/preload/PreloadMediaSource.java | 5 ++++- .../exoplayer/source/preload/DefaultPreloadManagerTest.java | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/preload/PreloadMediaSource.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/preload/PreloadMediaSource.java index d2a313672e..fc3d3870ca 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/preload/PreloadMediaSource.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/preload/PreloadMediaSource.java @@ -221,6 +221,7 @@ public final class PreloadMediaSource extends WrappingMediaSource { private final RendererCapabilities[] rendererCapabilities; private final Allocator allocator; private final Handler preloadHandler; + private final Handler releaseHandler; private boolean preloadCalled; private boolean prepareChildSourceCalled; private long startPositionUs; @@ -246,6 +247,7 @@ public final class PreloadMediaSource extends WrappingMediaSource { this.allocator = allocator; preloadHandler = Util.createHandler(preloadLooper, /* callback= */ null); + releaseHandler = Util.createHandler(preloadLooper, /* callback= */ null); startPositionUs = C.TIME_UNSET; } @@ -396,7 +398,7 @@ public final class PreloadMediaSource extends WrappingMediaSource { *

Can be called from any thread. */ public void releasePreloadMediaSource() { - preloadHandler.post( + releaseHandler.post( () -> { preloadCalled = false; startPositionUs = C.TIME_UNSET; @@ -407,6 +409,7 @@ public final class PreloadMediaSource extends WrappingMediaSource { } releaseSourceInternal(); preloadHandler.removeCallbacksAndMessages(null); + releaseHandler.removeCallbacksAndMessages(null); }); } 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 2f351868ea..c4975c9eed 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 @@ -830,8 +830,7 @@ public class DefaultPreloadManagerTest { } @Test - public void release_returnZeroCount_sourcesAndRendererCapabilitiesListReleased() - throws Exception { + public void release_returnZeroCount_sourcesAndRendererCapabilitiesListReleased() { TargetPreloadStatusControl targetPreloadStatusControl = rankingData -> new DefaultPreloadManager.Status(STAGE_SOURCE_PREPARED); MediaSource.Factory mockMediaSourceFactory = mock(MediaSource.Factory.class);