Skip to content

Commit

Permalink
Fix IndexOutOfBoundsException in LegacySubtitleUtil
Browse files Browse the repository at this point in the history
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
(cherry picked from commit 3763e5b)
  • Loading branch information
icbaker authored and tianyif committed Aug 21, 2024
1 parent cd2a36f commit bf93449
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 14 deletions.
4 changes: 4 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,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:
* DataSource:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit bf93449

Please sign in to comment.