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 3514bea01d..7363215fe2 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 a4b1238840..03a42deec1 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 @@ import java.util.Collections; 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 d9dde9d4e8..a74a2ac1ca 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 @@ import com.google.android.exoplayer2.util.Assertions; public final SampleStream[] sampleStreams; public final boolean[] mayRetainStreamFlags; - public long rendererPositionOffsetUs; public boolean prepared; public boolean hasEnabledTracks; public MediaPeriodInfo info; @@ -51,6 +50,7 @@ import com.google.android.exoplayer2.util.Assertions; private final TrackSelector trackSelector; private final MediaSource mediaSource; + private long rendererPositionOffsetUs; private TrackSelectorResult periodTrackSelectorResult; /** @@ -105,6 +105,10 @@ import com.google.android.exoplayer2.util.Assertions; 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 67222f5f13..3b81782ddf 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 static org.junit.Assert.fail; 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.source.ads.AdPlaybackState; 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.FakeTrackSelector; 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 final class ExoPlayerTest { 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) {