diff --git a/RELEASENOTES.md b/RELEASENOTES.md index e5d9d9a4b4..87f187075f 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -2,6 +2,11 @@ ### Unreleased changes +* Core library: + * Ensure that changing the `ShuffleOrder` with `ExoPlayer.setShuffleOrder` + results in a call to `Player.Listener#onTimelineChanged` with + `reason=Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED` + ([#9889](https://github.com/google/ExoPlayer/issues/9889)). * Extractors: * Add support for AVI ([#2092](https://github.com/google/ExoPlayer/issues/2092)). diff --git a/libraries/common/build.gradle b/libraries/common/build.gradle index 048fe60f41..85169e2ec8 100644 --- a/libraries/common/build.gradle +++ b/libraries/common/build.gradle @@ -75,6 +75,7 @@ dependencies { testImplementation 'junit:junit:' + junitVersion testImplementation 'com.google.truth:truth:' + truthVersion testImplementation 'org.robolectric:robolectric:' + robolectricVersion + testImplementation project(modulePrefix + 'lib-exoplayer') testImplementation project(modulePrefix + 'test-utils') } diff --git a/libraries/common/src/main/java/androidx/media3/common/Timeline.java b/libraries/common/src/main/java/androidx/media3/common/Timeline.java index 3f665f8bfc..14b04065f5 100644 --- a/libraries/common/src/main/java/androidx/media3/common/Timeline.java +++ b/libraries/common/src/main/java/androidx/media3/common/Timeline.java @@ -1351,6 +1351,27 @@ public boolean equals(@Nullable Object obj) { return false; } } + + // Check shuffled order + int windowIndex = getFirstWindowIndex(/* shuffleModeEnabled= */ true); + if (windowIndex != other.getFirstWindowIndex(/* shuffleModeEnabled= */ true)) { + return false; + } + int lastWindowIndex = getLastWindowIndex(/* shuffleModeEnabled= */ true); + if (lastWindowIndex != other.getLastWindowIndex(/* shuffleModeEnabled= */ true)) { + return false; + } + while (windowIndex != lastWindowIndex) { + int nextWindowIndex = + getNextWindowIndex(windowIndex, Player.REPEAT_MODE_OFF, /* shuffleModeEnabled= */ true); + if (nextWindowIndex + != other.getNextWindowIndex( + windowIndex, Player.REPEAT_MODE_OFF, /* shuffleModeEnabled= */ true)) { + return false; + } + windowIndex = nextWindowIndex; + } + return true; } @@ -1367,6 +1388,13 @@ public int hashCode() { for (int i = 0; i < getPeriodCount(); i++) { result = 31 * result + getPeriod(i, period, /* setIds= */ true).hashCode(); } + + for (int windowIndex = getFirstWindowIndex(true); + windowIndex != C.INDEX_UNSET; + windowIndex = getNextWindowIndex(windowIndex, Player.REPEAT_MODE_OFF, true)) { + result = 31 * result + windowIndex; + } + return result; } diff --git a/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java b/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java index f2de751193..6844330e14 100644 --- a/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java @@ -19,6 +19,7 @@ import androidx.annotation.Nullable; import androidx.media3.common.MediaItem.LiveConfiguration; +import androidx.media3.exoplayer.source.ShuffleOrder.DefaultShuffleOrder; import androidx.media3.test.utils.FakeTimeline; import androidx.media3.test.utils.FakeTimeline.TimelineWindowDefinition; import androidx.media3.test.utils.TimelineAsserts; @@ -64,6 +65,50 @@ public void multiPeriodTimeline() { TimelineAsserts.assertNextWindowIndices(timeline, Player.REPEAT_MODE_ALL, false, 0); } + @Test + public void timelineEquals() { + ImmutableList timelineWindowDefinitions = + ImmutableList.of( + new TimelineWindowDefinition(/* periodCount= */ 1, /* id= */ 111), + new TimelineWindowDefinition(/* periodCount= */ 2, /* id= */ 222), + new TimelineWindowDefinition(/* periodCount= */ 3, /* id= */ 333)); + Timeline timeline1 = + new FakeTimeline(timelineWindowDefinitions.toArray(new TimelineWindowDefinition[0])); + Timeline timeline2 = + new FakeTimeline(timelineWindowDefinitions.toArray(new TimelineWindowDefinition[0])); + + assertThat(timeline1).isEqualTo(timeline2); + assertThat(timeline1.hashCode()).isEqualTo(timeline2.hashCode()); + } + + @Test + public void timelineEquals_includesShuffleOrder() { + ImmutableList timelineWindowDefinitions = + ImmutableList.of( + new TimelineWindowDefinition(/* periodCount= */ 1, /* id= */ 111), + new TimelineWindowDefinition(/* periodCount= */ 2, /* id= */ 222), + new TimelineWindowDefinition(/* periodCount= */ 3, /* id= */ 333)); + Timeline timeline = + new FakeTimeline( + new Object[0], + new DefaultShuffleOrder(timelineWindowDefinitions.size(), /* randomSeed= */ 5), + timelineWindowDefinitions.toArray(new TimelineWindowDefinition[0])); + Timeline timelineWithEquivalentShuffleOrder = + new FakeTimeline( + new Object[0], + new DefaultShuffleOrder(timelineWindowDefinitions.size(), /* randomSeed= */ 5), + timelineWindowDefinitions.toArray(new TimelineWindowDefinition[0])); + Timeline timelineWithDifferentShuffleOrder = + new FakeTimeline( + new Object[0], + new DefaultShuffleOrder(timelineWindowDefinitions.size(), /* randomSeed= */ 3), + timelineWindowDefinitions.toArray(new TimelineWindowDefinition[0])); + + assertThat(timeline).isEqualTo(timelineWithEquivalentShuffleOrder); + assertThat(timeline.hashCode()).isEqualTo(timelineWithEquivalentShuffleOrder.hashCode()); + assertThat(timeline).isNotEqualTo(timelineWithDifferentShuffleOrder); + } + @Test public void windowEquals() { MediaItem mediaItem = new MediaItem.Builder().setUri("uri").setTag(new Object()).build(); diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java index de4f574764..5bd1058641 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java @@ -710,6 +710,7 @@ public void moveMediaItems(int fromIndex, int toIndex, int newFromIndex) { @Override public void setShuffleOrder(ShuffleOrder shuffleOrder) { verifyApplicationThread(); + this.shuffleOrder = shuffleOrder; Timeline timeline = createMaskingTimeline(); PlaybackInfo newPlaybackInfo = maskTimelineAndPosition( @@ -718,7 +719,6 @@ public void setShuffleOrder(ShuffleOrder shuffleOrder) { maskWindowPositionMsOrGetPeriodPositionUs( timeline, getCurrentMediaItemIndex(), getCurrentPosition())); pendingOperationAcks++; - this.shuffleOrder = shuffleOrder; internalPlayer.setShuffleOrder(shuffleOrder); updatePlaybackInfo( newPlaybackInfo, diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java index dbbb47fc69..1e0b0338ba 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -125,6 +125,7 @@ import androidx.media3.exoplayer.source.MediaSource; import androidx.media3.exoplayer.source.MediaSource.MediaPeriodId; import androidx.media3.exoplayer.source.MediaSourceEventListener; +import androidx.media3.exoplayer.source.ShuffleOrder; import androidx.media3.exoplayer.source.SinglePeriodTimeline; import androidx.media3.exoplayer.source.TrackGroupArray; import androidx.media3.exoplayer.source.ads.ServerSideAdInsertionMediaSource; @@ -6517,6 +6518,53 @@ public void run(ExoPlayer player) { assertThat(positionAfterSetShuffleOrder.get()).isAtLeast(5000); } + @Test + public void setShuffleOrder_notifiesTimelineChanged() throws Exception { + ExoPlayer player = + new TestExoPlayerBuilder(context) + .setClock(new FakeClock(/* isAutoAdvancing= */ true)) + .build(); + // No callback expected for this call, because the (empty) timeline doesn't change. We start + // with a deterministic shuffle order, to ensure when we call setShuffleOrder again below the + // order is definitely different (otherwise the test is flaky when the existing shuffle order + // matches the shuffle order passed in below). + player.setShuffleOrder(new FakeShuffleOrder(0)); + player.setMediaSources( + ImmutableList.of(new FakeMediaSource(), new FakeMediaSource(), new FakeMediaSource())); + Player.Listener mockListener = mock(Player.Listener.class); + player.addListener(mockListener); + player.prepare(); + TestPlayerRunHelper.playUntilPosition(player, /* mediaItemIndex= */ 0, /* positionMs= */ 5000); + player.play(); + ShuffleOrder.DefaultShuffleOrder newShuffleOrder = + new ShuffleOrder.DefaultShuffleOrder(player.getMediaItemCount(), /* randomSeed= */ 5); + player.setShuffleOrder(newShuffleOrder); + TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED); + player.release(); + + ArgumentCaptor timelineCaptor = ArgumentCaptor.forClass(Timeline.class); + verify(mockListener) + .onTimelineChanged( + timelineCaptor.capture(), eq(Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); + + Timeline capturedTimeline = Iterables.getOnlyElement(timelineCaptor.getAllValues()); + List newShuffleOrderIndexes = new ArrayList<>(newShuffleOrder.getLength()); + for (int i = newShuffleOrder.getFirstIndex(); + i != C.INDEX_UNSET; + i = newShuffleOrder.getNextIndex(i)) { + newShuffleOrderIndexes.add(i); + } + List capturedTimelineShuffleIndexes = new ArrayList<>(newShuffleOrder.getLength()); + for (int i = capturedTimeline.getFirstWindowIndex(/* shuffleModeEnabled= */ true); + i != C.INDEX_UNSET; + i = + capturedTimeline.getNextWindowIndex( + i, Player.REPEAT_MODE_OFF, /* shuffleModeEnabled= */ true)) { + capturedTimelineShuffleIndexes.add(i); + } + assertThat(capturedTimelineShuffleIndexes).isEqualTo(newShuffleOrderIndexes); + } + @Test public void setMediaSources_empty_whenEmpty_correctMaskingMediaItemIndex() throws Exception { final int[] currentMediaItemIndices = {C.INDEX_UNSET, C.INDEX_UNSET, C.INDEX_UNSET}; diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeTimeline.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeTimeline.java index 75e3d26e7b..9aad788a7a 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeTimeline.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeTimeline.java @@ -29,6 +29,7 @@ import androidx.media3.common.util.Assertions; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; +import androidx.media3.exoplayer.source.ShuffleOrder; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import java.util.ArrayList; @@ -275,7 +276,7 @@ public TimelineWindowDefinition( private final TimelineWindowDefinition[] windowDefinitions; private final Object[] manifests; private final int[] periodOffsets; - private final FakeShuffleOrder fakeShuffleOrder; + private final ShuffleOrder shuffleOrder; /** * Returns an ad playback state with the specified number of ads in each of the specified ad @@ -395,6 +396,19 @@ public FakeTimeline(TimelineWindowDefinition... windowDefinitions) { * @param windowDefinitions A list of {@link TimelineWindowDefinition}s. */ public FakeTimeline(Object[] manifests, TimelineWindowDefinition... windowDefinitions) { + this(manifests, new FakeShuffleOrder(windowDefinitions.length), windowDefinitions); + } + + /** + * Creates a fake timeline with the given window definitions and {@link + * androidx.media3.exoplayer.source.ShuffleOrder}. + * + * @param windowDefinitions A list of {@link TimelineWindowDefinition}s. + */ + public FakeTimeline( + Object[] manifests, + ShuffleOrder shuffleOrder, + TimelineWindowDefinition... windowDefinitions) { this.manifests = new Object[windowDefinitions.length]; System.arraycopy(manifests, 0, this.manifests, 0, min(this.manifests.length, manifests.length)); this.windowDefinitions = windowDefinitions; @@ -403,7 +417,7 @@ public FakeTimeline(Object[] manifests, TimelineWindowDefinition... windowDefini for (int i = 0; i < windowDefinitions.length; i++) { periodOffsets[i + 1] = periodOffsets[i] + windowDefinitions[i].periodCount; } - fakeShuffleOrder = new FakeShuffleOrder(windowDefinitions.length); + this.shuffleOrder = shuffleOrder; } @Override @@ -422,7 +436,7 @@ public int getNextWindowIndex( ? getFirstWindowIndex(shuffleModeEnabled) : C.INDEX_UNSET; } - return shuffleModeEnabled ? fakeShuffleOrder.getNextIndex(windowIndex) : windowIndex + 1; + return shuffleModeEnabled ? shuffleOrder.getNextIndex(windowIndex) : windowIndex + 1; } @Override @@ -436,20 +450,20 @@ public int getPreviousWindowIndex( ? getLastWindowIndex(shuffleModeEnabled) : C.INDEX_UNSET; } - return shuffleModeEnabled ? fakeShuffleOrder.getPreviousIndex(windowIndex) : windowIndex - 1; + return shuffleModeEnabled ? shuffleOrder.getPreviousIndex(windowIndex) : windowIndex - 1; } @Override public int getLastWindowIndex(boolean shuffleModeEnabled) { return shuffleModeEnabled - ? fakeShuffleOrder.getLastIndex() + ? shuffleOrder.getLastIndex() : super.getLastWindowIndex(/* shuffleModeEnabled= */ false); } @Override public int getFirstWindowIndex(boolean shuffleModeEnabled) { return shuffleModeEnabled - ? fakeShuffleOrder.getFirstIndex() + ? shuffleOrder.getFirstIndex() : super.getFirstWindowIndex(/* shuffleModeEnabled= */ false); } diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java index f6ac5ecb99..27872827fa 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/TestUtil.java @@ -200,8 +200,12 @@ public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) { /** * Asserts that the actual timelines are the same to the expected timelines. This assert differs - * from testing equality by not comparing period ids which may be different due to id mapping of - * child source period ids. + * from testing equality by not comparing: + * + *
    + *
  • Period IDs, which may be different due to ID mapping of child source period IDs. + *
  • Shuffle order, which by default is random and non-deterministic. + *
