Skip to content

Commit

Permalink
Deflake the DefaultPreloadManagerTest
Browse files Browse the repository at this point in the history
From [ the last change in `DefaultPreloadManagerTest`](2b54b1e), 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 0143884)
  • Loading branch information
tianyif authored and icbaker committed Nov 19, 2024
1 parent fd02ee1 commit 737fdd8
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ public PreloadMediaSource createMediaSource(MediaSource mediaSource) {
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;
Expand All @@ -246,6 +247,7 @@ private PreloadMediaSource(
this.allocator = allocator;

preloadHandler = Util.createHandler(preloadLooper, /* callback= */ null);
releaseHandler = Util.createHandler(preloadLooper, /* callback= */ null);
startPositionUs = C.TIME_UNSET;
}

Expand Down Expand Up @@ -396,7 +398,7 @@ protected void releaseSourceInternal() {
* <p>Can be called from any thread.
*/
public void releasePreloadMediaSource() {
preloadHandler.post(
releaseHandler.post(
() -> {
preloadCalled = false;
startPositionUs = C.TIME_UNSET;
Expand All @@ -407,6 +409,7 @@ public void releasePreloadMediaSource() {
}
releaseSourceInternal();
preloadHandler.removeCallbacksAndMessages(null);
releaseHandler.removeCallbacksAndMessages(null);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -830,8 +830,7 @@ protected void releaseSourceInternal() {
}

@Test
public void release_returnZeroCount_sourcesAndRendererCapabilitiesListReleased()
throws Exception {
public void release_returnZeroCount_sourcesAndRendererCapabilitiesListReleased() {
TargetPreloadStatusControl<Integer> targetPreloadStatusControl =
rankingData -> new DefaultPreloadManager.Status(STAGE_SOURCE_PREPARED);
MediaSource.Factory mockMediaSourceFactory = mock(MediaSource.Factory.class);
Expand Down

0 comments on commit 737fdd8

Please sign in to comment.