From d6e74bc19b05a3c4cc1d6c7684a4692aea5ce1e3 Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 5 Aug 2019 10:21:55 +0100 Subject: [PATCH] Ensure position reset keep window sequence number. We currently keep the sequence number if we don't reset the position. However, the sequence number should be kept if we don't reset the state. Otherwise re-prepare with position reset is counted as new playback although it's still the same. PiperOrigin-RevId: 261644924 --- .../android/exoplayer2/ExoPlayerImpl.java | 2 +- .../exoplayer2/ExoPlayerImplInternal.java | 6 +-- .../android/exoplayer2/PlaybackInfo.java | 19 ++++++--- .../android/exoplayer2/ExoPlayerTest.java | 42 +++++++++++++++++++ 4 files changed, 60 insertions(+), 9 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 3eed66402d..e99429d3b2 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 @@ -647,7 +647,7 @@ import java.util.concurrent.CopyOnWriteArrayList; resetPosition = resetPosition || resetState; MediaPeriodId mediaPeriodId = resetPosition - ? playbackInfo.getDummyFirstMediaPeriodId(shuffleModeEnabled, window) + ? playbackInfo.getDummyFirstMediaPeriodId(shuffleModeEnabled, window, period) : playbackInfo.periodId; long startPositionUs = resetPosition ? 0 : playbackInfo.positionUs; long contentPositionUs = resetPosition ? C.TIME_UNSET : playbackInfo.contentPositionUs; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index 4fe8da92c2..6ab0838e26 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -646,7 +646,7 @@ import java.util.concurrent.atomic.AtomicBoolean; if (resolvedSeekPosition == null) { // The seek position was valid for the timeline that it was performed into, but the // timeline has changed or is not ready and a suitable seek position could not be resolved. - periodId = playbackInfo.getDummyFirstMediaPeriodId(shuffleModeEnabled, window); + periodId = playbackInfo.getDummyFirstMediaPeriodId(shuffleModeEnabled, window, period); periodPositionUs = C.TIME_UNSET; contentPositionUs = C.TIME_UNSET; seekPositionAdjusted = true; @@ -884,7 +884,7 @@ import java.util.concurrent.atomic.AtomicBoolean; } } - queue.clear(/* keepFrontPeriodUid= */ !resetPosition); + queue.clear(/* keepFrontPeriodUid= */ !resetState); setIsLoading(false); if (resetState) { queue.setTimeline(Timeline.EMPTY); @@ -896,7 +896,7 @@ import java.util.concurrent.atomic.AtomicBoolean; } MediaPeriodId mediaPeriodId = resetPosition - ? playbackInfo.getDummyFirstMediaPeriodId(shuffleModeEnabled, window) + ? playbackInfo.getDummyFirstMediaPeriodId(shuffleModeEnabled, window, period) : playbackInfo.periodId; // Set the start position to TIME_UNSET so that a subsequent seek to 0 isn't ignored. long startPositionUs = resetPosition ? C.TIME_UNSET : playbackInfo.positionUs; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java b/library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java index 669f41ca13..e9b99acd77 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java @@ -150,17 +150,26 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; * * @param shuffleModeEnabled Whether shuffle mode is enabled. * @param window A writable {@link Timeline.Window}. + * @param period A writable {@link Timeline.Period}. * @return A dummy media period id for the first-to-be-played period of the current timeline. */ public MediaPeriodId getDummyFirstMediaPeriodId( - boolean shuffleModeEnabled, Timeline.Window window) { + boolean shuffleModeEnabled, Timeline.Window window, Timeline.Period period) { if (timeline.isEmpty()) { return DUMMY_MEDIA_PERIOD_ID; } - int firstPeriodIndex = - timeline.getWindow(timeline.getFirstWindowIndex(shuffleModeEnabled), window) - .firstPeriodIndex; - return new MediaPeriodId(timeline.getUidOfPeriod(firstPeriodIndex)); + int firstWindowIndex = timeline.getFirstWindowIndex(shuffleModeEnabled); + int firstPeriodIndex = timeline.getWindow(firstWindowIndex, window).firstPeriodIndex; + int currentPeriodIndex = timeline.getIndexOfPeriod(periodId.periodUid); + long windowSequenceNumber = C.INDEX_UNSET; + if (currentPeriodIndex != C.INDEX_UNSET) { + int currentWindowIndex = timeline.getPeriod(currentPeriodIndex, period).windowIndex; + if (firstWindowIndex == currentWindowIndex) { + // Keep window sequence number if the new position is still in the same window. + windowSequenceNumber = periodId.windowSequenceNumber; + } + } + return new MediaPeriodId(timeline.getUidOfPeriod(firstPeriodIndex), windowSequenceNumber); } /** 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 d5b0b2c667..f924dfb34c 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 @@ -28,6 +28,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.Player.DiscontinuityReason; import com.google.android.exoplayer2.Player.EventListener; import com.google.android.exoplayer2.Timeline.Window; +import com.google.android.exoplayer2.analytics.AnalyticsListener; import com.google.android.exoplayer2.source.ClippingMediaSource; import com.google.android.exoplayer2.source.ConcatenatingMediaSource; import com.google.android.exoplayer2.source.MediaSource; @@ -59,6 +60,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; @@ -1434,6 +1436,46 @@ public final class ExoPlayerTest { assertThat(windowIndexHolder[2]).isEqualTo(1); } + @Test + public void playbackErrorAndReprepareWithPositionResetKeepsWindowSequenceNumber() + throws Exception { + FakeMediaSource mediaSource = new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1)); + ActionSchedule actionSchedule = + new ActionSchedule.Builder("playbackErrorWithResetKeepsWindowSequenceNumber") + .pause() + .waitForPlaybackState(Player.STATE_READY) + .throwPlaybackException(ExoPlaybackException.createForSource(new IOException())) + .waitForPlaybackState(Player.STATE_IDLE) + .prepareSource(mediaSource, /* resetPosition= */ true, /* resetState= */ false) + .waitForPlaybackState(Player.STATE_READY) + .play() + .build(); + HashSet reportedWindowSequenceNumbers = new HashSet<>(); + AnalyticsListener listener = + new AnalyticsListener() { + @Override + public void onPlayerStateChanged( + EventTime eventTime, boolean playWhenReady, int playbackState) { + if (eventTime.mediaPeriodId != null) { + reportedWindowSequenceNumbers.add(eventTime.mediaPeriodId.windowSequenceNumber); + } + } + }; + ExoPlayerTestRunner testRunner = + new ExoPlayerTestRunner.Builder() + .setMediaSource(mediaSource) + .setActionSchedule(actionSchedule) + .setAnalyticsListener(listener) + .build(context); + try { + testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS); + fail(); + } catch (ExoPlaybackException e) { + // Expected exception. + } + assertThat(reportedWindowSequenceNumbers).hasSize(1); + } + @Test public void testPlaybackErrorTwiceStillKeepsTimeline() throws Exception { final Timeline timeline = new FakeTimeline(/* windowCount= */ 1);