Skip negative SubRip timecodes

Issue: #2145 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=140868079
google · Dec 2, 2016 · eee9bf5 · eee9bf5
1 parent 076f31a
commit eee9bf5
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 19 deletions.
diff --git a/library/src/androidTest/assets/subrip/typical_negative_timestamps b/library/src/androidTest/assets/subrip/typical_negative_timestamps
@@ -0,0 +1,12 @@
+1
+-0:00:04,567 --> -0:00:03,456
+This is the first subtitle.
+
+2
+-00:00:02,345 --> 00:00:01,234
+This is the second subtitle.
+Second subtitle with second line.
+
+3
+00:00:04,567 --> 00:00:08,901
+This is the third subtitle.
diff --git a/library/src/androidTest/java/com/google/android/exoplayer/text/subrip/SubripParserTest.java b/library/src/androidTest/java/com/google/android/exoplayer/text/subrip/SubripParserTest.java
@@ -30,6 +30,7 @@ public final class SubripParserTest extends InstrumentationTestCase {
   private static final String TYPICAL_EXTRA_BLANK_LINE = "subrip/typical_extra_blank_line";
   private static final String TYPICAL_MISSING_TIMECODE = "subrip/typical_missing_timecode";
   private static final String TYPICAL_MISSING_SEQUENCE = "subrip/typical_missing_sequence";
+  private static final String TYPICAL_NEGATIVE_TIMESTAMPS = "subrip/typical_negative_timestamps";
   private static final String NO_END_TIMECODES_FILE = "subrip/no_end_timecodes";
 
   public void testParseEmpty() throws IOException {
@@ -115,6 +116,15 @@ public void testParseNoEndTimecodes() throws IOException {
         subtitle.getCues(subtitle.getEventTime(2)).get(0).text.toString());
   }
 
+  public void testDecodeTypicalNegativeTimestamps() throws IOException {
+    // Parsing should succeed, parsing the third cue only.
+    SubripParser parser = new SubripParser();
+    byte[] bytes = TestUtil.getByteArray(getInstrumentation(), TYPICAL_NEGATIVE_TIMESTAMPS);
+    SubripSubtitle subtitle = parser.parse(bytes, 0, bytes.length);
+    assertEquals(2, subtitle.getEventTimeCount());
+    assertTypicalCue3(subtitle, 0);
+  }
+
   private static void assertTypicalCue1(SubripSubtitle subtitle, int eventIndex) {
     assertEquals(0, subtitle.getEventTime(eventIndex));
     assertEquals("This is the first subtitle.",

diff --git a/library/src/main/java/com/google/android/exoplayer/text/subrip/SubripParser.java b/library/src/main/java/com/google/android/exoplayer/text/subrip/SubripParser.java
@@ -35,9 +35,9 @@ public final class SubripParser implements SubtitleParser {
 
   private static final String TAG = "SubripParser";
 
-  private static final Pattern SUBRIP_TIMING_LINE = Pattern.compile("(\\S*)\\s*-->\\s*(\\S*)");
-  private static final Pattern SUBRIP_TIMESTAMP =
-      Pattern.compile("(?:(\\d+):)?(\\d+):(\\d+),(\\d+)");
+  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*");
 
   private final StringBuilder textBuilder;
 
@@ -56,7 +56,6 @@ public SubripSubtitle parse(byte[] bytes, int offset, int length) {
     LongArray cueTimesUs = new LongArray();
     ParsableByteArray subripData = new ParsableByteArray(bytes, offset + length);
     subripData.setPosition(offset);
-    boolean haveEndTimecode;
     String currentLine;
 
     while ((currentLine = subripData.readLine()) != null) {
@@ -74,15 +73,14 @@ public SubripSubtitle parse(byte[] bytes, int offset, int length) {
       }
 
       // Read and parse the timing line.
-      haveEndTimecode = false;
+      boolean haveEndTimecode = false;
       currentLine = subripData.readLine();
       Matcher matcher = SUBRIP_TIMING_LINE.matcher(currentLine);
-      if (matcher.find()) {
-        cueTimesUs.add(parseTimecode(matcher.group(1)));
-        String endTimecode = matcher.group(2);
-        if (!TextUtils.isEmpty(endTimecode)) {
+      if (matcher.matches()) {
+        cueTimesUs.add(parseTimecode(matcher, 1));
+        if (!TextUtils.isEmpty(matcher.group(6))) {
           haveEndTimecode = true;
-          cueTimesUs.add(parseTimecode(matcher.group(2)));
+          cueTimesUs.add(parseTimecode(matcher, 6));
         }
       } else {
         Log.w(TAG, "Skipping invalid timing: " + currentLine);
@@ -111,15 +109,11 @@ public SubripSubtitle parse(byte[] bytes, int offset, int length) {
     return new SubripSubtitle(cuesArray, cueTimesUsArray);
   }
 
-  private static long parseTimecode(String s) throws NumberFormatException {
-    Matcher matcher = SUBRIP_TIMESTAMP.matcher(s);
-    if (!matcher.matches()) {
-      throw new NumberFormatException("has invalid format");
-    }
-    long timestampMs = Long.parseLong(matcher.group(1)) * 60 * 60 * 1000;
-    timestampMs += Long.parseLong(matcher.group(2)) * 60 * 1000;
-    timestampMs += Long.parseLong(matcher.group(3)) * 1000;
-    timestampMs += Long.parseLong(matcher.group(4));
+  private static long parseTimecode(Matcher matcher, int groupOffset) {
+    long timestampMs = Long.parseLong(matcher.group(groupOffset + 1)) * 60 * 60 * 1000;
+    timestampMs += Long.parseLong(matcher.group(groupOffset + 2)) * 60 * 1000;
+    timestampMs += Long.parseLong(matcher.group(groupOffset + 3)) * 1000;
+    timestampMs += Long.parseLong(matcher.group(groupOffset + 4));
     return timestampMs * 1000;
   }