From 561dafcdeaef9e1c2caed9880accf5b8555f2f63 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 (cherry picked from commit 8b219b0ae69f80764183b680b44a8b0d6b03ed56) --- .../exoplayer/ExoPlayerImplInternal.java | 29 +++---- .../media3/exoplayer/ExoPlayerTest.java | 76 +++++++++++++++++++ 2 files changed, 91 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 27e9ed1496..4d9762d2cf 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; @@ -878,6 +877,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) { @@ -950,11 +952,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(); } } } @@ -1146,6 +1151,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 @@ -2324,14 +2332,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 fcaf562552..bae162aedb 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -13205,6 +13205,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. + playUntilPosition(player, /* 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. + playUntilStartOfMediaItem(player, /* mediaItemIndex= */ 1); + runUntilPendingCommandsAreFullyHandled(player); + @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. + playUntilPosition(player, /* mediaItemIndex= */ 0, /* positionMs= */ 5000); + // Pause in this "Read Ahead" state. + player.pause(); + runUntilPendingCommandsAreFullyHandled(player); + @Renderer.State int videoState1 = videoRenderer.getState(); + @Renderer.State int audioState1 = audioRenderer.getState(); + // Play in this "Read Ahead" state. + player.play(); + runUntilPendingCommandsAreFullyHandled(player); + @Renderer.State int videoState2 = videoRenderer.getState(); + @Renderer.State int audioState2 = audioRenderer.getState(); + // Play until the start of the second item. + playUntilStartOfMediaItem(player, /* mediaItemIndex= */ 1); + runUntilPendingCommandsAreFullyHandled(player); + @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();