From 8b219b0ae69f80764183b680b44a8b0d6b03ed56 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 --- .../exoplayer/ExoPlayerImplInternal.java | 29 +++---- .../media3/exoplayer/ExoPlayerTest.java | 77 +++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java index 6df6b37fca..d0aa961107 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java @@ -79,7 +79,6 @@ import java.io.IOException; 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; @@ -880,6 +879,9 @@ import java.util.concurrent.atomic.AtomicBoolean; 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) { @@ -952,11 +954,14 @@ import java.util.concurrent.atomic.AtomicBoolean; } 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(); } } } @@ -1148,6 +1153,9 @@ import java.util.concurrent.atomic.AtomicBoolean; 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 @@ -2327,14 +2335,7 @@ import java.util.concurrent.atomic.AtomicBoolean; 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/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java index 54925c687d..4f0f802dca 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -60,6 +60,7 @@ import static androidx.media3.test.utils.TestUtil.timelinesAreSame; import static androidx.media3.test.utils.robolectric.RobolectricUtil.runMainLooperUntil; import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.playUntilPosition; import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.playUntilStartOfMediaItem; +import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.run; import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.runUntilError; import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled; import static androidx.media3.test.utils.robolectric.TestPlayerRunHelper.runUntilPlayWhenReady; @@ -13230,6 +13231,82 @@ public final class ExoPlayerTest { 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();