From d18292c9c9a998157ca38b8730fa22fb936ecbb8 Mon Sep 17 00:00:00 2001 From: ibaker Date: Mon, 23 Mar 2020 18:03:04 +0000 Subject: [PATCH] Allow missing hours in SubRip (.srt) subtitle timecodes Add a test for this case, and extend the existing tests to ensure the hour is parsed when it's present. issue:#7122 PiperOrigin-RevId: 302472213 --- RELEASENOTES.md | 9 ++++-- .../exoplayer2/text/subrip/SubripDecoder.java | 5 ++- library/core/src/test/assets/subrip/typical | 2 +- .../assets/subrip/typical_extra_blank_line | 2 +- .../assets/subrip/typical_missing_sequence | 2 +- .../assets/subrip/typical_missing_timecode | 6 ++-- .../assets/subrip/typical_negative_timestamps | 2 +- .../src/test/assets/subrip/typical_no_hours | 12 +++++++ .../subrip/typical_with_byte_order_mark | 2 +- .../text/subrip/SubripDecoderTest.java | 31 ++++++++++++++++--- 10 files changed, 56 insertions(+), 17 deletions(-) create mode 100644 library/core/src/test/assets/subrip/typical_no_hours diff --git a/RELEASENOTES.md b/RELEASENOTES.md index c9b986f731..d9f0ac4274 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -2,9 +2,12 @@ ### 2.11.4 (not yet released) ### -* Text: Catch-and-log all fatal exceptions in `TextRenderer` instead of - re-throwing, allowing playback to continue even if subtitles fail - ([#6885](https://github.com/google/ExoPlayer/issues/6885)). +* Text: + * Catch-and-log all fatal exceptions in `TextRenderer` instead of + re-throwing, allowing playback to continue even if subtitles fail + ([#6885](https://github.com/google/ExoPlayer/issues/6885)). + * Allow missing hours in SubRip (.srt) timecodes + ([#7122](https://github.com/google/ExoPlayer/issues/7122)). * UI: Add an option to set whether to use the orientation sensor for rotation in spherical playbacks ([#6761](https://github.com/google/ExoPlayer/issues/6761)). diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/subrip/SubripDecoder.java b/library/core/src/main/java/com/google/android/exoplayer2/text/subrip/SubripDecoder.java index 0c402ac018..2b562ff8c7 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/text/subrip/SubripDecoder.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/text/subrip/SubripDecoder.java @@ -41,10 +41,12 @@ public final class SubripDecoder extends SimpleSubtitleDecoder { private static final String TAG = "SubripDecoder"; + // Some SRT files don't include hours in the timecode, so we use an optional group. private static final String SUBRIP_TIMECODE = "(?:(\\d+):)?(\\d+):(\\d+),(\\d+)"; private static final Pattern SUBRIP_TIMING_LINE = Pattern.compile("\\s*(" + SUBRIP_TIMECODE + ")\\s*-->\\s*(" + SUBRIP_TIMECODE + ")\\s*"); + // NOTE: Android Studio's suggestion to simplify '\\}' is incorrect [internal: b/144480183]. private static final Pattern SUBRIP_TAG_PATTERN = Pattern.compile("\\{\\\\.*?\\}"); private static final String SUBRIP_ALIGNMENT_TAG = "\\{\\\\an[1-9]\\}"; @@ -229,7 +231,8 @@ public final class SubripDecoder extends SimpleSubtitleDecoder { } private static long parseTimecode(Matcher matcher, int groupOffset) { - long timestampMs = Long.parseLong(matcher.group(groupOffset + 1)) * 60 * 60 * 1000; + @Nullable String hours = matcher.group(groupOffset + 1); + long timestampMs = hours != null ? Long.parseLong(hours) * 60 * 60 * 1000 : 0; timestampMs += Long.parseLong(matcher.group(groupOffset + 2)) * 60 * 1000; timestampMs += Long.parseLong(matcher.group(groupOffset + 3)) * 1000; timestampMs += Long.parseLong(matcher.group(groupOffset + 4)); diff --git a/library/core/src/test/assets/subrip/typical b/library/core/src/test/assets/subrip/typical index 1331f75651..cc9a3da871 100644 --- a/library/core/src/test/assets/subrip/typical +++ b/library/core/src/test/assets/subrip/typical @@ -8,5 +8,5 @@ This is the second subtitle. Second subtitle with second line. 3 -00:00:04,567 --> 00:00:08,901 +02:00:04,567 --> 02:00:08,901 This is the third subtitle. diff --git a/library/core/src/test/assets/subrip/typical_extra_blank_line b/library/core/src/test/assets/subrip/typical_extra_blank_line index f5882a1d68..392cb7e91c 100644 --- a/library/core/src/test/assets/subrip/typical_extra_blank_line +++ b/library/core/src/test/assets/subrip/typical_extra_blank_line @@ -9,5 +9,5 @@ This is the second subtitle. Second subtitle with second line. 3 -00:00:04,567 --> 00:00:08,901 +02:00:04,567 --> 02:00:08,901 This is the third subtitle. diff --git a/library/core/src/test/assets/subrip/typical_missing_sequence b/library/core/src/test/assets/subrip/typical_missing_sequence index 56d49ac63c..e75711d7a8 100644 --- a/library/core/src/test/assets/subrip/typical_missing_sequence +++ b/library/core/src/test/assets/subrip/typical_missing_sequence @@ -7,5 +7,5 @@ This is the second subtitle. Second subtitle with second line. 3 -00:00:04,567 --> 00:00:08,901 +02:00:04,567 --> 02:00:08,901 This is the third subtitle. diff --git a/library/core/src/test/assets/subrip/typical_missing_timecode b/library/core/src/test/assets/subrip/typical_missing_timecode index cd25ffca3b..bce61a77f9 100644 --- a/library/core/src/test/assets/subrip/typical_missing_timecode +++ b/library/core/src/test/assets/subrip/typical_missing_timecode @@ -7,13 +7,13 @@ This is the second subtitle. Second subtitle with second line. 3 -00:00:04,567 --> 00:00:08,901 +02:00:04,567 --> 02:00:08,901 This is the third subtitle. 4 - --> 00:00:10,901 + --> 02:00:10,901 This is the fourth subtitle. 5 -00:00:12,901 --> +02:00:12,901 --> This is the fifth subtitle. diff --git a/library/core/src/test/assets/subrip/typical_negative_timestamps b/library/core/src/test/assets/subrip/typical_negative_timestamps index 2a47c0993b..1df7bf68e3 100644 --- a/library/core/src/test/assets/subrip/typical_negative_timestamps +++ b/library/core/src/test/assets/subrip/typical_negative_timestamps @@ -8,5 +8,5 @@ This is the second subtitle. Second subtitle with second line. 3 -00:00:04,567 --> 00:00:08,901 +02:00:04,567 --> 02:00:08,901 This is the third subtitle. diff --git a/library/core/src/test/assets/subrip/typical_no_hours b/library/core/src/test/assets/subrip/typical_no_hours new file mode 100644 index 0000000000..c4bf372a3c --- /dev/null +++ b/library/core/src/test/assets/subrip/typical_no_hours @@ -0,0 +1,12 @@ +1 +00:00,000 --> 00:01,234 +This is the first subtitle. + +2 +00:02,345 --> 00:03,456 +This is the second subtitle. +Second subtitle with second line. + +3 +02:00:04,567 --> 02:00:08,901 +This is the third subtitle. diff --git a/library/core/src/test/assets/subrip/typical_with_byte_order_mark b/library/core/src/test/assets/subrip/typical_with_byte_order_mark index 4f5b32f4d7..050e1c02a6 100644 --- a/library/core/src/test/assets/subrip/typical_with_byte_order_mark +++ b/library/core/src/test/assets/subrip/typical_with_byte_order_mark @@ -8,5 +8,5 @@ This is the second subtitle. Second subtitle with second line. 3 -00:00:04,567 --> 00:00:08,901 +02:00:04,567 --> 02:00:08,901 This is the third subtitle. diff --git a/library/core/src/test/java/com/google/android/exoplayer2/text/subrip/SubripDecoderTest.java b/library/core/src/test/java/com/google/android/exoplayer2/text/subrip/SubripDecoderTest.java index 9f66f65a56..1f29239607 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/text/subrip/SubripDecoderTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/text/subrip/SubripDecoderTest.java @@ -39,6 +39,7 @@ public final class SubripDecoderTest { private static final String TYPICAL_NEGATIVE_TIMESTAMPS = "subrip/typical_negative_timestamps"; private static final String TYPICAL_UNEXPECTED_END = "subrip/typical_unexpected_end"; private static final String TYPICAL_WITH_TAGS = "subrip/typical_with_tags"; + private static final String TYPICAL_NO_HOURS = "subrip/typical_no_hours"; @Test public void testDecodeEmpty() throws IOException { @@ -151,9 +152,14 @@ public final class SubripDecoderTest { TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), TYPICAL_WITH_TAGS); Subtitle subtitle = decoder.decode(bytes, bytes.length, false); - assertTypicalCue1(subtitle, 0); - assertTypicalCue2(subtitle, 2); - assertTypicalCue3(subtitle, 4); + assertThat(subtitle.getCues(subtitle.getEventTime(0)).get(0).text.toString()) + .isEqualTo("This is the first subtitle."); + + assertThat(subtitle.getCues(subtitle.getEventTime(2)).get(0).text.toString()) + .isEqualTo("This is the second subtitle.\nSecond subtitle with second line."); + + assertThat(subtitle.getCues(subtitle.getEventTime(4)).get(0).text.toString()) + .isEqualTo("This is the third subtitle."); assertThat(subtitle.getCues(subtitle.getEventTime(6)).get(0).text.toString()) .isEqualTo("This { \\an2} is not a valid tag due to the space after the opening bracket."); @@ -172,6 +178,19 @@ public final class SubripDecoderTest { assertAlignmentCue(subtitle, 26, Cue.ANCHOR_TYPE_START, Cue.ANCHOR_TYPE_END); // {/an9} } + @Test + public void decodeTypicalNoHours() throws IOException { + SubripDecoder decoder = new SubripDecoder(); + byte[] bytes = + TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), TYPICAL_NO_HOURS); + Subtitle subtitle = decoder.decode(bytes, bytes.length, false); + + assertThat(subtitle.getEventTimeCount()).isEqualTo(6); + assertTypicalCue1(subtitle, 0); + assertTypicalCue2(subtitle, 2); + assertTypicalCue3(subtitle, 4); + } + private static void assertTypicalCue1(Subtitle subtitle, int eventIndex) { assertThat(subtitle.getEventTime(eventIndex)).isEqualTo(0); assertThat(subtitle.getCues(subtitle.getEventTime(eventIndex)).get(0).text.toString()) @@ -187,10 +206,12 @@ public final class SubripDecoderTest { } private static void assertTypicalCue3(Subtitle subtitle, int eventIndex) { - assertThat(subtitle.getEventTime(eventIndex)).isEqualTo(4567000); + long expectedStartTimeUs = (((2L * 60L * 60L) + 4L) * 1000L + 567L) * 1000L; + assertThat(subtitle.getEventTime(eventIndex)).isEqualTo(expectedStartTimeUs); assertThat(subtitle.getCues(subtitle.getEventTime(eventIndex)).get(0).text.toString()) .isEqualTo("This is the third subtitle."); - assertThat(subtitle.getEventTime(eventIndex + 1)).isEqualTo(8901000); + long expectedEndTimeUs = (((2L * 60L * 60L) + 8L) * 1000L + 901L) * 1000L; + assertThat(subtitle.getEventTime(eventIndex + 1)).isEqualTo(expectedEndTimeUs); } private static void assertAlignmentCue(