Pass the end position of a stream to TextRenderer when marking it final

In a period transition we pass the start time of the next period, for
the final period we pass the duration of the period or timeline, if
known.

This means sideloaded subtitles now respect the end point of
ClippingMediaSource and ensures that content subtitles aren't
incorrectly displayed over mid-roll ads.

When transitioning back into the subtitled content the subtitles still
appear slightly before the video transitions, meaning the first subtitle
of the content is shown with the last few frames of the ad. Resolving
this in a way that doesn't break anything else requires a deeper
investigation.

Issue: #5317
Issue: #8456
#minor-release
PiperOrigin-RevId: 361797118

RELEASENOTES.md

@@ -49,13 +49,20 @@
 *   Text
     *   Parse SSA/ASS bold & italic info in `Style:` lines
         ([#8435](https://github.com/google/ExoPlayer/issues/8435)).
+*   Text:
+    *   Don't display subtitles after the end position of the current media
+        period (if known). This ensures sideloaded subtitles respect the end
+        point of `ClippingMediaPeriod` and prevents content subtitles from
+        continuing to be displayed over mid-roll ads
+        ([#5317](https://github.com/google/ExoPlayer/issues/5317),
+        [#8456](https://github.com/google/ExoPlayer/issues/8456)).
 *   MediaSession extension: Remove dependency to core module and rely on common
     only. The `TimelineQueueEditor` uses a new `MediaDescriptionConverter` for
     this purpose and does not rely on the `ConcatenatingMediaSource` anymore.
 *   Cast extension:
-    *  Fix `onPositionDiscontinuity` event so that it is not triggered with
+    *   Fix `onPositionDiscontinuity` event so that it is not triggered with
-       reason `DISCONTINUITY_REASON_PERIOD_TRANSITION` after a seek to another
+        reason `DISCONTINUITY_REASON_PERIOD_TRANSITION` after a seek to another
-       media item and so that it is not triggered after a timeline change.
+        media item and so that it is not triggered after a timeline change.
 
 ### 2.13.2 (2021-02-25)
 

library/core/src/androidTest/java/com/google/android/exoplayer2/ClippedPlaybackTest.java

@@ -0,0 +1,155 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.android.exoplayer2;
+
+import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
+import static com.google.common.truth.Truth.assertThat;
+
+import android.net.Uri;
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+import com.google.android.exoplayer2.source.ClippingMediaSource;
+import com.google.android.exoplayer2.text.Cue;
+import com.google.android.exoplayer2.text.TextOutput;
+import com.google.android.exoplayer2.util.ConditionVariable;
+import com.google.android.exoplayer2.util.MimeTypes;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/**
+ * Instrumentation tests for playback of clipped items using {@link MediaItem#clippingProperties} or
+ * {@link ClippingMediaSource} directly.
+ */
+@RunWith(AndroidJUnit4.class)
+public final class ClippedPlaybackTest {
+
+  @Test
+  public void subtitlesRespectClipping_singlePeriod() throws Exception {
+    MediaItem mediaItem =
+        new MediaItem.Builder()
+            .setUri("asset:///media/mp4/sample.mp4")
+            .setSubtitles(
+                ImmutableList.of(
+                    new MediaItem.Subtitle(
+                        Uri.parse("asset:///media/webvtt/typical"),
+                        MimeTypes.TEXT_VTT,
+                        "en",
+                        C.SELECTION_FLAG_DEFAULT)))
+            // Expect the clipping to affect both subtitles and video.
+            .setClipEndPositionMs(1000)
+            .build();
+    AtomicReference<SimpleExoPlayer> player = new AtomicReference<>();
+    CapturingTextOutput textOutput = new CapturingTextOutput();
+    ConditionVariable playbackEnded = new ConditionVariable();
+    getInstrumentation()
+        .runOnMainSync(
+            () -> {
+              player.set(new SimpleExoPlayer.Builder(getInstrumentation().getContext()).build());
+              player.get().addTextOutput(textOutput);
+              player
+                  .get()
+                  .addListener(
+                      new Player.EventListener() {
+                        @Override
+                        public void onPlaybackStateChanged(@Player.State int state) {
+                          if (state == Player.STATE_ENDED) {
+                            playbackEnded.open();
+                          }
+                        }
+                      });
+              player.get().setMediaItem(mediaItem);
+              player.get().prepare();
+              player.get().play();
+            });
+
+    playbackEnded.block();
+
+    getInstrumentation().runOnMainSync(() -> player.get().release());
+    assertThat(Iterables.getOnlyElement(Iterables.concat(textOutput.cues)).text.toString())
+        .isEqualTo("This is the first subtitle.");
+  }
+
+  @Test
+  public void subtitlesRespectClipping_multiplePeriods() throws Exception {
+    ImmutableList<MediaItem> mediaItems =
+        ImmutableList.of(
+            new MediaItem.Builder()
+                .setUri("asset:///media/mp4/sample.mp4")
+                .setSubtitles(
+                    ImmutableList.of(
+                        new MediaItem.Subtitle(
+                            Uri.parse("asset:///media/webvtt/typical"),
+                            MimeTypes.TEXT_VTT,
+                            "en",
+                            C.SELECTION_FLAG_DEFAULT)))
+                // Expect the clipping to affect both subtitles and video.
+                .setClipEndPositionMs(1000)
+                .build(),
+            new MediaItem.Builder()
+                .setUri("asset:///media/mp4/sample.mp4")
+                // Not needed for correctness, just makes test run faster. Must be longer than the
+                // subtitle content (3.5s).
+                .setClipEndPositionMs(4_000)
+                .build());
+    AtomicReference<SimpleExoPlayer> player = new AtomicReference<>();
+    CapturingTextOutput textOutput = new CapturingTextOutput();
+    ConditionVariable playbackEnded = new ConditionVariable();
+    getInstrumentation()
+        .runOnMainSync(
+            () -> {
+              player.set(new SimpleExoPlayer.Builder(getInstrumentation().getContext()).build());
+              player.get().addTextOutput(textOutput);
+              player
+                  .get()
+                  .addListener(
+                      new Player.EventListener() {
+                        @Override
+                        public void onPlaybackStateChanged(@Player.State int state) {
+                          if (state == Player.STATE_ENDED) {
+                            playbackEnded.open();
+                          }
+                        }
+                      });
+              player.get().setMediaItems(mediaItems);
+              player.get().prepare();
+              player.get().play();
+            });
+
+    playbackEnded.block();
+
+    getInstrumentation().runOnMainSync(() -> player.get().release());
+    assertThat(Iterables.getOnlyElement(Iterables.concat(textOutput.cues)).text.toString())
+        .isEqualTo("This is the first subtitle.");
+  }
+
+  private static class CapturingTextOutput implements TextOutput {
+
+    private final List<List<Cue>> cues;
+
+    private CapturingTextOutput() {
+      cues = new ArrayList<>();
+    }
+
+    @Override
+    public void onCues(List<Cue> cues) {
+      this.cues.add(cues);
+    }
+  }
+}

library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java

@@ -40,6 +40,7 @@ import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId;
 import com.google.android.exoplayer2.source.SampleStream;
 import com.google.android.exoplayer2.source.ShuffleOrder;
 import com.google.android.exoplayer2.source.TrackGroupArray;
+import com.google.android.exoplayer2.text.TextRenderer;
 import com.google.android.exoplayer2.trackselection.ExoTrackSelection;
 import com.google.android.exoplayer2.trackselection.TrackSelector;
 import com.google.android.exoplayer2.trackselection.TrackSelectorResult;
@@ -1932,7 +1933,12 @@ import java.util.concurrent.atomic.AtomicBoolean;
           if (sampleStream != null
               && renderer.getStream() == sampleStream
               && renderer.hasReadStreamToEnd()) {
-            renderer.setCurrentStreamFinal();
+            long streamEndPositionUs =
+                readingPeriodHolder.info.durationUs != C.TIME_UNSET
+                        && readingPeriodHolder.info.durationUs != C.TIME_END_OF_SOURCE
+                    ? readingPeriodHolder.getRendererOffset() + readingPeriodHolder.info.durationUs
+                    : C.TIME_UNSET;
+            setCurrentStreamFinal(renderer, streamEndPositionUs);
           }
         }
       }
@@ -1957,7 +1963,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
         && readingPeriodHolder.mediaPeriod.readDiscontinuity() != C.TIME_UNSET) {
       // The new period starts with a discontinuity, so the renderers will play out all data, then
       // be disabled and re-enabled when they start playing the next period.
-      setAllRendererStreamsFinal();
+      setAllRendererStreamsFinal(
+          /* streamEndPositionUs= */ readingPeriodHolder.getStartPositionRendererTime());
       return;
     }
     for (int i = 0; i < renderers.length; i++) {
@@ -1973,7 +1980,9 @@ import java.util.concurrent.atomic.AtomicBoolean;
           // it's a no-sample renderer for which rendererOffsetUs should be updated only when
           // starting to play the next period. Mark the SampleStream as final to play out any
           // remaining data.
-          renderers[i].setCurrentStreamFinal();
+          setCurrentStreamFinal(
+              renderers[i],
+              /* streamEndPositionUs= */ readingPeriodHolder.getStartPositionRendererTime());
         }
       }
     }
