Correct SSA overlapping subtitle decoding, add tests

library/core/src/main/java/com/google/android/exoplayer2/text/ssa/SsaDecoder.java

@@ -226,44 +226,68 @@ public final class SsaDecoder extends SimpleSubtitleDecoder {
       cue = new Cue(text);
     }
 
-    int startTimeIndex = insertToCueTimes(cueTimesUs, startTimeUs);
+    int startTimeIndex = 0;
+    boolean startTimeFound = false;
+    // Search the insertion index for startTimeUs in cueTimesUs
+    for (int i = cueTimesUs.size() - 1; i >= 0; i--) {
+      if (cueTimesUs.get(i) == startTimeUs) {
+        startTimeIndex = i;
+        startTimeFound = true;
+        break;
+      }
 
-    List<Cue> startCueList = new ArrayList<>();
+      if (cueTimesUs.get(i) < startTimeUs) {
-    if (startTimeIndex != 0) {
+        startTimeIndex = i + 1;
-      startCueList.addAll(cues.get(startTimeIndex - 1));
+        break;
-    }
-    cues.add(startTimeIndex, startCueList);
-
-    if (endTimeUs != C.TIME_UNSET) {
-      int endTimeIndex = insertToCueTimes(cueTimesUs, endTimeUs);
-      List<Cue> endList = new ArrayList<>(cues.get(endTimeIndex - 1));
-      cues.add(endTimeIndex, endList);
-
-      int i = startTimeIndex;
-      do {
-        cues.get(i).add(cue);
-        i++;
-      } while (i != endTimeIndex);
-    }
-  }
-
-  /**
-   * Insert the given cue time into the given array keeping the array sorted.
-   *
-   * @param cueTimes The array with sorted cue times
-   * @param timeUs The cue time to be inserted
-   * @return The index where the cue time was inserted
-   */
-  private static int insertToCueTimes(List<Long> cueTimes, long timeUs) {
-    for (int i = cueTimes.size() - 1; i >= 0; i--) {
-      if (cueTimes.get(i) <= timeUs) {
-        cueTimes.add(i + 1, timeUs);
-        return i + 1;
       }
     }
 
-    cueTimes.add(0, timeUs);
+    if (startTimeIndex == 0) {
-    return 0;
+      // Handle first cue
+      cueTimesUs.add(startTimeIndex, startTimeUs);
+      cues.add(startTimeIndex, new ArrayList<>());
+    } else {
+      if (!startTimeFound) {
+        // Add the startTimeUs only if it wasn't found in cueTimesUs
+        cueTimesUs.add(startTimeIndex, startTimeUs);
+        // Copy over cues from left
+        List<Cue> startCueList = new ArrayList<>(cues.get(startTimeIndex - 1));
+        cues.add(startTimeIndex, startCueList);
+      }
+    }
+
+    int endTimeIndex = 0;
+    if (endTimeUs != C.TIME_UNSET) {
+      boolean endTimeFound = false;
+
+      // Search the insertion index for endTimeUs in cueTimesUs
+      for (int i = cueTimesUs.size() - 1; i >= 0; i--) {
+        if (cueTimesUs.get(i) == endTimeUs) {
+          endTimeIndex = i;
+          endTimeFound = true;
+          break;
+        }
+
+        if (cueTimesUs.get(i) < endTimeUs) {
+          endTimeIndex = i + 1;
+          break;
+        }
+      }
+
+      if (!endTimeFound) {
+        // Add the endTimeUs only if it wasn't found in cueTimesUs
+        cueTimesUs.add(endTimeIndex, endTimeUs);
+        // Copy over cues from left
+        cues.add(endTimeIndex, new ArrayList<>(cues.get(endTimeIndex - 1)));
+      }
+    }
+
+    // Iterate on cues from startTimeIndex until endTimeIndex, add the current cue
+    int i = startTimeIndex;
+    do {
+      cues.get(i).add(cue);
+      i++;
+    } while (i < endTimeIndex);
   }
 
   /**

library/core/src/test/assets/ssa/overlap

@@ -0,0 +1,16 @@
+[Script Info]
+Title: SomeTitle
+
+[V4+ Styles]
+Format: Name, Fontname, Fontsize, PrimaryColour, SecondaryColour, OutlineColour, BackColour, Bold, Italic, Underline, StrikeOut, ScaleX, ScaleY, Spacing, Angle, BorderStyle, Outline, Shadow, Alignment, MarginL, MarginR, MarginV, Encoding
+Style: Default,Open Sans Semibold,36,&H00FFFFFF,&H000000FF,&H00020713,&H00000000,-1,0,0,0,100,100,0,0,1,1.7,0,2,0,0,28,1
+
+[Events]
+Format: Layer, Start, End, Style, Name, Text
+Dialogue: 0,0:00:01.00,0:00:04.23,Default,Olly,Subtitle A
+Dialogue: 0,0:00:02.00,0:00:05.23,Default,Olly,Subtitle B
+Dialogue: 0,0:00:06.00,0:00:08.44,Default,Olly,Subtitle C
+Dialogue: 0,0:00:08.44,0:00:09.44,Default,Olly,Subtitle D
+Dialogue: 0,0:00:08.44,0:00:09.44,Default,Olly,Subtitle E
+Dialogue: 0,0:00:10.72,0:00:15.65,Default,Olly,Subtitle F
+Dialogue: 0,0:00:13.22,0:00:14.22,Default,Olly,Subtitle G

library/core/src/test/java/com/google/android/exoplayer2/text/ssa/SsaDecoderTest.java

@@ -32,6 +32,7 @@ public final class SsaDecoderTest {
 
   private static final String EMPTY = "ssa/empty";
   private static final String TYPICAL = "ssa/typical";
+  private static final String OVERLAP = "ssa/overlap";
   private static final String TYPICAL_HEADER_ONLY = "ssa/typical_header";
   private static final String TYPICAL_DIALOGUE_ONLY = "ssa/typical_dialogue";
   private static final String TYPICAL_FORMAT_ONLY = "ssa/typical_format";
@@ -60,6 +61,37 @@ public final class SsaDecoderTest {
     assertTypicalCue3(subtitle, 4);
   }
 
+  @Test
+  public void testDecodeOverlap() throws IOException {
+    SsaDecoder decoder = new SsaDecoder();
+    byte[] bytes = TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), OVERLAP);
+    Subtitle subtitle = decoder.decode(bytes, bytes.length, false);
+
+    assertThat(subtitle.getEventTime(0)).isEqualTo(1000000);
+    assertThat(subtitle.getEventTime(1)).isEqualTo(2000000);
+    assertThat(subtitle.getEventTime(2)).isEqualTo(4230000);
+    assertThat(subtitle.getEventTime(3)).isEqualTo(5230000);
+    assertThat(subtitle.getEventTime(4)).isEqualTo(6000000);
+    assertThat(subtitle.getEventTime(5)).isEqualTo(8440000);
+    assertThat(subtitle.getEventTime(6)).isEqualTo(9440000);
+    assertThat(subtitle.getEventTime(7)).isEqualTo(10720000);
+    assertThat(subtitle.getEventTime(8)).isEqualTo(13220000);
+    assertThat(subtitle.getEventTime(9)).isEqualTo(14220000);
+    assertThat(subtitle.getEventTime(10)).isEqualTo(15650000);
+
+    assertThat(subtitle.getCues(1000010).size()).isEqualTo(1);
+    assertThat(subtitle.getCues(2000010).size()).isEqualTo(2);
+    assertThat(subtitle.getCues(4230010).size()).isEqualTo(1);
+    assertThat(subtitle.getCues(5230010).size()).isEqualTo(0);
+    assertThat(subtitle.getCues(6000010).size()).isEqualTo(1);
+    assertThat(subtitle.getCues(8440010).size()).isEqualTo(2);
+    assertThat(subtitle.getCues(9440010).size()).isEqualTo(0);
+    assertThat(subtitle.getCues(10720010).size()).isEqualTo(1);
+    assertThat(subtitle.getCues(13220010).size()).isEqualTo(2);
+    assertThat(subtitle.getCues(14220010).size()).isEqualTo(1);
+    assertThat(subtitle.getCues(15650010).size()).isEqualTo(0);
+  }
+
   @Test
   public void testDecodeTypicalWithInitializationData() throws IOException {
     byte[] headerBytes =
@@ -107,10 +139,16 @@ public final class SsaDecoderTest {
 
     assertThat(subtitle.getEventTime(1)).isEqualTo(2340000);
     assertThat(subtitle.getCues(subtitle.getEventTime(1)).get(0).text.toString())
+        .isEqualTo("This is the first subtitle.");
+    assertThat(subtitle.getCues(subtitle.getEventTime(1)).get(1).text.toString())
         .isEqualTo("This is the second subtitle \nwith a newline \nand another.");
 
     assertThat(subtitle.getEventTime(2)).isEqualTo(4560000);
-    assertThat(subtitle.getCues(subtitle.getEventTime(2)).get(0).text.toString())
+    assertThat(subtitle.getCues(subtitle.getEventTime(1)).get(0).text.toString())
+        .isEqualTo("This is the first subtitle.");
+    assertThat(subtitle.getCues(subtitle.getEventTime(1)).get(1).text.toString())
+        .isEqualTo("This is the second subtitle \nwith a newline \nand another.");
+    assertThat(subtitle.getCues(subtitle.getEventTime(2)).get(2).text.toString())
         .isEqualTo("This is the third subtitle, with a comma.");
   }