From 9bb254f6975c40e795aea4e8a9b4f108d171aab0 Mon Sep 17 00:00:00 2001 From: ibaker Date: Wed, 22 Jan 2025 07:38:29 -0800 Subject: [PATCH] Skip SSA cues where end time is before or equal to start time The file in Issue: androidx/media#2052 contains a cue with the following timecode: ``` 0:00:00:00,0:00:00:00 ``` The content of this cue seems to be some 'converted by' metadata, i.e. it's basically a comment and clearly not intended to be shown on screen (since it has zero duration). There is some fiddly logic later in `SsaParser` to support overlapping cues with the old `Subtitle` structure [1], and this logic gets tripped up by the start and end time being equal, which results in a **single**, empty `List` being added - which trips up another assumption that every SSA cue line results in at least two `List` entries (one containing the cue text, and another containing an empty list to signal the end of the cues). This fiddly logic is no longer required, because overlapping `CuesWithTiming` objects can now be merged in `TextRenderer`, so there is a possible future simplification to `SsaParser` which removes a lot of this complexity. [1] Added in PiperOrigin-RevId: 718380386 --- RELEASENOTES.md | 3 +++ ...elegatingSubtitleDecoderWithSsaParserTest.java | 11 +++++++++-- .../media3/extractor/text/ssa/SsaParser.java | 2 +- .../media3/extractor/text/ssa/SsaParserTest.java | 15 ++++++++++++--- .../src/test/assets/media/ssa/invalid_timecodes | 12 ++++++++---- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index abee86beec..2494ab424f 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -63,6 +63,9 @@ ([#8260](https://github.com/google/ExoPlayer/issues/8260)). * Fix playback hanging when a playlist contains clipped items with CEA-608 or CEA-708 captions. + * Fix `IllegalStateException` when an SSA file contains a cue with zero + duration (start and end time equal) + ([#2052](https://github.com/androidx/media/issues/2052)). * Metadata: * Image: * DataSource: diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/DelegatingSubtitleDecoderWithSsaParserTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/DelegatingSubtitleDecoderWithSsaParserTest.java index 02a1a87f76..d15826ffea 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/DelegatingSubtitleDecoderWithSsaParserTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/DelegatingSubtitleDecoderWithSsaParserTest.java @@ -342,16 +342,23 @@ public final class DelegatingSubtitleDecoderWithSsaParserTest { assertThat(firstCue.line).isEqualTo(Cue.DIMEN_UNSET); } + /** + * Parsing should succeed, skipping the cues with invalid time codes, and parsing the third and + * final cues only. + */ @Test public void decodeInvalidTimecodes() throws IOException { - // Parsing should succeed, parsing the third cue only. DelegatingSubtitleDecoder decoder = new DelegatingSubtitleDecoder("SSA", new SsaParser()); byte[] bytes = TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), INVALID_TIMECODES); Subtitle subtitle = decoder.decode(bytes, bytes.length, false); - assertThat(subtitle.getEventTimeCount()).isEqualTo(2); + assertThat(subtitle.getEventTimeCount()).isEqualTo(4); assertTypicalCue3(subtitle, 0); + assertThat(subtitle.getEventTime(2)).isEqualTo(16_560_000); + assertThat(subtitle.getEventTime(3)).isEqualTo(17_900_000); + assertThat(Iterables.getOnlyElement(subtitle.getCues(16_560_000)).text.toString()) + .isEqualTo("This is the last subtitle."); } @Test diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/ssa/SsaParser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/ssa/SsaParser.java index ac60f22332..f387bb9f6c 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/ssa/SsaParser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/ssa/SsaParser.java @@ -334,7 +334,7 @@ public final class SsaParser implements SubtitleParser { } long endTimeUs = parseTimecodeUs(lineValues[format.endTimeIndex]); - if (endTimeUs == C.TIME_UNSET) { + if (endTimeUs == C.TIME_UNSET || endTimeUs <= startTimeUs) { Log.w(TAG, "Skipping invalid timing: " + dialogueLine); return; } diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/ssa/SsaParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/ssa/SsaParserTest.java index 914fc7673a..ba10f7974b 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/ssa/SsaParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/ssa/SsaParserTest.java @@ -425,16 +425,25 @@ public final class SsaParserTest { assertThat(firstCue.line).isEqualTo(Cue.DIMEN_UNSET); } + /** + * Parsing should succeed, skipping the cues with invalid time codes, and parsing the third and + * final cues only. + */ @Test public void parseInvalidTimecodes() throws IOException { - // Parsing should succeed, parsing the third cue only. SsaParser parser = new SsaParser(); byte[] bytes = TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), INVALID_TIMECODES); ImmutableList allCues = parseAllCues(parser, bytes); - assertThat(allCues).hasSize(1); - assertTypicalCue3(Iterables.getOnlyElement(allCues)); + assertThat(allCues).hasSize(2); + assertTypicalCue3(allCues.get(0)); + CuesWithTiming finalCue = allCues.get(1); + assertThat(finalCue.startTimeUs).isEqualTo(16_560_000); + assertThat(finalCue.endTimeUs).isEqualTo(17_900_000); + assertThat(finalCue.durationUs).isEqualTo(1_340_000); + assertThat(Iterables.getOnlyElement(finalCue.cues).text.toString()) + .isEqualTo("This is the last subtitle."); } @Test diff --git a/libraries/test_data/src/test/assets/media/ssa/invalid_timecodes b/libraries/test_data/src/test/assets/media/ssa/invalid_timecodes index 380d330a86..2c4ea031af 100644 --- a/libraries/test_data/src/test/assets/media/ssa/invalid_timecodes +++ b/libraries/test_data/src/test/assets/media/ssa/invalid_timecodes @@ -7,8 +7,12 @@ Style: Default,Open Sans Semibold,36,&H00FFFFFF,&H000000FF,&H00020713,&H00000000 [Events] Format: Layer, Start, End, Style, Name, Text -Dialogue: 0,Invalid,0:00:01.23,Default,Olly,This is the first subtitle{ignored}. -Dialogue: 0,0:00:02.34,Invalid,Default,Olly,This is the second subtitle \nwith a newline \Nand another. +Dialogue: 0,Invalid,0:00:01.23,Default,Olly,Non-integer start time. +Dialogue: 0,0:00:02.34,Invalid,Default,Olly,Non-integer end time. Dialogue: 0,0:00:04:56,0:00:08:90,Default,Olly,This is the third subtitle, with a comma. -Dialogue: 0, ,0:00:10:90,Default,Olly,This is the fourth subtitle. -Dialogue: 0,0:00:12:90, ,Default,Olly,This is the fifth subtitle. +Dialogue: 0, ,0:00:10:90,Default,Olly,Empty start time. +Dialogue: 0,0:00:12:90, ,Default,Olly,Empty end time. +Dialogue: 0,0:00:13:90,0:00:13:90,Default,Olly,Start and end time equal. +Dialogue: 0,0:00:15:90,0:00:14:90,Default,Olly,End time before start time. +Dialogue: 0,0:00:16:56,0:00:17:90,Default,Olly,This is the last subtitle. +