From 2a1e71b2036a471d0ae1640f5813abc1abe619f7 Mon Sep 17 00:00:00 2001 From: michaelkatz Date: Thu, 17 Oct 2024 04:17:54 -0700 Subject: [PATCH] Reduce needless loading period resets in clipping mediasource playlists When a `SampleQueue` is prepared prior to playback, the start position may be less than the timestamp of the first sample in the queue and still be valid. This scenario can come about with specific clipping values and if all samples are sync samples. Currently, with `ClippingMediaPeriods` around `ProgressiveMediaPeriods`, if the `SampleQueue` has already been reset through the seekTo operation in `onPrepared`, then in the aforementioned scenario the seekTo operation in `handleDiscontinuity` will remove all samples and reset the loading periods unnecessarily. The solution is that if the `ProgressiveMediaPeriod` has already handled a seekTo operation for the same position and the sample queue has not been read yet, then loading does not need to be reset. The tests in `MergingPlaylistPlaybackTest` were specifically causing this behavior through its setup of `MergingMediaSources` around clipped `FilteringMediaSources`. Since the video content was not 'all sync samples', there would always be a discontinuity to handle and the audio content being 'all sync samples' would start with samples post start time. These changes also remove the flakiness from the `MergingPlaylistPlaybackTest`. PiperOrigin-RevId: 686858444 --- .../source/ProgressiveMediaPeriod.java | 11 +++++-- .../e2etest/MergingPlaylistPlaybackTest.java | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java index ef5421c443..5902dee840 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java @@ -447,6 +447,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; positionUs = seekMap.isSeekable() ? positionUs : 0; notifyDiscontinuity = false; + boolean isSameAsLastSeekPosition = lastSeekPositionUs == positionUs; lastSeekPositionUs = positionUs; if (isPendingReset()) { // A reset is already pending. We only need to update its position. @@ -458,7 +459,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; // and seek within the existing buffer instead of restarting the load. if (dataType != C.DATA_TYPE_MEDIA_PROGRESSIVE_LIVE && (loadingFinished || loader.isLoading()) - && seekInsideBufferUs(trackIsAudioVideoFlags, positionUs)) { + && seekInsideBufferUs(trackIsAudioVideoFlags, positionUs, isSameAsLastSeekPosition)) { return positionUs; } @@ -927,12 +928,18 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; * * @param trackIsAudioVideoFlags Whether each track is audio/video. * @param positionUs The seek position in microseconds. + * @param isSameAsLastSeekPosition Whether new seek position is same as that last called with + * {@link #seekToUs}. * @return Whether the in-buffer seek was successful. */ - private boolean seekInsideBufferUs(boolean[] trackIsAudioVideoFlags, long positionUs) { + private boolean seekInsideBufferUs( + boolean[] trackIsAudioVideoFlags, long positionUs, boolean isSameAsLastSeekPosition) { int trackCount = sampleQueues.length; for (int i = 0; i < trackCount; i++) { SampleQueue sampleQueue = sampleQueues[i]; + if (sampleQueue.getReadIndex() == 0 && isSameAsLastSeekPosition) { + continue; + } boolean seekInsideQueue = isSingleSample ? sampleQueue.seekTo(sampleQueue.getFirstIndex()) diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/e2etest/MergingPlaylistPlaybackTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/e2etest/MergingPlaylistPlaybackTest.java index 212ccdf8ee..84e874d1ec 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/e2etest/MergingPlaylistPlaybackTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/e2etest/MergingPlaylistPlaybackTest.java @@ -15,6 +15,12 @@ */ package androidx.media3.exoplayer.e2etest; +import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.run; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; + import android.content.Context; import android.graphics.SurfaceTexture; import android.view.Surface; @@ -91,6 +97,8 @@ public final class MergingPlaylistPlaybackTest { new ExoPlayer.Builder(applicationContext, capturingRenderersFactory) .setClock(new FakeClock(/* isAutoAdvancing= */ true)) .build(); + Player.Listener listener = mock(Player.Listener.class); + player.addListener(listener); Surface surface = new Surface(new SurfaceTexture(/* texName= */ 1)); player.setVideoSurface(surface); PlaybackOutput playbackOutput = PlaybackOutput.register(player, capturingRenderersFactory); @@ -98,6 +106,17 @@ public final class MergingPlaylistPlaybackTest { player.addMediaSource(createMergingMediaSource(firstItemVideoClipped, firstItemAudioClipped)); player.addMediaSource(createMergingMediaSource(secondItemVideoClipped, secondItemAudioClipped)); player.prepare(); + // Load all content prior to play to reduce flaky-ness resulting from the playback advancement + // speed and handling of discontinuities. + long durationToBufferMs = + (firstItemVideoClipped || firstItemAudioClipped ? 300L : 1024L) + + (secondItemVideoClipped || secondItemAudioClipped ? 300L : 1024L); + run(player) + .untilBackgroundThreadCondition( + () -> player.getTotalBufferedDuration() >= durationToBufferMs); + run(player).untilPendingCommandsAreFullyHandled(); + // Reset the listener to avoid verifying the onIsLoadingChanged events from prepare(). + reset(listener); player.play(); TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED); player.release(); @@ -117,6 +136,7 @@ public final class MergingPlaylistPlaybackTest { + "_" + secondItemAudioClipped + ".dump"); + verify(listener, never()).onIsLoadingChanged(true); } @Test @@ -128,6 +148,8 @@ public final class MergingPlaylistPlaybackTest { new ExoPlayer.Builder(applicationContext, capturingRenderersFactory) .setClock(new FakeClock(/* isAutoAdvancing= */ true)) .build(); + Player.Listener listener = mock(Player.Listener.class); + player.addListener(listener); Surface surface = new Surface(new SurfaceTexture(/* texName= */ 1)); player.setVideoSurface(surface); PlaybackOutput playbackOutput = PlaybackOutput.register(player, capturingRenderersFactory); @@ -136,6 +158,15 @@ public final class MergingPlaylistPlaybackTest { player.addMediaSource(createMergingMediaSource(firstItemVideoClipped, firstItemAudioClipped)); } player.prepare(); + // Load all content prior to play to reduce flaky-ness resulting from the playback advancement + // speed and handling of discontinuities. + long durationToBufferMs = (firstItemVideoClipped || firstItemAudioClipped ? 300L : 1024L) * 5; + run(player) + .untilBackgroundThreadCondition( + () -> player.getTotalBufferedDuration() >= durationToBufferMs); + // Reset the listener to avoid verifying the onIsLoadingChanged events from prepare(). + run(player).untilPendingCommandsAreFullyHandled(); + reset(listener); player.play(); TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED); player.release(); @@ -151,6 +182,7 @@ public final class MergingPlaylistPlaybackTest { + "_" + firstItemAudioClipped + ".dump"); + verify(listener, never()).onIsLoadingChanged(true); } private MergingMediaSource createMergingMediaSource(boolean videoClipped, boolean audioClipped) {