Merge pull request #1315 from datdoantelus:ClosedCaption_708Decoder_bugfix

PiperOrigin-RevId: 636591170

RELEASENOTES.md

@@ -512,6 +512,9 @@ This release includes the following changes since the
         `rowLock` and `columnLock` should both be assumed to be true, regardless
         of the values present in the stream (`columnLock` support is not
         implemented, so it's effectively assumed to always be false).
+    *   CEA-708: Avoid duplicate newlines being added by ExoPlayer's naive
+        handling of the 'set pen location' command
+        ([#1315](https://github.com/androidx/media/pull/1315)).
 *   Image:
     *   Add support for DASH thumbnails. Grid images are cropped and individual
         thumbnails are provided to `ImageOutput` close to their presentation

libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Decoder.java

@@ -778,11 +778,9 @@ public final class Cea708Decoder extends CeaDecoder {
     // first byte
     captionChannelPacketData.skipBits(2); // null padding
     boolean visible = captionChannelPacketData.readBit();
-
     // ANSI/CTA-708-E S-2023 spec (Section 8.4.7) indicates that rowLock and columnLock values in
     // the media should be ignored and assumed to be true.
     captionChannelPacketData.skipBits(2);
-
     int priority = captionChannelPacketData.readBits(3);
     // second byte
     boolean relativePositioning = captionChannelPacketData.readBit();
@@ -1228,6 +1226,8 @@ public final class Cea708Decoder extends CeaDecoder {
             || rolledUpCaptions.size() >= MAXIMUM_ROW_COUNT) {
           rolledUpCaptions.remove(0);
         }
+        // update row value after newline
+        row = rolledUpCaptions.size();
       } else {
         captionStringBuilder.append(text);
       }

libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708DecoderTest.java

@@ -118,6 +118,91 @@ public class Cea708DecoderTest {
     assertThat(getOnlyCue(result).text.toString()).isEqualTo("row3\nrow4");
   }
 
+  /**
+   * ExoPlayer's incomplete implementation of the 'set pen location' command appends a newline if
+   * the 'new' row location is different to the 'current' row (this is to ensure that subtitles that
+   * are meant to be on different lines aren't concatenated together on a single line). This test
+   * demonstrates this, even though the target row is 2, only a single newline is appended.
+   */
+  @Test
+  public void setPenLocation_appendsNewlineIfRowChanges() throws Exception {
+    Cea708Decoder cea708Decoder =
+        new Cea708Decoder(
+
+            /* accessibilityChannel= */ Format.NO_VALUE, /* initializationData= */ null);
+    byte[] windowDefinition =
+        TestUtil.createByteArray(
+            0x98, // DF0 command (define window 0)
+            0b0010_0000, // visible=true, row lock and column lock disabled, priority=0
+            0xF0 | 50, // relative positioning, anchor vertical
+            50, // anchor horizontal
+            10, // anchor point = 0, row count = 10
+            30, // column count = 30
+            0b0000_1001); // window style = 1, pen style = 1
+    byte[] setCurrentWindow = TestUtil.createByteArray(0x80); // CW0 (set current window to 0)
+    // COMMAND_SPL with row 2 and column 0
+    byte[] setPenLocation = TestUtil.createByteArray(0x92, 0x02, 0x00);
+    byte[] subtitleData =
+        encodePacketIntoBytePairs(
+            createPacket(
+                /* sequenceNumber= */ 0,
+                createServiceBlock(
+                    Bytes.concat(
+                        windowDefinition,
+                        setCurrentWindow,
+                        "line1".getBytes(Charsets.UTF_8),
+                        setPenLocation,
+                        "line2".getBytes(Charsets.UTF_8)))));
+
+    Subtitle firstSubtitle = decodeSampleAndCopyResult(cea708Decoder, subtitleData);
+
+    assertThat(getOnlyCue(firstSubtitle).text.toString()).isEqualTo("line1\nline2");
+  }
+
+  /**
+   * ExoPlayer's incomplete implementation of the 'set pen location' command appends a newline if
+   * the 'new' row location is different to the 'current' row (this is to ensure that subtitles that
+   * are meant to be on different lines aren't concatenated together on a single line). This test
+   * ensures that if there's already an explicit newline appended before the command, a duplicate
+   * newline isn't appended.
+   */
+  @Test
+  public void setPenLocation_explicitNewLineBefore_secondNewlineNotAdded() throws Exception {
+    Cea708Decoder cea708Decoder =
+        new Cea708Decoder(
+
+            /* accessibilityChannel= */ Format.NO_VALUE, /* initializationData= */ null);
+    byte[] windowDefinition =
+        TestUtil.createByteArray(
+            0x98, // DF0 command (define window 0)
+            0b0010_0000, // visible=true, row lock and column lock disabled, priority=0
+            0xF0 | 50, // relative positioning, anchor vertical
+            50, // anchor horizontal
+            10, // anchor point = 0, row count = 10
+            30, // column count = 30
+            0b0000_1001); // window style = 1, pen style = 1
+    byte[] setCurrentWindow = TestUtil.createByteArray(0x80); // CW0 (set current window to 0)
+    // COMMAND_SPL with row 1 and column 0
+    byte[] setPenLocation = TestUtil.createByteArray(0x92, 0x01, 0x00);
+    byte[] newLine = TestUtil.createByteArray(0x0D); // new line
+    byte[] subtitleData =
+        encodePacketIntoBytePairs(
+            createPacket(
+                /* sequenceNumber= */ 0,
+                createServiceBlock(
+                    Bytes.concat(
+                        windowDefinition,
+                        setCurrentWindow,
+                        "line1".getBytes(Charsets.UTF_8),
+                        newLine,
+                        setPenLocation,
+                        "line2".getBytes(Charsets.UTF_8)))));
+
+    Subtitle firstSubtitle = decodeSampleAndCopyResult(cea708Decoder, subtitleData);
+
+    assertThat(getOnlyCue(firstSubtitle).text.toString()).isEqualTo("line1\nline2");
+  }
+
   /**
    * Queues {@code sample} to {@code decoder} and dequeues the result, then copies and returns it if
    * it's non-null.