From f6eb2e6dd55ed6c91fbd7cbf2413fa0979e2dea6 Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 10 Jan 2025 04:01:09 -0800 Subject: [PATCH] Fix discontinuity reporting in ClippingMediaPeriod The initial discontinuity is currently only reported if the period is prepared at the clip start position. However, we need the discontinuity whenever we prepare at a non-zero position (unless we know all samples are sync samples). PiperOrigin-RevId: 713994155 --- RELEASENOTES.md | 2 + .../exoplayer/source/ClippingMediaPeriod.java | 3 +- .../media3/exoplayer/ExoPlayerTest.java | 8 +- .../DefaultAnalyticsCollectorTest.java | 16 +- .../source/ClippingMediaPeriodTest.java | 212 +++++++++++++++--- .../video/MediaCodecVideoRendererTest.java | 2 + .../media3/test/utils/FakeRenderer.java | 4 + 7 files changed, 214 insertions(+), 33 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index efef6131a5..30804ef597 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -10,6 +10,8 @@ provide a secondary `MediaCodecVideoRenderer` to `ExoPlayer`. If enabled, `ExoPlayer` will pre-process the video of consecutive media items during playback to reduce media item transition latency. + * Fix issue where additional decode-only frames may be displayed in quick + succession when transitioning to content media after a mid-roll ad. * Transformer: * Enable support for Android platform diagnostics via `MediaMetricsManager`. Transformer will forward editing events and diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ClippingMediaPeriod.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ClippingMediaPeriod.java index cd30c9c22b..a887b7d043 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ClippingMediaPeriod.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ClippingMediaPeriod.java @@ -137,8 +137,7 @@ public final class ClippingMediaPeriod implements MediaPeriod, MediaPeriod.Callb selections, mayRetainStreamFlags, childStreams, streamResetFlags, positionUs); pendingInitialDiscontinuityPositionUs = isPendingInitialDiscontinuity() - && positionUs == startUs - && shouldKeepInitialDiscontinuity(startUs, selections) + && shouldKeepInitialDiscontinuity(enablePositionUs, selections) ? enablePositionUs : C.TIME_UNSET; Assertions.checkState( diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java index b5a1d2c29f..68383b5c13 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -69,6 +69,7 @@ import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.runUnti import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.runUntilPositionDiscontinuity; import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.runUntilSleepingForOffload; import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.runUntilTimelineChanged; +import static com.google.common.collect.Iterables.getLast; import static com.google.common.truth.Truth.assertThat; import static java.util.Arrays.stream; import static org.junit.Assert.assertArrayEquals; @@ -12022,6 +12023,9 @@ public class ExoPlayerTest { @Override public void setPlaybackParameters(PlaybackParameters parameters) { + if (!playbackParameters.isEmpty() && getLast(playbackParameters).equals(parameters)) { + return; + } playbackParameters.add(parameters); } @@ -12029,7 +12033,7 @@ public class ExoPlayerTest { public PlaybackParameters getPlaybackParameters() { return playbackParameters.isEmpty() ? PlaybackParameters.DEFAULT - : Iterables.getLast(playbackParameters); + : getLast(playbackParameters); } @Override @@ -12477,7 +12481,7 @@ public class ExoPlayerTest { verify(listener).onRepeatModeChanged(anyInt()); verify(listener).onShuffleModeEnabledChanged(anyBoolean()); verify(listener, times(2)).onEvents(eq(player), eventCaptor.capture()); - events = Iterables.getLast(eventCaptor.getAllValues()); + events = getLast(eventCaptor.getAllValues()); assertThat(events.contains(Player.EVENT_REPEAT_MODE_CHANGED)).isTrue(); assertThat(events.contains(Player.EVENT_SHUFFLE_MODE_ENABLED_CHANGED)).isTrue(); diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollectorTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollectorTest.java index 6e1118eded..3ad11f5a4f 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollectorTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultAnalyticsCollectorTest.java @@ -1253,7 +1253,10 @@ public final class DefaultAnalyticsCollectorTest { postrollAd, contentAfterPostroll) .inOrder(); - assertThat(listener.getEvents(EVENT_VIDEO_ENABLED)).containsExactly(prerollAd); + assertThat(listener.getEvents(EVENT_VIDEO_ENABLED)) + .containsExactly(prerollAd, contentAfterPreroll, contentAfterMidroll); + assertThat(listener.getEvents(EVENT_VIDEO_DISABLED)) + .containsExactly(prerollAd, contentAfterPreroll); assertThat(listener.getEvents(EVENT_VIDEO_DECODER_INITIALIZED)) .containsExactly( prerollAd, @@ -1273,9 +1276,14 @@ public final class DefaultAnalyticsCollectorTest { contentAfterPostroll) .inOrder(); assertThat(listener.getEvents(EVENT_DROPPED_VIDEO_FRAMES)) - .containsExactly(contentAfterPostroll); + .containsExactly(contentAfterPreroll, contentAfterPostroll); assertThat(listener.getEvents(EVENT_VIDEO_SIZE_CHANGED)) - .containsExactly(prerollAd) // First frame rendered + .containsExactly( + prerollAd, // First frame rendered + contentAfterPreroll, // Size unknown for renderer reset + contentAfterPreroll, // Content size + contentAfterMidroll, // Size unknown for renderer reset + contentAfterMidroll) // Content size .inOrder(); assertThat(listener.getEvents(EVENT_RENDERED_FIRST_FRAME)) .containsExactly( @@ -1287,7 +1295,7 @@ public final class DefaultAnalyticsCollectorTest { contentAfterPostroll) .inOrder(); assertThat(listener.getEvents(EVENT_VIDEO_FRAME_PROCESSING_OFFSET)) - .containsExactly(contentAfterPostroll); + .containsExactly(contentAfterPreroll, contentAfterPostroll); assertThat(listener.getEvents(EVENT_RENDERER_READY_CHANGED)) .containsExactly(prerollAd /* videoTrue */); listener.assertNoMoreEvents(); diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ClippingMediaPeriodTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ClippingMediaPeriodTest.java index fc85afcf74..1a6ea97992 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ClippingMediaPeriodTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ClippingMediaPeriodTest.java @@ -22,6 +22,7 @@ import static com.google.common.truth.Truth.assertThat; import androidx.annotation.Nullable; import androidx.media3.common.C; import androidx.media3.common.Format; +import androidx.media3.common.MimeTypes; import androidx.media3.common.TrackGroup; import androidx.media3.decoder.DecoderInputBuffer; import androidx.media3.exoplayer.FormatHolder; @@ -39,6 +40,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import org.junit.Test; import org.junit.runner.RunWith; @@ -47,13 +49,21 @@ import org.junit.runner.RunWith; @RunWith(AndroidJUnit4.class) public class ClippingMediaPeriodTest { + private static final Format VIDEO_FORMAT = + new Format.Builder().setSampleMimeType(MimeTypes.VIDEO_H264).build(); + private static final Format AUDIO_FORMAT_ALL_SYNC_SAMPLES = + new Format.Builder().setSampleMimeType(MimeTypes.AUDIO_AC3).build(); + private static final TrackGroup VIDEO_TRACK_GROUP = new TrackGroup(VIDEO_FORMAT); + private static final TrackGroup AUDIO_TRACK_GROUP_ALL_SYNC_SAMPLES = + new TrackGroup(AUDIO_FORMAT_ALL_SYNC_SAMPLES); + @Test public void fastLoadingStreamAfterFirstRead_canBeReadFully() throws Exception { - TrackGroup trackGroup = new TrackGroup(new Format.Builder().build()); + TrackGroupArray trackGroups = new TrackGroupArray(VIDEO_TRACK_GROUP); // Set up MediaPeriod with no samples and only add samples after the first SampleStream read. FakeMediaPeriod mediaPeriod = new FakeMediaPeriod( - new TrackGroupArray(trackGroup), + trackGroups, new DefaultAllocator(/* trimOnReset= */ true, /* individualAllocationSize= */ 1024), /* trackDataFactory= */ (format, mediaPeriodId) -> ImmutableList.of(), new MediaSourceEventListener.EventDispatcher() @@ -107,29 +117,9 @@ public class ClippingMediaPeriodTest { /* enableInitialDiscontinuity= */ true, /* startUs= */ 0, /* endUs= */ 500); - AtomicBoolean periodPrepared = new AtomicBoolean(); - clippingMediaPeriod.prepare( - new MediaPeriod.Callback() { - @Override - public void onPrepared(MediaPeriod mediaPeriod) { - periodPrepared.set(true); - } - - @Override - public void onContinueLoadingRequested(MediaPeriod source) { - clippingMediaPeriod.continueLoading( - new LoadingInfo.Builder().setPlaybackPositionUs(0).build()); - } - }, - /* positionUs= */ 0); - RobolectricUtil.runMainLooperUntil(periodPrepared::get); - SampleStream[] sampleStreams = new SampleStream[1]; - clippingMediaPeriod.selectTracks( - new ExoTrackSelection[] {new FixedTrackSelection(trackGroup, /* track= */ 0)}, - /* mayRetainStreamFlags= */ new boolean[] {false}, - sampleStreams, - /* streamResetFlags= */ new boolean[] {true}, - /* positionUs= */ 0); + SampleStream[] sampleStreams = + prepareMediaPeriodAndSelectTracks( + clippingMediaPeriod, /* preparePositionUs= */ 0, trackGroups); FormatHolder formatHolder = new FormatHolder(); DecoderInputBuffer buffer = new DecoderInputBuffer(DecoderInputBuffer.BUFFER_REPLACEMENT_MODE_NORMAL); @@ -145,4 +135,176 @@ public class ClippingMediaPeriodTest { assertThat(readSamples).containsExactly(0L, 200L, 400L).inOrder(); } + + @Test + public void readDiscontinuity_prepareFromNonZero_returnsPreparePosition() throws Exception { + TrackGroupArray trackGroups = + new TrackGroupArray(AUDIO_TRACK_GROUP_ALL_SYNC_SAMPLES, VIDEO_TRACK_GROUP); + ClippingMediaPeriod clippingMediaPeriod = + new ClippingMediaPeriod( + getFakeMediaPeriod(trackGroups), + /* enableInitialDiscontinuity= */ true, + /* startUs= */ 0, + /* endUs= */ 500); + + prepareMediaPeriodAndSelectTracks( + clippingMediaPeriod, /* preparePositionUs= */ 250, trackGroups); + long discontinuityPositionUs = clippingMediaPeriod.readDiscontinuity(); + + assertThat(discontinuityPositionUs).isEqualTo(250); + } + + @Test + public void readDiscontinuity_prepareFromNonZeroClipStartPosition_returnsPreparePosition() + throws Exception { + TrackGroupArray trackGroups = + new TrackGroupArray(AUDIO_TRACK_GROUP_ALL_SYNC_SAMPLES, VIDEO_TRACK_GROUP); + ClippingMediaPeriod clippingMediaPeriod = + new ClippingMediaPeriod( + getFakeMediaPeriod(trackGroups), + /* enableInitialDiscontinuity= */ true, + /* startUs= */ 250, + /* endUs= */ 500); + + prepareMediaPeriodAndSelectTracks( + clippingMediaPeriod, /* preparePositionUs= */ 250, trackGroups); + long discontinuityPositionUs = clippingMediaPeriod.readDiscontinuity(); + + assertThat(discontinuityPositionUs).isEqualTo(250); + } + + @Test + public void readDiscontinuity_prepareFromZero_returnsUnset() throws Exception { + TrackGroupArray trackGroups = + new TrackGroupArray(AUDIO_TRACK_GROUP_ALL_SYNC_SAMPLES, VIDEO_TRACK_GROUP); + ClippingMediaPeriod clippingMediaPeriod = + new ClippingMediaPeriod( + getFakeMediaPeriod(trackGroups), + /* enableInitialDiscontinuity= */ true, + /* startUs= */ 0, + /* endUs= */ 500); + + prepareMediaPeriodAndSelectTracks(clippingMediaPeriod, /* preparePositionUs= */ 0, trackGroups); + long discontinuityPositionUs = clippingMediaPeriod.readDiscontinuity(); + + assertThat(discontinuityPositionUs).isEqualTo(C.TIME_UNSET); + } + + @Test + public void readDiscontinuity_withSyncSamplesOnly_returnsUnset() throws Exception { + TrackGroupArray trackGroups = new TrackGroupArray(AUDIO_TRACK_GROUP_ALL_SYNC_SAMPLES); + ClippingMediaPeriod clippingMediaPeriod = + new ClippingMediaPeriod( + getFakeMediaPeriod(trackGroups), + /* enableInitialDiscontinuity= */ true, + /* startUs= */ 0, + /* endUs= */ 500); + + prepareMediaPeriodAndSelectTracks( + clippingMediaPeriod, /* preparePositionUs= */ 250, trackGroups); + long discontinuityPositionUs = clippingMediaPeriod.readDiscontinuity(); + + assertThat(discontinuityPositionUs).isEqualTo(C.TIME_UNSET); + } + + @Test + public void readDiscontinuity_repeatedCalls_returnsUnset() throws Exception { + TrackGroupArray trackGroups = + new TrackGroupArray(AUDIO_TRACK_GROUP_ALL_SYNC_SAMPLES, VIDEO_TRACK_GROUP); + ClippingMediaPeriod clippingMediaPeriod = + new ClippingMediaPeriod( + getFakeMediaPeriod(trackGroups), + /* enableInitialDiscontinuity= */ true, + /* startUs= */ 0, + /* endUs= */ 500); + + prepareMediaPeriodAndSelectTracks( + clippingMediaPeriod, /* preparePositionUs= */ 250, trackGroups); + clippingMediaPeriod.readDiscontinuity(); + long discontinuityPositionUs = clippingMediaPeriod.readDiscontinuity(); + + assertThat(discontinuityPositionUs).isEqualTo(C.TIME_UNSET); + } + + @Test + public void readDiscontinuity_withInitialDiscontinuityDisabled_returnsUnset() throws Exception { + TrackGroupArray trackGroups = + new TrackGroupArray(AUDIO_TRACK_GROUP_ALL_SYNC_SAMPLES, VIDEO_TRACK_GROUP); + ClippingMediaPeriod clippingMediaPeriod = + new ClippingMediaPeriod( + getFakeMediaPeriod(trackGroups), + /* enableInitialDiscontinuity= */ false, + /* startUs= */ 0, + /* endUs= */ 500); + + prepareMediaPeriodAndSelectTracks( + clippingMediaPeriod, /* preparePositionUs= */ 250, trackGroups); + long discontinuityPositionUs = clippingMediaPeriod.readDiscontinuity(); + + assertThat(discontinuityPositionUs).isEqualTo(C.TIME_UNSET); + } + + @Test + public void readDiscontinuity_afterInitialSeek_returnsUnset() throws Exception { + TrackGroupArray trackGroups = + new TrackGroupArray(AUDIO_TRACK_GROUP_ALL_SYNC_SAMPLES, VIDEO_TRACK_GROUP); + ClippingMediaPeriod clippingMediaPeriod = + new ClippingMediaPeriod( + getFakeMediaPeriod(trackGroups), + /* enableInitialDiscontinuity= */ true, + /* startUs= */ 0, + /* endUs= */ 500); + + prepareMediaPeriodAndSelectTracks( + clippingMediaPeriod, /* preparePositionUs= */ 250, trackGroups); + clippingMediaPeriod.seekToUs(400); + long discontinuityPositionUs = clippingMediaPeriod.readDiscontinuity(); + + assertThat(discontinuityPositionUs).isEqualTo(C.TIME_UNSET); + } + + private static SampleStream[] prepareMediaPeriodAndSelectTracks( + MediaPeriod mediaPeriod, long preparePositionUs, TrackGroupArray trackGroups) + throws TimeoutException { + AtomicBoolean periodPrepared = new AtomicBoolean(); + mediaPeriod.prepare( + new MediaPeriod.Callback() { + @Override + public void onPrepared(MediaPeriod mediaPeriod) { + periodPrepared.set(true); + } + + @Override + public void onContinueLoadingRequested(MediaPeriod source) { + mediaPeriod.continueLoading(new LoadingInfo.Builder().setPlaybackPositionUs(0).build()); + } + }, + preparePositionUs); + RobolectricUtil.runMainLooperUntil(periodPrepared::get); + SampleStream[] sampleStreams = new SampleStream[trackGroups.length]; + ExoTrackSelection[] trackSelections = new ExoTrackSelection[trackGroups.length]; + for (int i = 0; i < trackGroups.length; i++) { + trackSelections[i] = new FixedTrackSelection(trackGroups.get(i), /* track= */ 0); + } + mediaPeriod.selectTracks( + trackSelections, + /* mayRetainStreamFlags= */ new boolean[trackGroups.length], + sampleStreams, + /* streamResetFlags= */ new boolean[trackGroups.length], + preparePositionUs); + return sampleStreams; + } + + private static FakeMediaPeriod getFakeMediaPeriod(TrackGroupArray trackGroups) { + return new FakeMediaPeriod( + trackGroups, + new DefaultAllocator(/* trimOnReset= */ true, /* individualAllocationSize= */ 1024), + /* trackDataFactory= */ (format, mediaPeriodId) -> ImmutableList.of(), + new MediaSourceEventListener.EventDispatcher() + .withParameters( + /* windowIndex= */ 0, new MediaSource.MediaPeriodId(/* periodUid= */ new Object())), + DrmSessionManager.DRM_UNSUPPORTED, + new DrmSessionEventListener.EventDispatcher(), + /* deferOnPrepared= */ false); + } } diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/MediaCodecVideoRendererTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/MediaCodecVideoRendererTest.java index 6767db1139..95bd73f5c0 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/MediaCodecVideoRendererTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/MediaCodecVideoRendererTest.java @@ -647,6 +647,7 @@ public class MediaCodecVideoRendererTest { sampleStreams, /* streamResetFlags= */ new boolean[] {true}, /* positionUs= */ 100); + clippingMediaPeriod.readDiscontinuity(); mediaCodecVideoRenderer = new MediaCodecVideoRenderer( ApplicationProvider.getApplicationContext(), @@ -752,6 +753,7 @@ public class MediaCodecVideoRendererTest { sampleStreams, /* streamResetFlags= */ new boolean[] {true}, /* positionUs= */ 100); + clippingMediaPeriod.readDiscontinuity(); mediaCodecVideoRenderer = new MediaCodecVideoRenderer( ApplicationProvider.getApplicationContext(), diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeRenderer.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeRenderer.java index 4dc8b9df59..214ade1288 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeRenderer.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeRenderer.java @@ -83,6 +83,10 @@ public class FakeRenderer extends BaseRenderer { @Override protected void onPositionReset(long positionUs, boolean joining) throws ExoPlaybackException { + if (playbackPositionUs == positionUs && lastSamplePositionUs == Long.MIN_VALUE && !isEnded) { + // Nothing change, ignore reset operation. + return; + } playbackPositionUs = positionUs; lastSamplePositionUs = Long.MIN_VALUE; hasPendingBuffer = false;