From 0c982a7994444ce9355ff6b43de39cff2f901013 Mon Sep 17 00:00:00 2001 From: michaelkatz Date: Mon, 11 Nov 2024 04:33:05 -0800 Subject: [PATCH] Fix error where non-enabled renderers were set to final If the renderer has no stream, then it should not be called with `setCurrentStreamFinal` on it. PiperOrigin-RevId: 695284225 --- .../media3/exoplayer/RendererHolder.java | 4 +- .../media3/exoplayer/ExoPlayerTest.java | 44 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererHolder.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererHolder.java index 783310996f..4efe3b2e3f 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererHolder.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererHolder.java @@ -92,7 +92,9 @@ import java.io.IOException; * render until the end of the current stream. */ public void setCurrentStreamFinal(long streamEndPositionUs) { - setCurrentStreamFinal(renderer, streamEndPositionUs); + if (renderer.getStream() != null) { + setCurrentStreamFinal(renderer, streamEndPositionUs); + } } private void setCurrentStreamFinal(Renderer renderer, long streamEndPositionUs) { 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 615845e144..b5a1d2c29f 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -673,6 +673,50 @@ public class ExoPlayerTest { assertThat(videoRenderer.resetCount).isEqualTo(0); } + @Test + public void renderersLifecycle_onlyRenderersThatAreEnabled_areSetToFinal() throws Exception { + AtomicInteger videoStreamSetToFinalCount = new AtomicInteger(); + final FakeRenderer videoRenderer = new FakeRenderer(C.TRACK_TYPE_VIDEO); + final FakeRenderer audioRenderer = new FakeRenderer(C.TRACK_TYPE_AUDIO); + final ForwardingRenderer forwardingVideoRenderer = + new ForwardingRenderer(videoRenderer) { + @Override + public void setCurrentStreamFinal() { + super.setCurrentStreamFinal(); + videoStreamSetToFinalCount.getAndIncrement(); + } + }; + ExoPlayer player = + parameterizeTestExoPlayerBuilder( + new TestExoPlayerBuilder(context) + .setRenderers(forwardingVideoRenderer, audioRenderer)) + .build(); + // Use media sources with discontinuities so that enabled streams are set to final. + ClippingMediaSource clippedFakeAudioSource = + new ClippingMediaSource( + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.AUDIO_FORMAT), 0, 300_000L); + ClippingMediaSource clippedFakeAudioVideoSource = + new ClippingMediaSource( + new FakeMediaSource( + new FakeTimeline(), + ExoPlayerTestRunner.VIDEO_FORMAT, + ExoPlayerTestRunner.AUDIO_FORMAT), + 0, + 300_000L); + player.setMediaSources( + ImmutableList.of( + clippedFakeAudioSource, clippedFakeAudioVideoSource, clippedFakeAudioSource)); + player.prepare(); + + player.play(); + runUntilPlaybackState(player, Player.STATE_ENDED); + player.release(); + + assertThat(audioRenderer.enabledCount).isEqualTo(3); + assertThat(videoRenderer.enabledCount).isEqualTo(1); + assertThat(videoStreamSetToFinalCount.get()).isEqualTo(1); + } + /** * Tests that the player does not unnecessarily reset renderers when playing a multi-period * source.