From 852c3a53e70ed2e007512b96745447c559eeda12 Mon Sep 17 00:00:00 2001 From: bachinger Date: Fri, 20 Sep 2019 09:45:17 +0100 Subject: [PATCH] copy masked timeline before notifying listeners PiperOrigin-RevId: 270221478 --- .../android/exoplayer2/ExoPlayerImpl.java | 33 +++++++------------ .../android/exoplayer2/ExoPlayerTest.java | 31 +++++++++++++++++ 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index 0401f7d595..b9f29e2cb3 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -308,13 +308,11 @@ import java.util.concurrent.CopyOnWriteArrayList; /* fromIndex= */ 0, /* toIndexExclusive= */ mediaSourceHolders.size()); } List holders = addMediaSourceHolders(/* index= */ 0, mediaItems); - maskTimeline(); + Timeline timeline = maskTimeline(); internalPlayer.setMediaItems( holders, startWindowIndex, C.msToUs(startPositionMs), shuffleOrder); notifyListeners( - listener -> - listener.onTimelineChanged( - playbackInfo.timeline, TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); + listener -> listener.onTimelineChanged(timeline, TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); } @Override @@ -337,12 +335,10 @@ import java.util.concurrent.CopyOnWriteArrayList; Assertions.checkArgument(index >= 0); pendingOperationAcks++; List holders = addMediaSourceHolders(index, mediaSources); - maskTimeline(); + Timeline timeline = maskTimeline(); internalPlayer.addMediaItems(index, holders, shuffleOrder); notifyListeners( - listener -> - listener.onTimelineChanged( - playbackInfo.timeline, TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); + listener -> listener.onTimelineChanged(timeline, TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); } @Override @@ -374,12 +370,10 @@ import java.util.concurrent.CopyOnWriteArrayList; pendingOperationAcks++; newFromIndex = Math.min(newFromIndex, mediaSourceHolders.size() - (toIndex - fromIndex)); Playlist.moveMediaSourceHolders(mediaSourceHolders, fromIndex, toIndex, newFromIndex); - maskTimeline(); + Timeline timeline = maskTimeline(); internalPlayer.moveMediaItems(fromIndex, toIndex, newFromIndex, shuffleOrder); notifyListeners( - listener -> - listener.onTimelineChanged( - playbackInfo.timeline, TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); + listener -> listener.onTimelineChanged(timeline, TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); } @Override @@ -394,12 +388,10 @@ import java.util.concurrent.CopyOnWriteArrayList; public void setShuffleOrder(ShuffleOrder shuffleOrder) { pendingOperationAcks++; this.shuffleOrder = shuffleOrder; - maskTimeline(); + Timeline timeline = maskTimeline(); internalPlayer.setShuffleOrder(shuffleOrder); notifyListeners( - listener -> - listener.onTimelineChanged( - playbackInfo.timeline, TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); + listener -> listener.onTimelineChanged(timeline, TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); } @Override @@ -879,12 +871,10 @@ import java.util.concurrent.CopyOnWriteArrayList; pendingOperationAcks++; List mediaSourceHolders = removeMediaSourceHolders(fromIndex, /* toIndexExclusive= */ toIndex); - maskTimeline(); + Timeline timeline = maskTimeline(); internalPlayer.removeMediaItems(fromIndex, toIndex, shuffleOrder); notifyListeners( - listener -> - listener.onTimelineChanged( - playbackInfo.timeline, TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); + listener -> listener.onTimelineChanged(timeline, TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED)); return mediaSourceHolders; } @@ -898,12 +888,13 @@ import java.util.concurrent.CopyOnWriteArrayList; return removed; } - private void maskTimeline() { + private Timeline maskTimeline() { playbackInfo = playbackInfo.copyWithTimeline( mediaSourceHolders.isEmpty() ? Timeline.EMPTY : new Playlist.PlaylistTimeline(mediaSourceHolders, shuffleOrder)); + return playbackInfo.timeline; } private void notifyListeners(ListenerInvocation listenerInvocation) { diff --git a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java index 8fc5f90844..21a6ac2818 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -3096,6 +3096,37 @@ public final class ExoPlayerTest { Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED /* playlist cleared */); } + @Test + public void testMultipleModificationWithRecursiveListenerInvocations() throws Exception { + Timeline timeline = new FakeTimeline(/* windowCount= */ 1); + MediaSource mediaSource = new FakeMediaSource(timeline); + Timeline secondTimeline = new FakeTimeline(/* windowCount= */ 2); + MediaSource secondMediaSource = new FakeMediaSource(secondTimeline); + ActionSchedule actionSchedule = + new ActionSchedule.Builder("testMultipleModificationWithRecursiveListenerInvocations") + .waitForTimelineChanged(timeline, Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE) + .clearMediaItems() + .setMediaItems(secondMediaSource) + .build(); + ExoPlayerTestRunner exoPlayerTestRunner = + new Builder() + .setMediaSources(mediaSource) + .setActionSchedule(actionSchedule) + .build(context) + .start() + .blockUntilActionScheduleFinished(TIMEOUT_MS) + .blockUntilEnded(TIMEOUT_MS); + + exoPlayerTestRunner.assertTimelinesSame( + dummyTimeline, timeline, Timeline.EMPTY, dummyTimeline, secondTimeline); + exoPlayerTestRunner.assertTimelineChangeReasonsEqual( + Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED, + Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE, + Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED, + Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED, + Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE); + } + @Test public void testPrepareWhenAlreadyPreparedIsANoop() throws Exception { Timeline timeline = new FakeTimeline(/* windowCount= */ 1);