From 1cbe9ba95bbcbbcd33472d96134e3b4fb46032b2 Mon Sep 17 00:00:00 2001 From: michaelkatz Date: Mon, 11 Mar 2024 11:27:26 -0700 Subject: [PATCH] Start playing period-enabled renderers when setting playWhenReady true Renderers may be enabled for subsequent media items as soon as the current media item's renderer's isEnded() returns true. Currently, when a player is set to pause, it stops all renderers that are `STATE_STARTED`. When a player is set to play, it starts all renderers that are enabled. This would include renderers that were enabled early for the subsequent media item. The solution is to only start renderers that are enabled by the current playing period. Issue: androidx/media#1017 PiperOrigin-RevId: 614734437 --- .../exoplayer2/ExoPlayerImplInternal.java | 29 +++---- .../android/exoplayer2/ExoPlayerTest.java | 77 +++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) 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 813f00a6942..242c475c668 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 @@ -70,7 +70,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; @@ -879,6 +878,9 @@ private void setPlayWhenReadyInternal( updatePlaybackPositions(); } else { if (playbackInfo.playbackState == Player.STATE_READY) { + updateRebufferingState( + /* isRebuffering= */ false, /* resetLastRebufferRealtimeMs= */ false); + mediaClock.start(); startRenderers(); handler.sendEmptyMessage(MSG_DO_SOME_WORK); } else if (playbackInfo.playbackState == Player.STATE_BUFFERING) { @@ -951,11 +953,14 @@ private void seekToCurrentPosition(boolean sendDiscontinuity) throws ExoPlayback } private void startRenderers() throws ExoPlaybackException { - updateRebufferingState(/* isRebuffering= */ false, /* resetLastRebufferRealtimeMs= */ false); - mediaClock.start(); - for (Renderer renderer : renderers) { - if (isRendererEnabled(renderer)) { - renderer.start(); + MediaPeriodHolder playingPeriodHolder = queue.getPlayingPeriod(); + if (playingPeriodHolder == null) { + return; + } + TrackSelectorResult trackSelectorResult = playingPeriodHolder.getTrackSelectorResult(); + for (int i = 0; i < renderers.length; i++) { + if (trackSelectorResult.isRendererEnabled(i) && renderers[i].getState() == STATE_ENABLED) { + renderers[i].start(); } } } @@ -1147,6 +1152,9 @@ && shouldTransitionToReadyState(renderersAllowPlayback)) { setState(Player.STATE_READY); pendingRecoverableRendererError = null; // Any pending error was successfully recovered from. if (shouldPlayWhenReady()) { + updateRebufferingState( + /* isRebuffering= */ false, /* resetLastRebufferRealtimeMs= */ false); + mediaClock.start(); startRenderers(); } } else if (playbackInfo.playbackState == Player.STATE_READY @@ -2326,14 +2334,7 @@ private void maybeUpdatePlayingPeriod() throws ExoPlaybackException { resetPendingPauseAtEndOfPeriod(); updatePlaybackPositions(); if (playbackInfo.playbackState == Player.STATE_READY) { - for (int i = 0; i < renderers.length; i++) { - if (renderers[i].getState() == STATE_ENABLED - && queue.getPlayingPeriod() != null - && Objects.equals( - renderers[i].getStream(), queue.getPlayingPeriod().sampleStreams[i])) { - renderers[i].start(); - } - } + startRenderers(); } allowRenderersToRenderStartOfStreams(); advancedPlayingPeriod = true; 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 2564ecee0e9..4dc51e70b7b 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 @@ -53,6 +53,7 @@ import static com.google.android.exoplayer2.robolectric.RobolectricUtil.runMainLooperUntil; import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.playUntilPosition; import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.playUntilStartOfMediaItem; +import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.run; import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilError; import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled; import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPlayWhenReady; @@ -13220,6 +13221,82 @@ protected boolean shouldProcessBuffer( eventsInOrder.verify(mockListener).onPlayerError(any(), any()); } + @Test + public void play_withReadingAheadWithNewRenderer_enablesButNotStartsNewRenderer() + throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + Renderer videoRenderer = player.getRenderer(/* index= */ 0); + Renderer audioRenderer = player.getRenderer(/* index= */ 1); + // Set a playlist that allows a new renderer to be enabled early. + player.setMediaSources( + ImmutableList.of( + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.VIDEO_FORMAT), + new FakeMediaSource( + new FakeTimeline(), + ExoPlayerTestRunner.VIDEO_FORMAT, + ExoPlayerTestRunner.AUDIO_FORMAT))); + player.prepare(); + + // Play a bit until the second renderer has been enabled, but not yet started. + run(player).untilPosition(/* mediaItemIndex= */ 0, /* positionMs= */ 5000); + @Renderer.State int videoState1 = videoRenderer.getState(); + @Renderer.State int audioState1 = audioRenderer.getState(); + // Play until we reached the start of the second item. + run(player).untilStartOfMediaItem(/* mediaItemIndex= */ 1); + run(player).untilPendingCommandsAreFullyHandled(); + @Renderer.State int videoState2 = videoRenderer.getState(); + @Renderer.State int audioState2 = audioRenderer.getState(); + player.release(); + + assertThat(videoState1).isEqualTo(Renderer.STATE_STARTED); + assertThat(videoState2).isEqualTo(Renderer.STATE_STARTED); + assertThat(audioState1).isEqualTo(Renderer.STATE_ENABLED); + assertThat(audioState2).isEqualTo(Renderer.STATE_STARTED); + } + + @Test + public void play_withReadingAheadWithNewRendererAndPausing_enablesButNotStartsNewRenderer() + throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + Renderer videoRenderer = player.getRenderer(/* index= */ 0); + Renderer audioRenderer = player.getRenderer(/* index= */ 1); + // Set a playlist that allows a new renderer to be enabled early. + player.setMediaSources( + ImmutableList.of( + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.VIDEO_FORMAT), + new FakeMediaSource( + new FakeTimeline(), + ExoPlayerTestRunner.VIDEO_FORMAT, + ExoPlayerTestRunner.AUDIO_FORMAT))); + player.prepare(); + + // Play until the second renderer has been enabled, but has not yet started. + run(player).untilPosition(/* mediaItemIndex= */ 0, /* positionMs= */ 5000); + // Pause in this "Read Ahead" state. + player.pause(); + run(player).untilPendingCommandsAreFullyHandled(); + @Renderer.State int videoState1 = videoRenderer.getState(); + @Renderer.State int audioState1 = audioRenderer.getState(); + // Play in this "Read Ahead" state. + player.play(); + run(player).untilPendingCommandsAreFullyHandled(); + @Renderer.State int videoState2 = videoRenderer.getState(); + @Renderer.State int audioState2 = audioRenderer.getState(); + // Play until the start of the second item. + run(player).untilStartOfMediaItem(/* mediaItemIndex= */ 1); + run(player).untilPendingCommandsAreFullyHandled(); + @Renderer.State int videoState3 = videoRenderer.getState(); + @Renderer.State int audioState3 = audioRenderer.getState(); + player.release(); + + assertThat(videoState1).isEqualTo(Renderer.STATE_ENABLED); + assertThat(videoState2).isEqualTo(Renderer.STATE_STARTED); + assertThat(videoState3).isEqualTo(Renderer.STATE_STARTED); + assertThat(audioState1).isEqualTo(Renderer.STATE_ENABLED); + assertThat(audioState2).isEqualTo(Renderer.STATE_ENABLED); + assertThat(audioState3).isEqualTo(Renderer.STATE_STARTED); + } + @Test public void replaceMediaItems_notReplacingCurrentItem_correctMasking() throws Exception { ExoPlayer player = new TestExoPlayerBuilder(context).build();