From a237ae18100989bcb44216064b04cdf31b409895 Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 27 Jul 2018 06:40:40 -0700 Subject: [PATCH] Fix period transition with non-zero start position. Period transitions with non-zero start position happen too early as the playing period is advanced as soon as the renderer offset is reached not taking into account that the start position needs to be added to that. Issue:#4583 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=206310328 --- RELEASENOTES.md | 2 + .../exoplayer2/ExoPlayerImplInternal.java | 5 +- .../android/exoplayer2/MediaPeriodHolder.java | 6 +- .../android/exoplayer2/ExoPlayerTest.java | 67 +++++++++++++++++++ 4 files changed, 77 insertions(+), 3 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 3514bea01d9..7363215fe2a 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -77,6 +77,8 @@ `MediaCodecRenderer`, and provide an (optional) `MediaCodecSelector` that falls back to less preferred decoders like `MediaCodec.createDecoderByType` ([#273](https://github.com/google/ExoPlayer/issues/273)). +* Fix where transitions to clipped media sources happened too early + ([#4583](https://github.com/google/ExoPlayer/issues/4583)). ### 2.8.3 ### 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 a4b12388406..03a42deec1c 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 @@ -1403,8 +1403,9 @@ private void updatePeriods() throws ExoPlaybackException, IOException { MediaPeriodHolder playingPeriodHolder = queue.getPlayingPeriod(); MediaPeriodHolder readingPeriodHolder = queue.getReadingPeriod(); boolean advancedPlayingPeriod = false; - while (playWhenReady && playingPeriodHolder != readingPeriodHolder - && rendererPositionUs >= playingPeriodHolder.next.rendererPositionOffsetUs) { + while (playWhenReady + && playingPeriodHolder != readingPeriodHolder + && rendererPositionUs >= playingPeriodHolder.next.getStartPositionRendererTime()) { // All enabled renderers' streams have been read to the end, and the playback position reached // the end of the playing period, so advance playback to the next period. if (advancedPlayingPeriod) { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java index d9dde9d4e8a..a74a2ac1cac 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java @@ -39,7 +39,6 @@ public final SampleStream[] sampleStreams; public final boolean[] mayRetainStreamFlags; - public long rendererPositionOffsetUs; public boolean prepared; public boolean hasEnabledTracks; public MediaPeriodInfo info; @@ -51,6 +50,7 @@ private final TrackSelector trackSelector; private final MediaSource mediaSource; + private long rendererPositionOffsetUs; private TrackSelectorResult periodTrackSelectorResult; /** @@ -105,6 +105,10 @@ public long getRendererOffset() { return rendererPositionOffsetUs; } + public long getStartPositionRendererTime() { + return info.startPositionUs + rendererPositionOffsetUs; + } + public boolean isFullyBuffered() { return prepared && (!hasEnabledTracks || mediaPeriod.getBufferedPositionUs() == C.TIME_END_OF_SOURCE); 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 67222f5f13e..3b81782ddf4 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 @@ -20,8 +20,10 @@ import android.support.annotation.Nullable; import android.view.Surface; +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.source.ClippingMediaSource; import com.google.android.exoplayer2.source.ConcatenatingMediaSource; import com.google.android.exoplayer2.source.MediaSource; import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId; @@ -32,6 +34,7 @@ import com.google.android.exoplayer2.testutil.ActionSchedule; import com.google.android.exoplayer2.testutil.ActionSchedule.PlayerRunnable; import com.google.android.exoplayer2.testutil.ActionSchedule.PlayerTarget; +import com.google.android.exoplayer2.testutil.AutoAdvancingFakeClock; import com.google.android.exoplayer2.testutil.ExoPlayerTestRunner; import com.google.android.exoplayer2.testutil.ExoPlayerTestRunner.Builder; import com.google.android.exoplayer2.testutil.FakeMediaClockRenderer; @@ -46,6 +49,7 @@ import com.google.android.exoplayer2.testutil.RobolectricUtil; import com.google.android.exoplayer2.upstream.Allocator; import com.google.android.exoplayer2.upstream.TransferListener; +import com.google.android.exoplayer2.util.Clock; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -2211,6 +2215,69 @@ public void run(SimpleExoPlayer player) { assertThat(eventListenerPlayWhenReady).containsExactly(true, true, true, false).inOrder(); } + @Test + public void testClippedLoopedPeriodsArePlayedFully() throws Exception { + long startPositionUs = 300_000; + long expectedDurationUs = 700_000; + MediaSource mediaSource = + new ClippingMediaSource( + new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1), /* manifest= */ null), + startPositionUs, + startPositionUs + expectedDurationUs); + Clock clock = new AutoAdvancingFakeClock(); + AtomicReference playerReference = new AtomicReference<>(); + AtomicReference positionAtDiscontinuityMs = new AtomicReference<>(); + AtomicReference clockAtStartMs = new AtomicReference<>(); + AtomicReference clockAtDiscontinuityMs = new AtomicReference<>(); + EventListener eventListener = + new EventListener() { + @Override + public void onPlayerStateChanged(boolean playWhenReady, int playbackState) { + if (playbackState == Player.STATE_READY && clockAtStartMs.get() == null) { + clockAtStartMs.set(clock.elapsedRealtime()); + } + } + + @Override + public void onPositionDiscontinuity(@DiscontinuityReason int reason) { + if (reason == Player.DISCONTINUITY_REASON_PERIOD_TRANSITION) { + positionAtDiscontinuityMs.set(playerReference.get().getCurrentPosition()); + clockAtDiscontinuityMs.set(clock.elapsedRealtime()); + } + } + }; + ActionSchedule actionSchedule = + new ActionSchedule.Builder("testClippedLoopedPeriodsArePlayedFully") + .executeRunnable( + new PlayerRunnable() { + @Override + public void run(SimpleExoPlayer player) { + playerReference.set(player); + player.addListener(eventListener); + } + }) + .pause() + .setRepeatMode(Player.REPEAT_MODE_ALL) + .waitForPlaybackState(Player.STATE_READY) + // Play until the media repeats once. + .playUntilPosition(/* windowIndex= */ 0, /* positionMs= */ 1) + .playUntilStartOfWindow(/* windowIndex= */ 0) + .setRepeatMode(Player.REPEAT_MODE_OFF) + .play() + .build(); + new ExoPlayerTestRunner.Builder() + .setClock(clock) + .setMediaSource(mediaSource) + .setActionSchedule(actionSchedule) + .build() + .start() + .blockUntilEnded(TIMEOUT_MS); + + assertThat(positionAtDiscontinuityMs.get()).isAtLeast(0L); + assertThat(clockAtDiscontinuityMs.get() - clockAtStartMs.get()) + .isAtLeast(C.usToMs(expectedDurationUs)); + } + // Internal methods. private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) {