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 2e2d592d0a..54b363274c 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 @@ -1535,69 +1535,59 @@ import java.util.concurrent.atomic.AtomicBoolean; shuffleModeEnabled, window, period); - - playbackInfo = playbackInfo.copyWithTimeline(timeline); - resolvePendingMessagePositions(); - if (!timeline.isEmpty()) { - // Retain pending seek position only while the timeline is still empty. - pendingInitialSeekPosition = null; - } - - MediaPeriodId oldPeriodId = playbackInfo.periodId; - long oldContentPositionUs = - oldPeriodId.isAd() ? playbackInfo.contentPositionUs : playbackInfo.positionUs; MediaPeriodId newPeriodId = positionUpdate.periodId; long newContentPositionUs = positionUpdate.contentPositionUs; boolean forceBufferingState = positionUpdate.forceBufferingState; long newPositionUs = newPeriodId.isAd() ? 0 : newContentPositionUs; + long oldContentPositionUs = + playbackInfo.periodId.isAd() ? playbackInfo.contentPositionUs : playbackInfo.positionUs; + boolean isPlaybackPositionUnchanged = + playbackInfo.periodId.equals(newPeriodId) && oldContentPositionUs == newContentPositionUs; - if (positionUpdate.endPlayback) { - if (playbackInfo.playbackState != Player.STATE_IDLE) { - setState(Player.STATE_ENDED); + try { + if (positionUpdate.endPlayback) { + if (playbackInfo.playbackState != Player.STATE_IDLE) { + setState(Player.STATE_ENDED); + } + resetInternal( + /* resetRenderers= */ false, + /* resetPosition= */ false, + /* releasePlaylist= */ false, + /* clearPlaylist= */ false, + /* resetError= */ true); } - playbackInfo = copyWithNewPosition(newPeriodId, newPositionUs, newContentPositionUs); - // Reset, but retain the playlist and new position. - resetInternal( - /* resetRenderers= */ false, - /* resetPosition= */ false, - /* releasePlaylist= */ false, - /* clearPlaylist= */ false, - /* resetError= */ true); - if (timeline.isEmpty()) { - return; - } - } - - if (oldPeriodId.equals(newPeriodId) && oldContentPositionUs == newContentPositionUs) { - // We can keep the current playing period. Update the rest of the queued periods. - if (!queue.updateQueuedPeriods( - timeline, rendererPositionUs, getMaxRendererReadPositionUs())) { - seekToCurrentPosition(/* sendDiscontinuity= */ false); - } - } else { - // Something changed. Seek to new start position. - @Nullable MediaPeriodHolder periodHolder = queue.getPlayingPeriod(); - if (periodHolder != null) { - // Update the new playing media period info if it already exists. - while (periodHolder.getNext() != null) { - periodHolder = periodHolder.getNext(); - if (periodHolder.info.id.equals(newPeriodId)) { - periodHolder.info = queue.getUpdatedMediaPeriodInfo(timeline, periodHolder.info); + if (isPlaybackPositionUnchanged) { + // We can keep the current playing period. Update the rest of the queued periods. + if (!queue.updateQueuedPeriods( + timeline, rendererPositionUs, getMaxRendererReadPositionUs())) { + seekToCurrentPosition(/* sendDiscontinuity= */ false); + } + } else if (!timeline.isEmpty()) { + // Something changed. Seek to new start position. + @Nullable MediaPeriodHolder periodHolder = queue.getPlayingPeriod(); + if (periodHolder != null) { + // Update the new playing media period info if it already exists. + while (periodHolder.getNext() != null) { + periodHolder = periodHolder.getNext(); + if (periodHolder.info.id.equals(newPeriodId)) { + periodHolder.info = queue.getUpdatedMediaPeriodInfo(timeline, periodHolder.info); + } } } + newPositionUs = seekToPeriodPosition(newPeriodId, newPositionUs, forceBufferingState); } - if (!newPeriodId.isAd() && newContentPositionUs == C.TIME_UNSET) { - // Get the default position for the first new period that is not an ad. - int windowIndex = timeline.getPeriodByUid(newPeriodId.periodUid, period).windowIndex; - newContentPositionUs = timeline.getWindow(windowIndex, window).getDefaultPositionUs(); - newPositionUs = newContentPositionUs; + } finally { + if (!isPlaybackPositionUnchanged) { + playbackInfo = copyWithNewPosition(newPeriodId, newPositionUs, newContentPositionUs); } - // Actually do the seek. - long seekedToPositionUs = - seekToPeriodPosition(newPeriodId, newPositionUs, forceBufferingState); - playbackInfo = copyWithNewPosition(newPeriodId, seekedToPositionUs, newContentPositionUs); + playbackInfo = playbackInfo.copyWithTimeline(timeline); + resolvePendingMessagePositions(); + if (!timeline.isEmpty()) { + // Retain pending seek position only while the timeline is still empty. + pendingInitialSeekPosition = null; + } + handleLoadingMediaPeriodChanged(/* loadingTrackSelectionChanged= */ false); } - handleLoadingMediaPeriodChanged(/* loadingTrackSelectionChanged= */ false); } private long getMaxRendererReadPositionUs() { @@ -2178,12 +2168,17 @@ import java.util.concurrent.atomic.AtomicBoolean; // Ensure ad insertion metadata is up to date. long contentPositionForAdResolution = newContentPositionUs; if (contentPositionForAdResolution == C.TIME_UNSET) { + // TODO: Fix me. Using a window position as period position is wrong. contentPositionForAdResolution = timeline.getWindow(timeline.getPeriodByUid(newPeriodUid, period).windowIndex, window) .defaultPositionUs; } MediaPeriodId periodIdWithAds = queue.resolveMediaPeriodIdForAds(timeline, newPeriodUid, contentPositionForAdResolution); + if (!periodIdWithAds.isAd() && newContentPositionUs == C.TIME_UNSET) { + // We are not going to play an ad, so use resolved content position. + newContentPositionUs = contentPositionForAdResolution; + } boolean oldAndNewPeriodIdAreSame = oldPeriodId.periodUid.equals(newPeriodUid) && !oldPeriodId.isAd() 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 36809c4ee7..8e42309e89 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 @@ -5593,7 +5593,7 @@ public final class ExoPlayerTest { } @Test - public void errorThrownDuringPeriodTransition_keepsConsistentPlayerState() throws Exception { + public void errorThrownDuringPeriodTransition_keepsConsistentPlayerState() { FakeMediaSource source1 = new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1), Builder.VIDEO_FORMAT); FakeMediaSource source2 = @@ -5658,6 +5658,75 @@ public final class ExoPlayerTest { assertThat(trackSelectionsAfterError.get().get(1)).isNotNull(); // Audio renderer. } + @Test + public void errorThrownDuringPlaylistUpdate_keepsConsistentPlayerState() { + FakeMediaSource source1 = + new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1), Builder.VIDEO_FORMAT); + FakeMediaSource source2 = + new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1), Builder.AUDIO_FORMAT); + FakeRenderer videoRenderer = new FakeRenderer(Builder.VIDEO_FORMAT); + FakeRenderer audioRenderer = + new FakeRenderer(Builder.AUDIO_FORMAT) { + @Override + protected void onEnabled(boolean joining) throws ExoPlaybackException { + // Fail when enabling the renderer. This will happen during the playlist update. + throw createRendererException(new IllegalStateException(), Builder.AUDIO_FORMAT); + } + }; + AtomicReference timelineAfterError = new AtomicReference<>(); + AtomicReference trackGroupsAfterError = new AtomicReference<>(); + AtomicReference trackSelectionsAfterError = new AtomicReference<>(); + AtomicInteger windowIndexAfterError = new AtomicInteger(); + ActionSchedule actionSchedule = + new ActionSchedule.Builder("errorThrownDuringPlaylistUpdate_keepsConsistentPlayerState") + .executeRunnable( + new PlayerRunnable() { + @Override + public void run(SimpleExoPlayer player) { + player.addAnalyticsListener( + new AnalyticsListener() { + @Override + public void onPlayerError( + EventTime eventTime, ExoPlaybackException error) { + timelineAfterError.set(player.getCurrentTimeline()); + trackGroupsAfterError.set(player.getCurrentTrackGroups()); + trackSelectionsAfterError.set(player.getCurrentTrackSelections()); + windowIndexAfterError.set(player.getCurrentWindowIndex()); + } + }); + } + }) + .pause() + // Wait until fully buffered so that the new renderer can be enabled immediately. + .waitForIsLoading(true) + .waitForIsLoading(false) + .waitForIsLoading(true) + .waitForIsLoading(false) + .removeMediaItem(0) + .build(); + ExoPlayerTestRunner testRunner = + new Builder() + .setMediaSources(source1, source2) + .setActionSchedule(actionSchedule) + .setRenderers(videoRenderer, audioRenderer) + .build(context); + + assertThrows( + ExoPlaybackException.class, + () -> + testRunner + .start(/* doPrepare= */ true) + .blockUntilActionScheduleFinished(TIMEOUT_MS) + .blockUntilEnded(TIMEOUT_MS)); + + assertThat(timelineAfterError.get().getWindowCount()).isEqualTo(1); + assertThat(windowIndexAfterError.get()).isEqualTo(0); + assertThat(trackGroupsAfterError.get().length).isEqualTo(1); + assertThat(trackGroupsAfterError.get().get(0).getFormat(0)).isEqualTo(Builder.AUDIO_FORMAT); + assertThat(trackSelectionsAfterError.get().get(0)).isNull(); // Video renderer. + assertThat(trackSelectionsAfterError.get().get(1)).isNotNull(); // Audio renderer. + } + // Internal methods. private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) {