Fix IndexOutOfBoundsException in LegacySubtitleUtil

This is caused when the requested "output start time" is equal to or larger than the last event time in a `Subtitle` object. This resolves the error in Issue: #1516, but subtitles are still not renderered (probably because the timestamps aren't what we expect somewhere, but I need to investigate this part further). #cherrypick PiperOrigin-RevId: 660462720
androidx · Aug 7, 2024 · 3763e5b · 3763e5b
1 parent 0530d66
commit 3763e5b
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 14 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -61,6 +61,10 @@
     *   TTML: Fix handling of percentage `tts:fontSize` values to ensure they
         are correctly inherited from parent nodes with percentage `tts:fontSize`
         values.
+    *   Fix `IndexOutOfBoundsException` in `LegacySubtitleUtil` due to
+        incorrectly handling the case of the requested output start time being
+        greater than or equal to the final event time in the `Subtitle`
+        ([#1516](https://github.com/androidx/media/issues/1516)).
 *   Metadata:
 *   Image:
     *   Add `ExternallyLoadedImageDecoder` for simplified integration with

diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/LegacySubtitleUtil.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/LegacySubtitleUtil.java
@@ -37,17 +37,12 @@ private LegacySubtitleUtil() {}
    */
   public static void toCuesWithTiming(
       Subtitle subtitle, OutputOptions outputOptions, Consumer<CuesWithTiming> output) {
-    if (subtitle.getEventTimeCount() == 0) {
-      return;
-    }
-    int startIndex = getStartIndex(subtitle, outputOptions);
+    int startIndex = getStartIndex(subtitle, outputOptions.startTimeUs);
     boolean startedInMiddleOfCue = false;
-    if (outputOptions.startTimeUs != C.TIME_UNSET) {
+    if (outputOptions.startTimeUs != C.TIME_UNSET && startIndex < subtitle.getEventTimeCount()) {
       List<Cue> cuesAtStartTime = subtitle.getCues(outputOptions.startTimeUs);
       long firstEventTimeUs = subtitle.getEventTime(startIndex);
-      if (!cuesAtStartTime.isEmpty()
-          && startIndex < subtitle.getEventTimeCount()
-          && outputOptions.startTimeUs < firstEventTimeUs) {
+      if (!cuesAtStartTime.isEmpty() && outputOptions.startTimeUs < firstEventTimeUs) {
         output.accept(
             new CuesWithTiming(
                 cuesAtStartTime,
@@ -74,16 +69,20 @@ public static void toCuesWithTiming(
     }
   }
 
-  private static int getStartIndex(Subtitle subtitle, OutputOptions outputOptions) {
-    if (outputOptions.startTimeUs == C.TIME_UNSET) {
+  /**
+   * Returns the event index from {@code subtitle} that is equal to or after {@code startTimeUs}, or
+   * zero if {@code startTimeUs == C.TIME_UNSET}, or {@code subtitle.getEventTimeCount()} if {@code
+   * startTimeUs} is after all events in {@code subtitle}.
+   */
+  private static int getStartIndex(Subtitle subtitle, long startTimeUs) {
+    if (startTimeUs == C.TIME_UNSET) {
       return 0;
     }
-    int nextEventTimeIndex = subtitle.getNextEventTimeIndex(outputOptions.startTimeUs);
+    int nextEventTimeIndex = subtitle.getNextEventTimeIndex(startTimeUs);
     if (nextEventTimeIndex == C.INDEX_UNSET) {
-      return subtitle.getEventTimeCount();
+      nextEventTimeIndex = subtitle.getEventTimeCount();
     }
-    if (nextEventTimeIndex > 0
-        && subtitle.getEventTime(nextEventTimeIndex - 1) == outputOptions.startTimeUs) {
+    if (nextEventTimeIndex > 0 && subtitle.getEventTime(nextEventTimeIndex - 1) == startTimeUs) {
       nextEventTimeIndex--;
     }
     return nextEventTimeIndex;

diff --git a/...tor/src/test/java/androidx/media3/extractor/text/webvtt/LegacySubtitleUtilWebvttTest.java b/...tor/src/test/java/androidx/media3/extractor/text/webvtt/LegacySubtitleUtilWebvttTest.java
@@ -207,6 +207,24 @@ public void toCuesWithTiming_onlyEmitCuesAfterStartTime_startInMiddleOfCue_simpl
         .containsExactly(SECOND_SUBTITLE_STRING);
   }
 
+  @Test
+  public void toCuesWithTiming_onlyEmitCuesAfterStartTime_startAtSubtitleEnd_simpleSubtitle() {
+    ImmutableList<CuesWithTiming> cuesWithTimingsList =
+        toCuesWithTimingList(
+            SIMPLE_SUBTITLE, SubtitleParser.OutputOptions.onlyCuesAfter(4_000_000));
+
+    assertThat(cuesWithTimingsList).isEmpty();
+  }
+
+  @Test
+  public void toCuesWithTiming_onlyEmitCuesAfterStartTime_startAfterSubtitleEnd_simpleSubtitle() {
+    ImmutableList<CuesWithTiming> cuesWithTimingsList =
+        toCuesWithTimingList(
+            SIMPLE_SUBTITLE, SubtitleParser.OutputOptions.onlyCuesAfter(4_500_000));
+
+    assertThat(cuesWithTimingsList).isEmpty();
+  }
+
   @Test
   public void toCuesWithTiming_onlyEmitCuesAfterStartTime_startBetweenCues_consecutiveSubtitle() {
     ImmutableList<CuesWithTiming> cuesWithTimingsList =
@@ -340,6 +358,48 @@ public void toCuesWithTiming_onlyEmitCuesAfterStartTime_emptySubtitle() {
         .containsExactly(FIRST_SUBTITLE_STRING);
   }
 
+  @Test
+  public void
+      toCuesWithTiming_emitCuesAfterStartTimeThenThoseBefore_startAtSubtitleEnd_simpleSubtitle() {
+    ImmutableList<CuesWithTiming> cuesWithTimingsList =
+        toCuesWithTimingList(
+            SIMPLE_SUBTITLE,
+            SubtitleParser.OutputOptions.cuesAfterThenRemainingCuesBefore(4_000_000));
+
+    assertThat(cuesWithTimingsList).hasSize(2);
+    assertThat(cuesWithTimingsList.get(0).startTimeUs).isEqualTo(1_000_000);
+    assertThat(cuesWithTimingsList.get(0).durationUs).isEqualTo(1_000_000);
+    assertThat(cuesWithTimingsList.get(0).endTimeUs).isEqualTo(2_000_000);
+    assertThat(cuesWithTimingsList.get(0).cues.stream().map(c -> c.text))
+        .containsExactly(FIRST_SUBTITLE_STRING);
+    assertThat(cuesWithTimingsList.get(1).startTimeUs).isEqualTo(3_000_000);
+    assertThat(cuesWithTimingsList.get(1).durationUs).isEqualTo(1_000_000);
+    assertThat(cuesWithTimingsList.get(1).endTimeUs).isEqualTo(4_000_000);
+    assertThat(cuesWithTimingsList.get(1).cues.stream().map(c -> c.text))
+        .containsExactly(SECOND_SUBTITLE_STRING);
+  }
+
+  @Test
+  public void
+      toCuesWithTiming_emitCuesAfterStartTimeThenThoseBefore_startAfterSubtitleEnd_simpleSubtitle() {
+    ImmutableList<CuesWithTiming> cuesWithTimingsList =
+        toCuesWithTimingList(
+            SIMPLE_SUBTITLE,
+            SubtitleParser.OutputOptions.cuesAfterThenRemainingCuesBefore(4_500_000));
+
+    assertThat(cuesWithTimingsList).hasSize(2);
+    assertThat(cuesWithTimingsList.get(0).startTimeUs).isEqualTo(1_000_000);
+    assertThat(cuesWithTimingsList.get(0).durationUs).isEqualTo(1_000_000);
+    assertThat(cuesWithTimingsList.get(0).endTimeUs).isEqualTo(2_000_000);
+    assertThat(cuesWithTimingsList.get(0).cues.stream().map(c -> c.text))
+        .containsExactly(FIRST_SUBTITLE_STRING);
+    assertThat(cuesWithTimingsList.get(1).startTimeUs).isEqualTo(3_000_000);
+    assertThat(cuesWithTimingsList.get(1).durationUs).isEqualTo(1_000_000);
+    assertThat(cuesWithTimingsList.get(1).endTimeUs).isEqualTo(4_000_000);
+    assertThat(cuesWithTimingsList.get(1).cues.stream().map(c -> c.text))
+        .containsExactly(SECOND_SUBTITLE_STRING);
+  }
+
   @Test
   public void
       toCuesWithTiming_emitCuesAfterStartTimeThenThoseBefore_startBetweenCues_consecutiveSubtitle() {