diff --git a/RELEASENOTES.md b/RELEASENOTES.md index e625514eb2a..4b2948cf4f1 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -34,6 +34,10 @@ tunneling even if the device does not do this automatically as required by the API ([#1169](https://github.com/androidx/media/issues/1169)). ([#966](https://github.com/androidx/media/issues/966)). +* Text: + * WebVTT: Prevent directly consecutive cues from creating spurious + additional `CuesWithTiming` instances from `WebvttParser.parse` + ([#1177](https://github.com/androidx/media/issues/1177)). * DRM: * Work around a `NoSuchMethodError` which can be thrown by the `MediaDrm` framework instead of `ResourceBusyException` or 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 2d9ebe21fb1..5d58569cee5 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 @@ -101,6 +101,8 @@ private static void outputSubtitleEvent( // It's safe to inspect element i+1, because we already exited the loop above if // i == getEventTimeCount() - 1. long durationUs = subtitle.getEventTime(eventIndex + 1) - subtitle.getEventTime(eventIndex); - output.accept(new CuesWithTiming(cuesForThisStartTime, startTimeUs, durationUs)); + if (durationUs > 0) { + output.accept(new CuesWithTiming(cuesForThisStartTime, startTimeUs, durationUs)); + } } } 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 efa0cc8ba71..2645c788748 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 @@ -52,6 +52,18 @@ public class LegacySubtitleUtilWebvttTest { /* startTimeUs= */ 3_000_000, /* endTimeUs= */ 4_000_000))); + private static final WebvttSubtitle CONSECUTIVE_SUBTITLE = + new WebvttSubtitle( + Arrays.asList( + new WebvttCueInfo( + WebvttCueParser.newCueForText(FIRST_SUBTITLE_STRING), + /* startTimeUs= */ 1_000_000, + /* endTimeUs= */ 2_000_000), + new WebvttCueInfo( + WebvttCueParser.newCueForText(SECOND_SUBTITLE_STRING), + /* startTimeUs= */ 2_000_000, + /* endTimeUs= */ 4_000_000))); + private static final WebvttSubtitle OVERLAPPING_SUBTITLE = new WebvttSubtitle( Arrays.asList( @@ -82,6 +94,24 @@ public void toCuesWithTiming_allCues_simpleSubtitle() { .containsExactly(SECOND_SUBTITLE_STRING); } + @Test + public void toCuesWithTiming_allCues_consecutiveSubtitle() { + ImmutableList cuesWithTimingsList = + toCuesWithTimingList(CONSECUTIVE_SUBTITLE, SubtitleParser.OutputOptions.allCues()); + + 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(Lists.transform(cuesWithTimingsList.get(0).cues, c -> c.text)) + .containsExactly(FIRST_SUBTITLE_STRING); + assertThat(cuesWithTimingsList.get(1).startTimeUs).isEqualTo(2_000_000); + assertThat(cuesWithTimingsList.get(1).durationUs).isEqualTo(2_000_000); + assertThat(cuesWithTimingsList.get(1).endTimeUs).isEqualTo(4_000_000); + assertThat(Lists.transform(cuesWithTimingsList.get(1).cues, c -> c.text)) + .containsExactly(SECOND_SUBTITLE_STRING); + } + @Test public void toCuesWithTiming_allCues_overlappingSubtitle() { ImmutableList cuesWithTimingsList = @@ -168,6 +198,40 @@ public void toCuesWithTiming_onlyEmitCuesAfterStartTime_startInMiddleOfCue_simpl .containsExactly(SECOND_SUBTITLE_STRING); } + @Test + public void toCuesWithTiming_onlyEmitCuesAfterStartTime_startBetweenCues_consecutiveSubtitle() { + ImmutableList cuesWithTimingsList = + toCuesWithTimingList( + CONSECUTIVE_SUBTITLE, SubtitleParser.OutputOptions.onlyCuesAfter(2_000_000)); + + assertThat(cuesWithTimingsList).hasSize(1); + assertThat(cuesWithTimingsList.get(0).startTimeUs).isEqualTo(2_000_000); + assertThat(cuesWithTimingsList.get(0).durationUs).isEqualTo(2_000_000); + assertThat(cuesWithTimingsList.get(0).endTimeUs).isEqualTo(4_000_000); + assertThat(Lists.transform(cuesWithTimingsList.get(0).cues, c -> c.text)) + .containsExactly(SECOND_SUBTITLE_STRING); + } + + @Test + public void toCuesWithTiming_onlyEmitCuesAfterStartTime_startInMiddleOfCue_consecutiveSubtitle() { + ImmutableList cuesWithTimingsList = + toCuesWithTimingList( + CONSECUTIVE_SUBTITLE, SubtitleParser.OutputOptions.onlyCuesAfter(1_500_000)); + + assertThat(cuesWithTimingsList).hasSize(2); + // First cue is truncated to start at OutputOptions.startTimeUs + assertThat(cuesWithTimingsList.get(0).startTimeUs).isEqualTo(1_500_000); + assertThat(cuesWithTimingsList.get(0).durationUs).isEqualTo(500_000); + assertThat(cuesWithTimingsList.get(0).endTimeUs).isEqualTo(2_000_000); + assertThat(Lists.transform(cuesWithTimingsList.get(0).cues, c -> c.text)) + .containsExactly(FIRST_SUBTITLE_STRING); + assertThat(cuesWithTimingsList.get(1).startTimeUs).isEqualTo(2_000_000); + assertThat(cuesWithTimingsList.get(1).durationUs).isEqualTo(2_000_000); + assertThat(cuesWithTimingsList.get(1).endTimeUs).isEqualTo(4_000_000); + assertThat(Lists.transform(cuesWithTimingsList.get(1).cues, c -> c.text)) + .containsExactly(SECOND_SUBTITLE_STRING); + } + @Test public void toCuesWithTiming_onlyEmitCuesAfterStartTime_overlappingSubtitle() { ImmutableList cuesWithTimingsList = @@ -259,6 +323,55 @@ public void toCuesWithTiming_onlyEmitCuesAfterStartTime_overlappingSubtitle() { .containsExactly(FIRST_SUBTITLE_STRING); } + @Test + public void + toCuesWithTiming_emitCuesAfterStartTimeThenThoseBefore_startBetweenCues_consecutiveSubtitle() { + ImmutableList cuesWithTimingsList = + toCuesWithTimingList( + CONSECUTIVE_SUBTITLE, + SubtitleParser.OutputOptions.cuesAfterThenRemainingCuesBefore(2_000_000)); + + assertThat(cuesWithTimingsList).hasSize(2); + assertThat(cuesWithTimingsList.get(0).startTimeUs).isEqualTo(2_000_000); + assertThat(cuesWithTimingsList.get(0).durationUs).isEqualTo(2_000_000); + assertThat(cuesWithTimingsList.get(0).endTimeUs).isEqualTo(4_000_000); + assertThat(Lists.transform(cuesWithTimingsList.get(0).cues, c -> c.text)) + .containsExactly(SECOND_SUBTITLE_STRING); + assertThat(cuesWithTimingsList.get(1).startTimeUs).isEqualTo(1_000_000); + assertThat(cuesWithTimingsList.get(1).durationUs).isEqualTo(1_000_000); + assertThat(cuesWithTimingsList.get(1).endTimeUs).isEqualTo(2_000_000); + assertThat(Lists.transform(cuesWithTimingsList.get(1).cues, c -> c.text)) + .containsExactly(FIRST_SUBTITLE_STRING); + } + + @Test + public void + toCuesWithTiming_emitCuesAfterStartTimeThenThoseBefore_startInMiddleOfCue_consecutiveSubtitle() { + ImmutableList cuesWithTimingsList = + toCuesWithTimingList( + CONSECUTIVE_SUBTITLE, + SubtitleParser.OutputOptions.cuesAfterThenRemainingCuesBefore(1_500_000)); + + assertThat(cuesWithTimingsList).hasSize(3); + // First event is truncated to start at OutputOptions.startTimeUs. + assertThat(cuesWithTimingsList.get(0).startTimeUs).isEqualTo(1_500_000); + assertThat(cuesWithTimingsList.get(0).durationUs).isEqualTo(500_000); + assertThat(cuesWithTimingsList.get(0).endTimeUs).isEqualTo(2_000_000); + assertThat(Lists.transform(cuesWithTimingsList.get(0).cues, c -> c.text)) + .containsExactly(FIRST_SUBTITLE_STRING); + assertThat(cuesWithTimingsList.get(1).startTimeUs).isEqualTo(2_000_000); + assertThat(cuesWithTimingsList.get(1).durationUs).isEqualTo(2_000_000); + assertThat(cuesWithTimingsList.get(1).endTimeUs).isEqualTo(4_000_000); + assertThat(Lists.transform(cuesWithTimingsList.get(1).cues, c -> c.text)) + .containsExactly(SECOND_SUBTITLE_STRING); + // Final event is the part of the 'first event' that is before OutputOptions.startTimeUs + assertThat(cuesWithTimingsList.get(2).startTimeUs).isEqualTo(1_000_000); + assertThat(cuesWithTimingsList.get(2).durationUs).isEqualTo(500_000); + assertThat(cuesWithTimingsList.get(2).endTimeUs).isEqualTo(1_500_000); + assertThat(Lists.transform(cuesWithTimingsList.get(2).cues, c -> c.text)) + .containsExactly(FIRST_SUBTITLE_STRING); + } + @Test public void toCuesWithTiming_emitCuesAfterStartTimeThenThoseBefore_overlappingSubtitle() { ImmutableList cuesWithTimingsList = diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttParserTest.java index 984eb6d1d50..e0834919c00 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttParserTest.java @@ -50,6 +50,8 @@ public class WebvttParserTest { private static final String TYPICAL_WITH_IDS_FILE = "media/webvtt/typical_with_identifiers"; private static final String TYPICAL_WITH_COMMENTS_FILE = "media/webvtt/typical_with_comments"; private static final String WITH_POSITIONING_FILE = "media/webvtt/with_positioning"; + private static final String WITH_CONSECUTIVE_TIMESTAMPS_FILE = + "media/webvtt/with_consecutive_cues"; private static final String WITH_OVERLAPPING_TIMESTAMPS_FILE = "media/webvtt/with_overlapping_timestamps"; private static final String WITH_VERTICAL_FILE = "media/webvtt/with_vertical"; @@ -334,6 +336,26 @@ public void parseWithPositioning() throws Exception { assertThat(eighthCue.positionAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); } + // https://github.com/androidx/media/issues/1177 + @Test + public void parseWithConsecutiveTimestamps() throws Exception { + ImmutableList allCues = getCuesForTestAsset(WITH_CONSECUTIVE_TIMESTAMPS_FILE); + + assertThat(allCues).hasSize(2); + + assertThat(allCues.get(0).startTimeUs).isEqualTo(0L); + assertThat(allCues.get(0).durationUs).isEqualTo(1_234_000L); + assertThat(allCues.get(0).endTimeUs).isEqualTo(1_234_000L); + Cue firstCue = Iterables.getOnlyElement(allCues.get(0).cues); + assertThat(firstCue.text.toString()).isEqualTo("This is the first subtitle."); + + assertThat(allCues.get(1).startTimeUs).isEqualTo(1_234_000L); + assertThat(allCues.get(1).durationUs).isEqualTo(3_456_000L - 1_234_000L); + assertThat(allCues.get(1).endTimeUs).isEqualTo(3_456_000L); + Cue secondCue = Iterables.getOnlyElement(allCues.get(1).cues); + assertThat(secondCue.text.toString()).isEqualTo("This is the second subtitle."); + } + @Test public void parseWithOverlappingTimestamps() throws Exception { List allCues = getCuesForTestAsset(WITH_OVERLAPPING_TIMESTAMPS_FILE); diff --git a/libraries/test_data/src/test/assets/media/webvtt/with_consecutive_cues b/libraries/test_data/src/test/assets/media/webvtt/with_consecutive_cues new file mode 100644 index 00000000000..1384fdf3054 --- /dev/null +++ b/libraries/test_data/src/test/assets/media/webvtt/with_consecutive_cues @@ -0,0 +1,7 @@ +WEBVTT + +00:00.000 --> 00:01.234 +This is the first subtitle. + +00:01.234 --> 00:03.456 +This is the second subtitle.