loliball/media3
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix the flakiness in DefaultPreloadManagerTest

Browse Source 
We began to use a different preload thread than the main thread in the `DefaultPreloadManagerTest`, then in the test execution, we should also ensure that the events queued on the preload looper have been executed.

Also, there is an edge case also causing flakiness. Assume that `DefaultPreloadManager` is preloading source B, then the app invalidates and triggers another sequence of preloading, say they are A, B and C. There is possibility that source B finishes preloading (started before `invalidate`) when A starting to preload, then `onPreloadSkipped` will be triggered for source B instead of `onPreloadCompleted`. However, the functional block inside of `onPreloadSkipped` is dispatched asynchronously, and by then it's possible that B starts to preload again at the consequence of A being completed, then the functional block just mentioned may think that the current source matches at B, and will advance the current preloading source to C, without informing the `Listener.onPreloadCompleted` for it. As the result, the `Listener.onPreloadCompleted` can never be triggered for B for the second sequence. To fix this, we should prevent the functional block in `onPreloadCompleted`, `onPreloadError`, `onPreloadSkipped` to be even dispatched, when the source doesn't match the current preloading one.

PiperOrigin-RevId: 679145353
This commit is contained in:
tianyifeng 2024-09-26 07:44:47 -07:00 committed by Copybara-Service
parent 45c400c7b5
commit 2dde824bde
2 changed files with 32 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/preload/BasePreloadManager.java
View File

@ -256,6 +256,11 @@ public abstract class BasePreloadManager<T> {
/** Called when the given {@link MediaSource} completes preloading. */
protected final void onPreloadCompleted(MediaSource source) {
synchronized (lock) {
if (!isPreloading(source)) {
return;
}
}
applicationHandler.post(
() -> {
listeners.sendEvent(
@ -267,6 +272,11 @@ public abstract class BasePreloadManager<T> {
/** Called when an error occurs. */
protected final void onPreloadError(PreloadException error, MediaSource source) {
synchronized (lock) {
if (!isPreloading(source)) {
return;
}
}
applicationHandler.post(
() -> {
listeners.sendEvent(/* eventFlag= */ C.INDEX_UNSET, listener -> listener.onError(error));
@ -276,13 +286,17 @@ public abstract class BasePreloadManager<T> {
/** Called when the given {@link MediaSource} has been skipped before completing preloading. */
protected final void onPreloadSkipped(MediaSource source) {
synchronized (lock) {
if (!isPreloading(source)) {
return;
}
}
applicationHandler.post(() -> maybeAdvanceToNextSource(source));
}
private void maybeAdvanceToNextSource(MediaSource preloadingSource) {
private void maybeAdvanceToNextSource(MediaSource currentSource) {
synchronized (lock) {
if (sourceHolderPriorityQueue.isEmpty()
|| checkNotNull(sourceHolderPriorityQueue.peek()).mediaSource != preloadingSource) {
if (!isPreloading(currentSource)) {
return;
}
do {
@ -291,6 +305,13 @@ public abstract class BasePreloadManager<T> {
}
}
/** Returns whether the {@link MediaSource} is currently preloading. */
@GuardedBy("lock")
private boolean isPreloading(MediaSource mediaSource) {
return !sourceHolderPriorityQueue.isEmpty()
&& checkNotNull(sourceHolderPriorityQueue.peek()).mediaSource == mediaSource;
}
/**
* Returns the {@linkplain TargetPreloadStatusControl.PreloadStatus target preload status} of the
* given {@link MediaSource}.
@ -299,8 +320,7 @@ public abstract class BasePreloadManager<T> {
protected final TargetPreloadStatusControl.PreloadStatus getTargetPreloadStatus(
MediaSource source) {
synchronized (lock) {
if (sourceHolderPriorityQueue.isEmpty()
|| checkNotNull(sourceHolderPriorityQueue.peek()).mediaSource != source) {
if (!isPreloading(source)) {
return null;
}
return targetPreloadStatusOfCurrentPreloadingSource;

7
libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/preload/DefaultPreloadManagerTest.java
View File

@ -237,6 +237,7 @@ public class DefaultPreloadManagerTest {
preloadManager.add(mediaItem2, /* rankingData= */ 2);
preloadManager.invalidate();
shadowOf(preloadThread.getLooper()).idle();
runMainLooperUntil(() -> preloadManagerListener.onCompletedMediaItemRecords.size() == 3);
assertThat(targetPreloadStatusControlCallStates).containsExactly(0, 1, 2).inOrder();
@ -300,6 +301,7 @@ public class DefaultPreloadManagerTest {
currentPlayingItemIndex.set(2);
preloadManager.invalidate();
shadowOf(preloadThread.getLooper()).idle();
runMainLooperUntil(() -> preloadManagerListener.onCompletedMediaItemRecords.size() == 3);
assertThat(targetPreloadStatusControlCallStates).containsExactly(2, 1, 0).inOrder();
@ -353,6 +355,7 @@ public class DefaultPreloadManagerTest {
(source, timeline) -> {}, bandwidthMeter.getTransferListener(), PlayerId.UNSET);
wrappedMediaSource0.setAllowPreparation(true);
wrappedMediaSource1.setAllowPreparation(true);
shadowOf(preloadThread.getLooper()).idle();
runMainLooperUntil(() -> preloadManagerListener.onCompletedMediaItemRecords.size() == 1);
assertThat(targetPreloadStatusControlCallStates).containsExactly(0, 1).inOrder();
@ -404,6 +407,7 @@ public class DefaultPreloadManagerTest {
preloadManager.invalidate();
wrappedMediaSource0.setAllowPreparation(true);
shadowOf(preloadThread.getLooper()).idle();
runMainLooperUntil(() -> preloadManagerListener.onCompletedMediaItemRecords.size() == 1);
assertThat(targetPreloadStatusControlCallStates).containsExactly(0, 1).inOrder();
assertThat(preloadManagerListener.onCompletedMediaItemRecords).containsExactly(mediaItem0);
@ -420,6 +424,7 @@ public class DefaultPreloadManagerTest {
// preloadManagerListener.onCompletedMediaItemRecords.
wrappedMediaSource1.setAllowPreparation(true);
wrappedMediaSource2.setAllowPreparation(true);
shadowOf(preloadThread.getLooper()).idle();
runMainLooperUntil(() -> preloadManagerListener.onCompletedMediaItemRecords.size() == 3);
assertThat(targetPreloadStatusControlCallStates).containsExactly(2, 1, 0).inOrder();
assertThat(preloadManagerListener.onCompletedMediaItemRecords)
@ -471,6 +476,7 @@ public class DefaultPreloadManagerTest {
preloadManager.add(mediaItem1, /* rankingData= */ 1);
preloadManager.invalidate();
shadowOf(preloadThread.getLooper()).idle();
runMainLooperUntil(() -> preloadManagerListener.onCompletedMediaItemRecords.size() == 1);
assertThat(targetPreloadStatusControlCallStates).containsExactly(0, 1);
@ -557,6 +563,7 @@ public class DefaultPreloadManagerTest {
preloadManager.add(mediaItem1, /* rankingData= */ 1);
preloadManager.invalidate();
shadowOf(preloadThread.getLooper()).idle();
runMainLooperUntil(() -> preloadManagerListener.onCompletedMediaItemRecords.size() == 1);
assertThat(targetPreloadStatusControlCallStates).containsExactly(0, 1).inOrder();