diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 4327760db3..23479c37aa 100644 --- 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 index 79e3970b36..508c97e84b 100644 --- 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 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 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/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/LegacySubtitleUtilWebvttTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/LegacySubtitleUtilWebvttTest.java index 9a9d2ce693..287d2f9d27 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/LegacySubtitleUtilWebvttTest.java +++ b/libraries/extractor/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 cuesWithTimingsList = + toCuesWithTimingList( + SIMPLE_SUBTITLE, SubtitleParser.OutputOptions.onlyCuesAfter(4_000_000)); + + assertThat(cuesWithTimingsList).isEmpty(); + } + + @Test + public void toCuesWithTiming_onlyEmitCuesAfterStartTime_startAfterSubtitleEnd_simpleSubtitle() { + ImmutableList cuesWithTimingsList = + toCuesWithTimingList( + SIMPLE_SUBTITLE, SubtitleParser.OutputOptions.onlyCuesAfter(4_500_000)); + + assertThat(cuesWithTimingsList).isEmpty(); + } + @Test public void toCuesWithTiming_onlyEmitCuesAfterStartTime_startBetweenCues_consecutiveSubtitle() { ImmutableList cuesWithTimingsList = @@ -340,6 +358,48 @@ public void toCuesWithTiming_onlyEmitCuesAfterStartTime_emptySubtitle() { .containsExactly(FIRST_SUBTITLE_STRING); } + @Test + public void + toCuesWithTiming_emitCuesAfterStartTimeThenThoseBefore_startAtSubtitleEnd_simpleSubtitle() { + ImmutableList 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 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() {