* * @param actualTimelines A list of actual {@link Timeline timelines}. * @param expectedTimelines A list of expected {@link Timeline timelines}. @@ -218,10 +222,11 @@ public static void assertTimelinesSame( /** * Returns true if {@code thisTimeline} is equal to {@code thatTimeline}, ignoring {@link - * Timeline.Window#uid} and {@link Timeline.Period#uid} values. + * Timeline.Window#uid} and {@link Timeline.Period#uid} values, and shuffle order. */ public static boolean timelinesAreSame(Timeline thisTimeline, Timeline thatTimeline) { - return new NoUidTimeline(thisTimeline).equals(new NoUidTimeline(thatTimeline)); + return new NoUidOrShufflingTimeline(thisTimeline) + .equals(new NoUidOrShufflingTimeline(thatTimeline)); } /** @@ -505,11 +510,11 @@ public static List getPublicMethods(Class clazz) { return list; } - private static final class NoUidTimeline extends Timeline { + private static final class NoUidOrShufflingTimeline extends Timeline { private final Timeline delegate; - public NoUidTimeline(Timeline timeline) { + public NoUidOrShufflingTimeline(Timeline timeline) { this.delegate = timeline; } @@ -518,6 +523,27 @@ public int getWindowCount() { return delegate.getWindowCount(); } + @Override + public int getNextWindowIndex(int windowIndex, int repeatMode, boolean shuffleModeEnabled) { + return delegate.getNextWindowIndex(windowIndex, repeatMode, /* shuffleModeEnabled= */ false); + } + + @Override + public int getPreviousWindowIndex(int windowIndex, int repeatMode, boolean shuffleModeEnabled) { + return delegate.getPreviousWindowIndex( + windowIndex, repeatMode, /* shuffleModeEnabled= */ false); + } + + @Override + public int getLastWindowIndex(boolean shuffleModeEnabled) { + return delegate.getLastWindowIndex(/* shuffleModeEnabled= */ false); + } + + @Override + public int getFirstWindowIndex(boolean shuffleModeEnabled) { + return delegate.getFirstWindowIndex(/* shuffleModeEnabled= */ false); + } + @Override public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) { delegate.getWindow(windowIndex, window, defaultPositionProjectionUs);