@@ -2098,14 +2107,21 @@ import java.util.concurrent.atomic.AtomicBoolean;
     return true;
   }
 
-  private void setAllRendererStreamsFinal() {
+  private void setAllRendererStreamsFinal(long streamEndPositionUs) {
     for (Renderer renderer : renderers) {
       if (renderer.getStream() != null) {
-        renderer.setCurrentStreamFinal();
+        setCurrentStreamFinal(renderer, streamEndPositionUs);
       }
     }
   }
 
+  private void setCurrentStreamFinal(Renderer renderer, long streamEndPositionUs) {
+    renderer.setCurrentStreamFinal();
+    if (renderer instanceof TextRenderer) {
+      ((TextRenderer) renderer).setFinalStreamEndPositionUs(streamEndPositionUs);
+    }
+  }
+
   private void handlePeriodPrepared(MediaPeriod mediaPeriod) throws ExoPlaybackException {
     if (!queue.isLoading(mediaPeriod)) {
       // Stale event.

library/core/src/main/java/com/google/android/exoplayer2/text/TextRenderer.java

@@ -16,6 +16,7 @@
 package com.google.android.exoplayer2.text;
 
 import static com.google.android.exoplayer2.util.Assertions.checkNotNull;
+import static com.google.android.exoplayer2.util.Assertions.checkState;
 
 import android.os.Handler;
 import android.os.Handler.Callback;
@@ -91,6 +92,7 @@ public final class TextRenderer extends BaseRenderer implements Callback {
   @Nullable private SubtitleOutputBuffer subtitle;
   @Nullable private SubtitleOutputBuffer nextSubtitle;
   private int nextSubtitleEventIndex;
+  private long finalStreamEndPositionUs;
 
   /**
    * @param output The output.
@@ -121,6 +123,7 @@ public final class TextRenderer extends BaseRenderer implements Callback {
         outputLooper == null ? null : Util.createHandler(outputLooper, /* callback= */ this);
     this.decoderFactory = decoderFactory;
     formatHolder = new FormatHolder();
+    finalStreamEndPositionUs = C.TIME_UNSET;
   }
 
   @Override
@@ -141,6 +144,21 @@ public final class TextRenderer extends BaseRenderer implements Callback {
     }
   }
 
+  /**
+   * Sets the position at which to stop rendering the current stream.
+   *
+   * <p>Must be called after {@link #setCurrentStreamFinal()}.
+   *
+   * @param streamEndPositionUs The position to stop rendering at or {@link C#LENGTH_UNSET} to
+   *     render until the end of the current stream.
+   */
+  // TODO(internal b/181312195): Remove this when it's no longer needed once subtitles are decoded
+  // on the loading side of SampleQueue.
+  public void setFinalStreamEndPositionUs(long streamEndPositionUs) {
+    checkState(isCurrentStreamFinal());
+    this.finalStreamEndPositionUs = streamEndPositionUs;
+  }
+
   @Override
   protected void onStreamChanged(Format[] formats, long startPositionUs, long offsetUs) {
     streamFormat = formats[0];
@@ -156,6 +174,7 @@ public final class TextRenderer extends BaseRenderer implements Callback {
     clearOutput();
     inputStreamEnded = false;
     outputStreamEnded = false;
+    finalStreamEndPositionUs = C.TIME_UNSET;
     if (decoderReplacementState != REPLACEMENT_STATE_NONE) {
       replaceDecoder();
     } else {
@@ -166,6 +185,13 @@ public final class TextRenderer extends BaseRenderer implements Callback {
 
   @Override
   public void render(long positionUs, long elapsedRealtimeUs) {
+    if (isCurrentStreamFinal()
+        && finalStreamEndPositionUs != C.TIME_UNSET
+        && positionUs >= finalStreamEndPositionUs) {
+      releaseBuffers();
+      outputStreamEnded = true;
+    }
+
     if (outputStreamEnded) {
       return;
     }
@@ -278,6 +304,7 @@ public final class TextRenderer extends BaseRenderer implements Callback {
   @Override
   protected void onDisabled() {
     streamFormat = null;
+    finalStreamEndPositionUs = C.TIME_UNSET;
     clearOutput();
     releaseDecoder();
   }