Implement the rest of WebVTT's line computation

Steps 4-10 of https://www.w3.org/TR/webvtt1/#cue-computed-line

This part is harder to fit into our code structure because it depends on
how many cues are simultaneously visible - so it has to go in
WebvttSubtitle not WebvttCueParser (which only deals with individual
cues in isolation).

This removes the `isNormal()` method that was trying to approximate
the correct behaviour.

PiperOrigin-RevId: 309021686

RELEASENOTES.md

@@ -109,6 +109,8 @@
         seconds ([#7181](https://github.com/google/ExoPlayer/issues/7181)).
     *   Add special-case positioning behaviour for vertical cues being rendered
         horizontally.
+    *   Implement steps 4-10 of the
+        [WebVTT line computation algorithm](https://www.w3.org/TR/webvtt1/#cue-computed-line).
 *   DRM:
     *   Add support for attaching DRM sessions to clear content in the demo app.
     *   Remove `DrmSessionManager` references from all renderers.

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

@@ -15,7 +15,6 @@
  */
 package com.google.android.exoplayer2.text.webvtt;
 
-import android.text.SpannableStringBuilder;
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.text.Cue;
 import com.google.android.exoplayer2.text.Subtitle;
@@ -23,6 +22,7 @@ import com.google.android.exoplayer2.util.Assertions;
 import com.google.android.exoplayer2.util.Util;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -30,17 +30,16 @@ import java.util.List;
  */
 /* package */ final class WebvttSubtitle implements Subtitle {
 
-  private final List<Cue> cues;
+  private final List<WebvttCueInfo> cueInfos;
   private final long[] cueTimesUs;
   private final long[] sortedCueTimesUs;
 
   /** Constructs a new WebvttSubtitle from a list of {@link WebvttCueInfo}s. */
   public WebvttSubtitle(List<WebvttCueInfo> cueInfos) {
-    this.cues = new ArrayList<>(cueInfos.size());
+    this.cueInfos = Collections.unmodifiableList(new ArrayList<>(cueInfos));
     cueTimesUs = new long[2 * cueInfos.size()];
     for (int cueIndex = 0; cueIndex < cueInfos.size(); cueIndex++) {
       WebvttCueInfo cueInfo = cueInfos.get(cueIndex);
-      this.cues.add(cueInfo.cue);
       int arrayIndex = cueIndex * 2;
       cueTimesUs[arrayIndex] = cueInfo.startTimeUs;
       cueTimesUs[arrayIndex + 1] = cueInfo.endTimeUs;
@@ -69,53 +68,34 @@ import java.util.List;
 
   @Override
   public List<Cue> getCues(long timeUs) {
-    List<Cue> list = new ArrayList<>();
+    List<Cue> currentCues = new ArrayList<>();
-    Cue firstNormalCue = null;
+    List<WebvttCueInfo> cuesWithUnsetLine = new ArrayList<>();
-    SpannableStringBuilder normalCueTextBuilder = null;
+    for (int i = 0; i < cueInfos.size(); i++) {
-
-    for (int i = 0; i < cues.size(); i++) {
       if ((cueTimesUs[i * 2] <= timeUs) && (timeUs < cueTimesUs[i * 2 + 1])) {
-        Cue cue = cues.get(i);
+        WebvttCueInfo cueInfo = cueInfos.get(i);
-        // TODO(ibaker): Replace this with a closer implementation of the WebVTT spec (keeping
+        if (cueInfo.cue.line == Cue.DIMEN_UNSET) {
-        // individual cues, but tweaking their `line` value):
+          cuesWithUnsetLine.add(cueInfo);
-        // https://www.w3.org/TR/webvtt1/#cue-computed-line
-        if (isNormal(cue)) {
-          // We want to merge all of the normal cues into a single cue to ensure they are drawn
-          // correctly (i.e. don't overlap) and to emulate roll-up, but only if there are multiple
-          // normal cues, otherwise we can just append the single normal cue.
-          if (firstNormalCue == null) {
-            firstNormalCue = cue;
-          } else if (normalCueTextBuilder == null) {
-            normalCueTextBuilder = new SpannableStringBuilder();
-            normalCueTextBuilder
-                .append(Assertions.checkNotNull(firstNormalCue.text))
-                .append("\n")
-                .append(Assertions.checkNotNull(cue.text));
-          } else {
-            normalCueTextBuilder.append("\n").append(Assertions.checkNotNull(cue.text));
-          }
         } else {
-          list.add(cue);
+          currentCues.add(cueInfo.cue);
         }
       }
     }
-    if (normalCueTextBuilder != null) {
+    // Steps 4 - 10 of https://www.w3.org/TR/webvtt1/#cue-computed-line
-      // There were multiple normal cues, so create a new cue with all of the text.
+    // (steps 1 - 3 are handled by WebvttCueParser#computeLine(float, int))
-      list.add(WebvttCueParser.newCueForText(normalCueTextBuilder));
+    Collections.sort(cuesWithUnsetLine, (c1, c2) -> Long.compare(c1.startTimeUs, c2.startTimeUs));
-    } else if (firstNormalCue != null) {
+    for (int i = 0; i < cuesWithUnsetLine.size(); i++) {
-      // There was only a single normal cue, so just add it to the list.
+      Cue cue = cuesWithUnsetLine.get(i).cue;
-      list.add(firstNormalCue);
+      currentCues.add(
+          cue.buildUpon()
+              .setLine((float) (-1 - i), Cue.LINE_TYPE_NUMBER)
+              // WebVTT doesn't use 'line alignment' (i.e. Cue#lineAnchor) when computing position
+              // with snap-to-lines=true (i.e. Cue#LINE_TYPE_NUMBER) but Cue does use lineAnchor
+              // when describing how numeric cues should be displayed. So we have to manually set
+              // lineAnchor=ANCHOR_TYPE_END to avoid the bottom line of cues being off the screen.
+              // https://www.w3.org/TR/webvtt1/#processing-cue-settings
+              .setLineAnchor(Cue.ANCHOR_TYPE_END)
+              .build());
     }
-    return list;
+    return currentCues;
-  }
-
-  /**
-   * Returns whether or not this cue should be placed in the default position and rolled-up with the
-   * other "normal" cues.
-   *
-   * @return Whether this cue should be placed in the default position.
-   */
-  private static boolean isNormal(Cue cue) {
-    return (cue.line == Cue.DIMEN_UNSET && cue.position == WebvttCueParser.DEFAULT_POSITION);
   }
 }

library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttDecoderTest.java

@@ -31,6 +31,7 @@ import com.google.android.exoplayer2.util.ColorParser;
 import com.google.common.collect.Iterables;
 import com.google.common.truth.Expect;
 import java.io.IOException;
+import java.util.List;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -44,6 +45,8 @@ public class WebvttDecoderTest {
   private static final String TYPICAL_WITH_IDS_FILE = "webvtt/typical_with_identifiers";
   private static final String TYPICAL_WITH_COMMENTS_FILE = "webvtt/typical_with_comments";
   private static final String WITH_POSITIONING_FILE = "webvtt/with_positioning";
+  private static final String WITH_OVERLAPPING_TIMESTAMPS_FILE =
+      "webvtt/with_overlapping_timestamps";
   private static final String WITH_VERTICAL_FILE = "webvtt/with_vertical";
   private static final String WITH_BAD_CUE_HEADER_FILE = "webvtt/with_bad_cue_header";
   private static final String WITH_TAGS_FILE = "webvtt/with_tags";
@@ -194,9 +197,14 @@ public class WebvttDecoderTest {
     assertThat(firstCue.positionAnchor).isEqualTo(Cue.ANCHOR_TYPE_END);
     assertThat(firstCue.textAlignment).isEqualTo(Alignment.ALIGN_NORMAL);
     assertThat(firstCue.size).isEqualTo(0.35f);
+
     // Unspecified values should use WebVTT defaults
-    assertThat(firstCue.line).isEqualTo(Cue.DIMEN_UNSET);
+    assertThat(firstCue.line).isEqualTo(-1f);
     assertThat(firstCue.lineType).isEqualTo(Cue.LINE_TYPE_NUMBER);
+    // WebVTT specifies START as the default, but it doesn't expect this to be used if
+    // lineType=NUMBER so we have to override it to END in this case, otherwise the Cue will be
+    // displayed off the bottom of the screen.
+    assertThat(firstCue.lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END);
     assertThat(firstCue.verticalType).isEqualTo(Cue.TYPE_UNSET);
 
     assertThat(subtitle.getEventTime(2)).isEqualTo(2_345_000L);
@@ -260,6 +268,58 @@ public class WebvttDecoderTest {
     assertThat(eighthCue.positionAnchor).isEqualTo(Cue.ANCHOR_TYPE_END);
   }
 
+  @Test
+  public void decodeWithOverlappingTimestamps() throws Exception {
+    WebvttSubtitle subtitle = getSubtitleForTestAsset(WITH_OVERLAPPING_TIMESTAMPS_FILE);
+
+    assertThat(subtitle.getEventTimeCount()).isEqualTo(8);
+
+    Cue firstCue = Iterables.getOnlyElement(subtitle.getCues(subtitle.getEventTime(0)));
+    assertThat(firstCue.text.toString()).isEqualTo("Displayed at the bottom for 3 seconds.");
+    assertThat(firstCue.line).isEqualTo(-1f);
+    assertThat(firstCue.lineType).isEqualTo(Cue.LINE_TYPE_NUMBER);
+    assertThat(firstCue.lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END);
+
+    List<Cue> firstAndSecondCue = subtitle.getCues(subtitle.getEventTime(1));
+    assertThat(firstAndSecondCue).hasSize(2);
+    assertThat(firstAndSecondCue.get(0).text.toString())
+        .isEqualTo("Displayed at the bottom for 3 seconds.");
+    assertThat(firstAndSecondCue.get(0).line).isEqualTo(-1f);
+    assertThat(firstAndSecondCue.get(0).lineType).isEqualTo(Cue.LINE_TYPE_NUMBER);
+    assertThat(firstAndSecondCue.get(0).lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END);
+    assertThat(firstAndSecondCue.get(1).text.toString())
+        .isEqualTo("Appears directly above for 1 second.");
+    assertThat(firstAndSecondCue.get(1).line).isEqualTo(-2f);
+    assertThat(firstAndSecondCue.get(1).lineType).isEqualTo(Cue.LINE_TYPE_NUMBER);
+    assertThat(firstAndSecondCue.get(1).lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END);
+
+    Cue thirdCue = Iterables.getOnlyElement(subtitle.getCues(subtitle.getEventTime(4)));
+    assertThat(thirdCue.text.toString()).isEqualTo("Displayed at the bottom for 2 seconds.");
+    assertThat(thirdCue.line).isEqualTo(-1f);
+    assertThat(thirdCue.lineType).isEqualTo(Cue.LINE_TYPE_NUMBER);
+    assertThat(thirdCue.lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END);
+
+    List<Cue> thirdAndFourthCue = subtitle.getCues(subtitle.getEventTime(5));
+    assertThat(thirdAndFourthCue).hasSize(2);
+    assertThat(thirdAndFourthCue.get(0).text.toString())
+        .isEqualTo("Displayed at the bottom for 2 seconds.");
+    assertThat(thirdAndFourthCue.get(0).line).isEqualTo(-1f);
+    assertThat(thirdAndFourthCue.get(0).lineType).isEqualTo(Cue.LINE_TYPE_NUMBER);
+    assertThat(thirdAndFourthCue.get(0).lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END);
+    assertThat(thirdAndFourthCue.get(1).text.toString())
+        .isEqualTo("Appears directly above the previous cue, then replaces it after 1 second.");
+    assertThat(thirdAndFourthCue.get(1).line).isEqualTo(-2f);
+    assertThat(thirdAndFourthCue.get(1).lineType).isEqualTo(Cue.LINE_TYPE_NUMBER);
+    assertThat(thirdAndFourthCue.get(1).lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END);
+
+    Cue fourthCue = Iterables.getOnlyElement(subtitle.getCues(subtitle.getEventTime(6)));
+    assertThat(fourthCue.text.toString())
+        .isEqualTo("Appears directly above the previous cue, then replaces it after 1 second.");
+    assertThat(fourthCue.line).isEqualTo(-1f);
+    assertThat(fourthCue.lineType).isEqualTo(Cue.LINE_TYPE_NUMBER);
+    assertThat(fourthCue.lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END);
+  }
+
   @Test
   public void decodeWithVertical() throws Exception {
     WebvttSubtitle subtitle = getSubtitleForTestAsset(WITH_VERTICAL_FILE);

library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttSubtitleTest.java

@@ -21,6 +21,7 @@ import static java.lang.Long.MAX_VALUE;
 
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import com.google.android.exoplayer2.text.Cue;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -33,8 +34,6 @@ public class WebvttSubtitleTest {
 
   private static final String FIRST_SUBTITLE_STRING = "This is the first subtitle.";
   private static final String SECOND_SUBTITLE_STRING = "This is the second subtitle.";
-  private static final String FIRST_AND_SECOND_SUBTITLE_STRING =
-      FIRST_SUBTITLE_STRING + "\n" + SECOND_SUBTITLE_STRING;
 
   private static final WebvttSubtitle emptySubtitle = new WebvttSubtitle(Collections.emptyList());
 
@@ -143,12 +142,12 @@ public class WebvttSubtitleTest {
     assertSingleCueTextEquals(FIRST_SUBTITLE_STRING, overlappingSubtitle.getCues(1_999_999));
 
     // Test after first and second subtitle
-    assertSingleCueTextEquals(
+    assertThat(getCueTexts(overlappingSubtitle.getCues(2_000_000)))
-        FIRST_AND_SECOND_SUBTITLE_STRING, overlappingSubtitle.getCues(2_000_000));
+        .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING);
-    assertSingleCueTextEquals(
+    assertThat(getCueTexts(overlappingSubtitle.getCues(2_500_000)))
-        FIRST_AND_SECOND_SUBTITLE_STRING, overlappingSubtitle.getCues(2_500_000));
+        .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING);
-    assertSingleCueTextEquals(
+    assertThat(getCueTexts(overlappingSubtitle.getCues(2_999_999)))
-        FIRST_AND_SECOND_SUBTITLE_STRING, overlappingSubtitle.getCues(2_999_999));
+        .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING);
 
     // Test second subtitle
     assertSingleCueTextEquals(SECOND_SUBTITLE_STRING, overlappingSubtitle.getCues(3_000_000));
@@ -184,9 +183,12 @@ public class WebvttSubtitleTest {
     assertSingleCueTextEquals(FIRST_SUBTITLE_STRING, nestedSubtitle.getCues(1_999_999));
 
     // Test after first and second subtitle
-    assertSingleCueTextEquals(FIRST_AND_SECOND_SUBTITLE_STRING, nestedSubtitle.getCues(2_000_000));
+    assertThat(getCueTexts(nestedSubtitle.getCues(2_000_000)))
-    assertSingleCueTextEquals(FIRST_AND_SECOND_SUBTITLE_STRING, nestedSubtitle.getCues(2_500_000));
+        .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING);
-    assertSingleCueTextEquals(FIRST_AND_SECOND_SUBTITLE_STRING, nestedSubtitle.getCues(2_999_999));
+    assertThat(getCueTexts(nestedSubtitle.getCues(2_500_000)))
+        .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING);
+    assertThat(getCueTexts(nestedSubtitle.getCues(2_999_999)))
+        .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING);
 
     // Test first subtitle
     assertSingleCueTextEquals(FIRST_SUBTITLE_STRING, nestedSubtitle.getCues(3_000_000));
@@ -241,4 +243,12 @@ public class WebvttSubtitleTest {
     assertThat(cues).hasSize(1);
     assertThat(cues.get(0).text.toString()).isEqualTo(expected);
   }
+
+  private static List<String> getCueTexts(List<Cue> cues) {
+    List<String> cueTexts = new ArrayList<>();
+    for (int i = 0; i < cues.size(); i++) {
+      cueTexts.add(cues.get(i).text.toString());
+    }
+    return cueTexts;
+  }
 }

testdata/src/test/assets/webvtt/with_overlapping_timestamps

@@ -0,0 +1,13 @@
+WEBVTT
+
+00:00:00.000 --> 00:00:03.000
+Displayed at the bottom for 3 seconds.
+
+00:01.000 --> 00:02.000
+Appears directly above for 1 second.
+
+00:04.000 --> 00:06.000
+Displayed at the bottom for 2 seconds.
+
+00:05.000 --> 00:07.000
+Appears directly above the previous cue, then replaces it after 1 second.