diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 0510055942..e2f33d9150 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -19,6 +19,9 @@ ([#4788](https://github.com/google/ExoPlayer/issues/4788)). * SubRip: Add support for alignment tags, and remove tags from the displayed captions ([#4306](https://github.com/google/ExoPlayer/issues/4306)). +* Fix issue where buffered position is not updated correctly when transitioning + between periods + ([#4899](https://github.com/google/ExoPlayer/issues/4899)). ### 2.9.0 ### 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 5fb6202d2f..e1f942147d 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 @@ -619,7 +619,7 @@ import java.util.concurrent.CopyOnWriteArraySet; if (playbackInfo.startPositionUs == C.TIME_UNSET) { // Replace internal unset start position with externally visible start position of zero. playbackInfo = - playbackInfo.fromNewPosition( + playbackInfo.resetToNewPosition( playbackInfo.periodId, /* startPositionUs= */ 0, playbackInfo.contentPositionUs); } if ((!this.playbackInfo.timeline.isEmpty() || hasPendingPrepare) 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 8b22999679..40716c14c0 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 @@ -448,7 +448,11 @@ import java.util.Collections; seekToPeriodPosition(periodId, playbackInfo.positionUs, /* forceDisableRenderers= */ true); if (newPositionUs != playbackInfo.positionUs) { playbackInfo = - playbackInfo.fromNewPosition(periodId, newPositionUs, playbackInfo.contentPositionUs); + playbackInfo.copyWithNewPosition( + periodId, + newPositionUs, + playbackInfo.contentPositionUs, + getTotalBufferedDurationUs()); if (sendDiscontinuity) { playbackInfoUpdate.setPositionDiscontinuity(Player.DISCONTINUITY_REASON_INTERNAL); } @@ -483,8 +487,12 @@ import java.util.Collections; // A MediaPeriod may report a discontinuity at the current playback position to ensure the // renderers are flushed. Only report the discontinuity externally if the position changed. if (periodPositionUs != playbackInfo.positionUs) { - playbackInfo = playbackInfo.fromNewPosition(playbackInfo.periodId, periodPositionUs, - playbackInfo.contentPositionUs); + playbackInfo = + playbackInfo.copyWithNewPosition( + playbackInfo.periodId, + periodPositionUs, + playbackInfo.contentPositionUs, + getTotalBufferedDurationUs()); playbackInfoUpdate.setPositionDiscontinuity(Player.DISCONTINUITY_REASON_INTERNAL); } } else { @@ -645,7 +653,9 @@ import java.util.Collections; periodPositionUs = newPeriodPositionUs; } } finally { - playbackInfo = playbackInfo.fromNewPosition(periodId, periodPositionUs, contentPositionUs); + playbackInfo = + playbackInfo.copyWithNewPosition( + periodId, periodPositionUs, contentPositionUs, getTotalBufferedDurationUs()); if (seekPositionAdjusted) { playbackInfoUpdate.setPositionDiscontinuity(Player.DISCONTINUITY_REASON_SEEK_ADJUSTMENT); } @@ -1020,8 +1030,12 @@ import java.util.Collections; newTrackSelectorResult, playbackInfo.positionUs, recreateStreams, streamResetFlags); if (playbackInfo.playbackState != Player.STATE_ENDED && periodPositionUs != playbackInfo.positionUs) { - playbackInfo = playbackInfo.fromNewPosition(playbackInfo.periodId, periodPositionUs, - playbackInfo.contentPositionUs); + playbackInfo = + playbackInfo.copyWithNewPosition( + playbackInfo.periodId, + periodPositionUs, + playbackInfo.contentPositionUs, + getTotalBufferedDurationUs()); playbackInfoUpdate.setPositionDiscontinuity(Player.DISCONTINUITY_REASON_INTERNAL); resetRendererPosition(periodPositionUs); } @@ -1163,7 +1177,7 @@ import java.util.Collections; resolveSeekPosition(pendingInitialSeekPosition, /* trySubsequentPeriods= */ true); } catch (IllegalSeekPositionException e) { playbackInfo = - playbackInfo.fromNewPosition(getFirstMediaPeriodId(), C.TIME_UNSET, C.TIME_UNSET); + playbackInfo.resetToNewPosition(getFirstMediaPeriodId(), C.TIME_UNSET, C.TIME_UNSET); throw e; } pendingInitialSeekPosition = null; @@ -1176,7 +1190,7 @@ import java.util.Collections; long positionUs = periodPosition.second; MediaPeriodId periodId = queue.resolveMediaPeriodIdForAds(periodUid, positionUs); playbackInfo = - playbackInfo.fromNewPosition( + playbackInfo.resetToNewPosition( periodId, periodId.isAd() ? 0 : positionUs, /* contentPositionUs= */ positionUs); } } else if (playbackInfo.startPositionUs == C.TIME_UNSET) { @@ -1190,7 +1204,7 @@ import java.util.Collections; long startPositionUs = defaultPosition.second; MediaPeriodId periodId = queue.resolveMediaPeriodIdForAds(periodUid, startPositionUs); playbackInfo = - playbackInfo.fromNewPosition( + playbackInfo.resetToNewPosition( periodId, periodId.isAd() ? 0 : startPositionUs, /* contentPositionUs= */ startPositionUs); @@ -1209,7 +1223,7 @@ import java.util.Collections; long startPositionUs = defaultPosition.second; MediaPeriodId periodId = queue.resolveMediaPeriodIdForAds(periodUid, startPositionUs); playbackInfo = - playbackInfo.fromNewPosition( + playbackInfo.resetToNewPosition( periodId, /* startPositionUs= */ periodId.isAd() ? 0 : startPositionUs, /* contentPositionUs= */ startPositionUs); @@ -1248,7 +1262,9 @@ import java.util.Collections; } // Actually do the seek. long seekPositionUs = seekToPeriodPosition(periodId, periodId.isAd() ? 0 : contentPositionUs); - playbackInfo = playbackInfo.fromNewPosition(periodId, seekPositionUs, contentPositionUs); + playbackInfo = + playbackInfo.copyWithNewPosition( + periodId, seekPositionUs, contentPositionUs, getTotalBufferedDurationUs()); return; } @@ -1260,7 +1276,9 @@ import java.util.Collections; // The previously playing ad should no longer be played, so skip it. long seekPositionUs = seekToPeriodPosition(periodId, periodId.isAd() ? 0 : contentPositionUs); - playbackInfo = playbackInfo.fromNewPosition(periodId, seekPositionUs, contentPositionUs); + playbackInfo = + playbackInfo.copyWithNewPosition( + periodId, seekPositionUs, contentPositionUs, getTotalBufferedDurationUs()); return; } } @@ -1416,8 +1434,12 @@ import java.util.Collections; MediaPeriodHolder oldPlayingPeriodHolder = playingPeriodHolder; playingPeriodHolder = queue.advancePlayingPeriod(); updatePlayingPeriodRenderers(oldPlayingPeriodHolder); - playbackInfo = playbackInfo.fromNewPosition(playingPeriodHolder.info.id, - playingPeriodHolder.info.startPositionUs, playingPeriodHolder.info.contentPositionUs); + playbackInfo = + playbackInfo.copyWithNewPosition( + playingPeriodHolder.info.id, + playingPeriodHolder.info.startPositionUs, + playingPeriodHolder.info.contentPositionUs, + getTotalBufferedDurationUs()); playbackInfoUpdate.setPositionDiscontinuity(discontinuityReason); updatePlaybackPositions(); advancedPlayingPeriod = true; @@ -1667,6 +1689,11 @@ import java.util.Collections; if (loadingMediaPeriodChanged) { playbackInfo = playbackInfo.copyWithLoadingMediaPeriodId(loadingMediaPeriodId); } + playbackInfo.bufferedPositionUs = + loadingMediaPeriodHolder == null + ? playbackInfo.positionUs + : loadingMediaPeriodHolder.getBufferedPositionUs(); + playbackInfo.totalBufferedDurationUs = getTotalBufferedDurationUs(); if ((loadingMediaPeriodChanged || loadingTrackSelectionChanged) && loadingMediaPeriodHolder != null && loadingMediaPeriodHolder.prepared) { 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 02058c0484..8c73fde3be 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 @@ -15,6 +15,7 @@ */ package com.google.android.exoplayer2; +import android.support.annotation.CheckResult; import android.support.annotation.Nullable; import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId; import com.google.android.exoplayer2.source.TrackGroupArray; @@ -40,7 +41,8 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; public final MediaPeriodId periodId; /** * The start position at which playback started in {@link #periodId} relative to the start of the - * associated period in the {@link #timeline}, in microseconds. + * associated period in the {@link #timeline}, in microseconds. Note that this value changes for + * each position discontinuity. */ public final long startPositionUs; /** @@ -103,6 +105,23 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; startPositionUs); } + /** + * Create playback info. + * + * @param timeline See {@link #timeline}. + * @param manifest See {@link #manifest}. + * @param periodId See {@link #periodId}. + * @param startPositionUs See {@link #startPositionUs}. + * @param contentPositionUs See {@link #contentPositionUs}. + * @param playbackState See {@link #playbackState}. + * @param isLoading See {@link #isLoading}. + * @param trackGroups See {@link #trackGroups}. + * @param trackSelectorResult See {@link #trackSelectorResult}. + * @param loadingMediaPeriodId See {@link #loadingMediaPeriodId}. + * @param bufferedPositionUs See {@link #bufferedPositionUs}. + * @param totalBufferedDurationUs See {@link #totalBufferedDurationUs}. + * @param positionUs See {@link #positionUs}. + */ public PlaybackInfo( Timeline timeline, @Nullable Object manifest, @@ -132,7 +151,17 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; this.positionUs = positionUs; } - public PlaybackInfo fromNewPosition( + /** + * Copies playback info and resets playing and loading position. + * + * @param periodId New playing and loading {@link MediaPeriodId}. + * @param startPositionUs New start position. See {@link #startPositionUs}. + * @param contentPositionUs New content position. See {@link #contentPositionUs}. Value is ignored + * if {@code periodId.isAd()} is true. + * @return Copied playback info with reset position. + */ + @CheckResult + public PlaybackInfo resetToNewPosition( MediaPeriodId periodId, long startPositionUs, long contentPositionUs) { return new PlaybackInfo( timeline, @@ -150,6 +179,46 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; startPositionUs); } + /** + * Copied playback info with new playing position. + * + * @param periodId New playing media period. See {@link #periodId}. + * @param positionUs New position. See {@link #positionUs}. + * @param contentPositionUs New content position. See {@link #contentPositionUs}. Value is ignored + * if {@code periodId.isAd()} is true. + * @param totalBufferedDurationUs New buffered duration. See {@link #totalBufferedDurationUs}. + * @return Copied playback info with new playing position. + */ + @CheckResult + public PlaybackInfo copyWithNewPosition( + MediaPeriodId periodId, + long positionUs, + long contentPositionUs, + long totalBufferedDurationUs) { + return new PlaybackInfo( + timeline, + manifest, + periodId, + positionUs, + periodId.isAd() ? contentPositionUs : C.TIME_UNSET, + playbackState, + isLoading, + trackGroups, + trackSelectorResult, + loadingMediaPeriodId, + bufferedPositionUs, + totalBufferedDurationUs, + positionUs); + } + + /** + * Copies playback info with new timeline and manifest. + * + * @param timeline New timeline. See {@link #timeline}. + * @param manifest New manifest. See {@link #manifest}. + * @return Copied playback info with new timeline and manifest. + */ + @CheckResult public PlaybackInfo copyWithTimeline(Timeline timeline, Object manifest) { return new PlaybackInfo( timeline, @@ -167,6 +236,13 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; positionUs); } + /** + * Copies playback info with new playback state. + * + * @param playbackState New playback state. See {@link #playbackState}. + * @return Copied playback info with new playback state. + */ + @CheckResult public PlaybackInfo copyWithPlaybackState(int playbackState) { return new PlaybackInfo( timeline, @@ -184,6 +260,13 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; positionUs); } + /** + * Copies playback info with new loading state. + * + * @param isLoading New loading state. See {@link #isLoading}. + * @return Copied playback info with new loading state. + */ + @CheckResult public PlaybackInfo copyWithIsLoading(boolean isLoading) { return new PlaybackInfo( timeline, @@ -201,6 +284,14 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; positionUs); } + /** + * Copies playback info with new track information. + * + * @param trackGroups New track groups. See {@link #trackGroups}. + * @param trackSelectorResult New track selector result. See {@link #trackSelectorResult}. + * @return Copied playback info with new track information. + */ + @CheckResult public PlaybackInfo copyWithTrackInfo( TrackGroupArray trackGroups, TrackSelectorResult trackSelectorResult) { return new PlaybackInfo( @@ -219,6 +310,13 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; positionUs); } + /** + * Copies playback info with new loading media period. + * + * @param loadingMediaPeriodId New loading media period id. See {@link #loadingMediaPeriodId}. + * @return Copied playback info with new loading media period. + */ + @CheckResult public PlaybackInfo copyWithLoadingMediaPeriodId(MediaPeriodId loadingMediaPeriodId) { return new PlaybackInfo( timeline, 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 407e9a3827..d131ed0f51 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 @@ -2534,6 +2534,59 @@ public final class ExoPlayerTest { assertThat(positionWhenReady.get()).isEqualTo(periodDurationMs + 10); } + @Test + public void periodTransitionReportsCorrectBufferedPosition() throws Exception { + int periodCount = 3; + long periodDurationUs = 5 * C.MICROS_PER_SECOND; + long windowDurationUs = periodCount * periodDurationUs; + Timeline timeline = + new FakeTimeline( + new TimelineWindowDefinition( + periodCount, + /* id= */ new Object(), + /* isSeekable= */ true, + /* isDynamic= */ false, + windowDurationUs)); + AtomicReference playerReference = new AtomicReference<>(); + AtomicLong bufferedPositionAtFirstDiscontinuityMs = new AtomicLong(C.TIME_UNSET); + EventListener eventListener = + new EventListener() { + @Override + public void onPositionDiscontinuity(@DiscontinuityReason int reason) { + if (reason == Player.DISCONTINUITY_REASON_PERIOD_TRANSITION) { + if (bufferedPositionAtFirstDiscontinuityMs.get() == C.TIME_UNSET) { + bufferedPositionAtFirstDiscontinuityMs.set( + playerReference.get().getBufferedPosition()); + } + } + } + }; + ActionSchedule actionSchedule = + new ActionSchedule.Builder("periodTransitionReportsCorrectBufferedPosition") + .executeRunnable( + new PlayerRunnable() { + @Override + public void run(SimpleExoPlayer player) { + playerReference.set(player); + player.addListener(eventListener); + } + }) + .pause() + // Wait until all periods are fully buffered. + .waitForIsLoading(/* targetIsLoading= */ true) + .waitForIsLoading(/* targetIsLoading= */ false) + .play() + .build(); + new Builder() + .setTimeline(timeline) + .setActionSchedule(actionSchedule) + .build(context) + .start() + .blockUntilEnded(TIMEOUT_MS); + + assertThat(bufferedPositionAtFirstDiscontinuityMs.get()).isEqualTo(C.usToMs(windowDurationUs)); + } + // Internal methods. private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) {