Reset only renderers that have been enabled

#exofixit

PiperOrigin-RevId: 396938258

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<Renderer> 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 {
+        if (renderersToReset.remove(renderer)) {
-          renderer.reset();
+          try {
-        } catch (RuntimeException e) {
+            renderer.reset();
-          // There's nothing we can do.
+          } catch (RuntimeException e) {
-          Log.e(TAG, "Reset failed.", 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() {

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.

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();