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 93b8311894..02cf785eed 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 @@ -57,10 +57,12 @@ import com.google.android.exoplayer2.util.TraceUtil; import com.google.android.exoplayer2.util.Util; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Sets; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; /** Implements the internal behavior of {@link ExoPlayerImpl}. */ @@ -165,6 +167,7 @@ import java.util.concurrent.atomic.AtomicBoolean; private static final long MIN_RENDERER_SLEEP_DURATION_MS = 2000; private final Renderer[] renderers; + private final Set renderersToReset; private final RendererCapabilities[] rendererCapabilities; private final TrackSelector trackSelector; private final TrackSelectorResult emptyTrackSelectorResult; @@ -254,6 +257,7 @@ import java.util.concurrent.atomic.AtomicBoolean; } mediaClock = new DefaultMediaClock(this, clock); pendingMessages = new ArrayList<>(); + renderersToReset = Sets.newIdentityHashSet(); window = new Timeline.Window(); period = new Timeline.Period(); trackSelector.init(/* listener= */ this, bandwidthMeter); @@ -1322,7 +1326,7 @@ import java.util.concurrent.atomic.AtomicBoolean; this.foregroundMode = foregroundMode; if (!foregroundMode) { for (Renderer renderer : renderers) { - if (!isRendererEnabled(renderer)) { + if (!isRendererEnabled(renderer) && renderersToReset.remove(renderer)) { renderer.reset(); } } @@ -1382,11 +1386,13 @@ import java.util.concurrent.atomic.AtomicBoolean; } if (resetRenderers) { for (Renderer renderer : renderers) { - try { - renderer.reset(); - } catch (RuntimeException e) { - // There's nothing we can do. - Log.e(TAG, "Reset failed.", e); + if (renderersToReset.remove(renderer)) { + try { + renderer.reset(); + } catch (RuntimeException e) { + // There's nothing we can do. + Log.e(TAG, "Reset failed.", e); + } } } } @@ -2385,7 +2391,7 @@ import java.util.concurrent.atomic.AtomicBoolean; // Reset all disabled renderers before enabling any new ones. This makes sure resources released // by the disabled renderers will be available to renderers that are being enabled. for (int i = 0; i < renderers.length; i++) { - if (!trackSelectorResult.isRendererEnabled(i)) { + if (!trackSelectorResult.isRendererEnabled(i) && renderersToReset.remove(renderers[i])) { renderers[i].reset(); } } @@ -2417,6 +2423,7 @@ import java.util.concurrent.atomic.AtomicBoolean; boolean joining = !wasRendererEnabled && playing; // Enable the renderer. enabledRendererCount++; + renderersToReset.add(renderer); renderer.enable( rendererConfiguration, formats, @@ -2426,7 +2433,6 @@ import java.util.concurrent.atomic.AtomicBoolean; mayRenderStartOfStream, periodHolder.getStartPositionRendererTime(), periodHolder.getRendererOffset()); - renderer.handleMessage( Renderer.MSG_SET_WAKEUP_LISTENER, new Renderer.WakeupListener() { 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 e004a19a99..fab7b86a16 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 @@ -335,6 +335,146 @@ public final class ExoPlayerTest { assertThat(renderer.isEnded).isTrue(); } + @Test + public void renderersLifecycle_renderersThatAreNeverEnabled_areNotReset() throws Exception { + Timeline timeline = new FakeTimeline(); + final FakeRenderer videoRenderer = new FakeRenderer(C.TRACK_TYPE_VIDEO); + final FakeRenderer audioRenderer = new FakeRenderer(C.TRACK_TYPE_AUDIO); + SimpleExoPlayer player = + new TestExoPlayerBuilder(context).setRenderers(videoRenderer, audioRenderer).build(); + Player.Listener mockPlayerListener = mock(Player.Listener.class); + player.addListener(mockPlayerListener); + player.setMediaSource(new FakeMediaSource(timeline, ExoPlayerTestRunner.AUDIO_FORMAT)); + player.prepare(); + + player.play(); + runUntilPlaybackState(player, Player.STATE_ENDED); + player.release(); + + assertThat(audioRenderer.enabledCount).isEqualTo(1); + assertThat(audioRenderer.resetCount).isEqualTo(1); + assertThat(videoRenderer.enabledCount).isEqualTo(0); + assertThat(videoRenderer.resetCount).isEqualTo(0); + } + + @Test + public void renderersLifecycle_setForegroundMode_resetsDisabledRenderersThatHaveBeenEnabled() + throws Exception { + Timeline timeline = new FakeTimeline(); + final FakeRenderer videoRenderer = new FakeRenderer(C.TRACK_TYPE_VIDEO); + final FakeRenderer audioRenderer = new FakeRenderer(C.TRACK_TYPE_AUDIO); + final FakeRenderer textRenderer = new FakeRenderer(C.TRACK_TYPE_TEXT); + SimpleExoPlayer player = + new TestExoPlayerBuilder(context).setRenderers(videoRenderer, audioRenderer).build(); + Player.Listener mockPlayerListener = mock(Player.Listener.class); + player.addListener(mockPlayerListener); + player.setMediaSources( + ImmutableList.of( + new FakeMediaSource( + timeline, ExoPlayerTestRunner.AUDIO_FORMAT, ExoPlayerTestRunner.VIDEO_FORMAT), + new FakeMediaSource(timeline, ExoPlayerTestRunner.AUDIO_FORMAT))); + player.prepare(); + + player.play(); + runUntilPositionDiscontinuity(player, Player.DISCONTINUITY_REASON_AUTO_TRANSITION); + player.setForegroundMode(/* foregroundMode= */ true); + // Only the video renderer that is disabled in the second window has been reset. + assertThat(audioRenderer.resetCount).isEqualTo(0); + assertThat(videoRenderer.resetCount).isEqualTo(1); + + runUntilPlaybackState(player, Player.STATE_ENDED); + player.release(); + // After release the audio renderer is reset as well. + assertThat(audioRenderer.enabledCount).isEqualTo(1); + assertThat(audioRenderer.resetCount).isEqualTo(1); + assertThat(videoRenderer.enabledCount).isEqualTo(1); + assertThat(videoRenderer.resetCount).isEqualTo(1); + assertThat(textRenderer.enabledCount).isEqualTo(0); + assertThat(textRenderer.resetCount).isEqualTo(0); + } + + @Test + public void renderersLifecycle_selectTextTracksWhilePlaying_textRendererEnabledAndReset() + throws Exception { + Timeline timeline = new FakeTimeline(); + final FakeRenderer audioRenderer = new FakeRenderer(C.TRACK_TYPE_AUDIO); + final FakeRenderer videoRenderer = new FakeRenderer(C.TRACK_TYPE_VIDEO); + final FakeRenderer textRenderer = new FakeRenderer(C.TRACK_TYPE_TEXT); + Format textFormat = + new Format.Builder().setSampleMimeType(MimeTypes.TEXT_VTT).setLanguage("en").build(); + SimpleExoPlayer player = + new TestExoPlayerBuilder(context).setRenderers(audioRenderer, textRenderer).build(); + Player.Listener mockPlayerListener = mock(Player.Listener.class); + player.addListener(mockPlayerListener); + player.setMediaSources( + ImmutableList.of( + new FakeMediaSource(timeline, ExoPlayerTestRunner.AUDIO_FORMAT), + new FakeMediaSource(timeline, ExoPlayerTestRunner.AUDIO_FORMAT, textFormat))); + player.prepare(); + + player.play(); + runUntilPositionDiscontinuity(player, Player.DISCONTINUITY_REASON_AUTO_TRANSITION); + // Only the audio renderer enabled so far. + assertThat(audioRenderer.enabledCount).isEqualTo(1); + assertThat(textRenderer.enabledCount).isEqualTo(0); + player.setTrackSelectionParameters( + player.getTrackSelectionParameters().buildUpon().setPreferredTextLanguage("en").build()); + runUntilPlaybackState(player, Player.STATE_ENDED); + player.release(); + + assertThat(audioRenderer.enabledCount).isEqualTo(1); + assertThat(audioRenderer.resetCount).isEqualTo(1); + assertThat(textRenderer.enabledCount).isEqualTo(1); + assertThat(textRenderer.resetCount).isEqualTo(1); + assertThat(videoRenderer.enabledCount).isEqualTo(0); + assertThat(videoRenderer.resetCount).isEqualTo(0); + } + + @Test + public void renderersLifecycle_seekTo_resetsDisabledRenderersIfRequired() throws Exception { + Timeline timeline = new FakeTimeline(); + final FakeRenderer audioRenderer = new FakeRenderer(C.TRACK_TYPE_AUDIO); + final FakeRenderer videoRenderer = new FakeRenderer(C.TRACK_TYPE_VIDEO); + final FakeRenderer textRenderer = new FakeRenderer(C.TRACK_TYPE_TEXT); + Format textFormat = + new Format.Builder().setSampleMimeType(MimeTypes.TEXT_VTT).setLanguage("en").build(); + SimpleExoPlayer player = + new TestExoPlayerBuilder(context) + .setRenderers(videoRenderer, audioRenderer, textRenderer) + .build(); + Player.Listener mockPlayerListener = mock(Player.Listener.class); + player.addListener(mockPlayerListener); + player.setTrackSelectionParameters( + player.getTrackSelectionParameters().buildUpon().setPreferredTextLanguage("en").build()); + player.setMediaSources( + ImmutableList.of( + new FakeMediaSource(timeline, ExoPlayerTestRunner.AUDIO_FORMAT), + new FakeMediaSource(timeline, ExoPlayerTestRunner.AUDIO_FORMAT, textFormat))); + player.prepare(); + + player.play(); + runUntilPositionDiscontinuity(player, Player.DISCONTINUITY_REASON_AUTO_TRANSITION); + // Disable text renderer by selecting a language that is not available. + player.setTrackSelectionParameters( + player.getTrackSelectionParameters().buildUpon().setPreferredTextLanguage("de").build()); + player.seekTo(/* windowIndex= */ 0, /* positionMs= */ 1000); + runUntilPlaybackState(player, Player.STATE_READY); + // Expect formerly enabled renderers to be reset after seek. + assertThat(textRenderer.resetCount).isEqualTo(1); + assertThat(audioRenderer.resetCount).isEqualTo(0); + assertThat(videoRenderer.resetCount).isEqualTo(0); + runUntilPlaybackState(player, Player.STATE_ENDED); + player.release(); + + // Verify that the text renderer has not been reset a second time. + assertThat(audioRenderer.enabledCount).isEqualTo(2); + assertThat(audioRenderer.resetCount).isEqualTo(1); + assertThat(textRenderer.enabledCount).isEqualTo(1); + assertThat(textRenderer.resetCount).isEqualTo(1); + assertThat(videoRenderer.enabledCount).isEqualTo(0); + assertThat(videoRenderer.resetCount).isEqualTo(0); + } + /** * Tests that the player does not unnecessarily reset renderers when playing a multi-period * source. diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeRenderer.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeRenderer.java index 3aab64eab6..1018207de1 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeRenderer.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeRenderer.java @@ -61,6 +61,8 @@ public class FakeRenderer extends BaseRenderer { public boolean isEnded; public int positionResetCount; public int sampleBufferReadCount; + public int enabledCount; + public int resetCount; public FakeRenderer(@C.TrackType int trackType) { super(trackType); @@ -136,6 +138,17 @@ public class FakeRenderer extends BaseRenderer { } } + @Override + protected void onEnabled(boolean joining, boolean mayRenderStartOfStream) + throws ExoPlaybackException { + enabledCount++; + } + + @Override + protected void onReset() { + resetCount++; + } + @Override public boolean isReady() { return lastSamplePositionUs >= playbackPositionUs || hasPendingBuffer || isSourceReady();