From 8b39f7e2876ecf48348927468934ff901c826b3c Mon Sep 17 00:00:00 2001 From: datdoantelus Date: Tue, 2 Jan 2024 14:18:51 -0500 Subject: [PATCH 1/5] Ignore rowLock and columnLock values as specified by ANSI/CTA-708-E S-2023 document (section 8.4.7) --- .../androidx/media3/extractor/text/cea/Cea708Parser.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java index e5ff146f52..ec6822f509 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java @@ -783,8 +783,12 @@ public final class Cea708Parser implements SubtitleParser { // first byte captionChannelPacketData.skipBits(2); // null padding boolean visible = captionChannelPacketData.readBit(); - boolean rowLock = captionChannelPacketData.readBit(); - boolean columnLock = captionChannelPacketData.readBit(); + + // ANSI/CTA-708-E S-2023 spec (Section 8.4.7) indicates that rowLock and columnLock should be ignored and assumed to be lock + captionChannelPacketData.readBits(2); + boolean rowLock = true; + boolean columnLock = true; + int priority = captionChannelPacketData.readBits(3); // second byte boolean relativePositioning = captionChannelPacketData.readBit(); From 28eb55c077efcf8ab739acbb208e6bbe106c91d0 Mon Sep 17 00:00:00 2001 From: datdoantelus Date: Wed, 3 Jan 2024 09:26:51 -0500 Subject: [PATCH 2/5] Use skipBits(2) instead of readBits for ignored bits. Add singleServiceAndWindowDefinition_ignoreRowLock test to Cea708ParserTest --- .../extractor/text/cea/Cea708Parser.java | 2 +- .../extractor/text/cea/Cea708ParserTest.java | 42 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java index ec6822f509..9f8002f8de 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java @@ -785,7 +785,7 @@ public final class Cea708Parser implements SubtitleParser { boolean visible = captionChannelPacketData.readBit(); // ANSI/CTA-708-E S-2023 spec (Section 8.4.7) indicates that rowLock and columnLock should be ignored and assumed to be lock - captionChannelPacketData.readBits(2); + captionChannelPacketData.skipBits(2); boolean rowLock = true; boolean columnLock = true; diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java index 3910fa0093..c1bbc2fbed 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java @@ -42,6 +42,7 @@ public class Cea708ParserTest { private static final byte CHANNEL_PACKET_DATA = 0x6; private static final byte CHANNEL_PACKET_END = 0x2; + @Test public void singleServiceAndWindowDefinition() throws Exception { Cea708Parser cea708Parser = @@ -115,6 +116,47 @@ public class Cea708ParserTest { .isEqualTo("test subtitle"); } + @Test + public void singleServiceAndWindowDefinition_ignoreRowLock() throws Exception { + Cea708Parser cea708Parser = + new Cea708Parser( + /* 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 + 1, // 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) + byte[] subtitleData = + encodePacketIntoBytePairs( + createPacket( + /* sequenceNumber= */ 0, + createServiceBlock( + Bytes.concat( + windowDefinition, + setCurrentWindow, + "row1\r\nrow2\r\nrow3\r\nrow4".getBytes(Charsets.US_ASCII))))); + byte[] garbagePrefix = TestUtil.buildTestData(subtitleData.length * 2); + byte[] garbageSuffix = TestUtil.buildTestData(10); + byte[] subtitleDataWithGarbagePrefixAndSuffix = + Bytes.concat(garbagePrefix, subtitleData, garbageSuffix); + + List result = new ArrayList<>(); + cea708Parser.parse( + subtitleDataWithGarbagePrefixAndSuffix, + garbagePrefix.length, + subtitleData.length, + SubtitleParser.OutputOptions.allCues(), + result::add); + + assertThat(Iterables.getOnlyElement(Iterables.getOnlyElement(result).cues).text.toString()) + .isEqualTo("row3\nrow4"); + } + /** See section 4.4.1 of the CEA-708-B spec. */ private static byte[] encodePacketIntoBytePairs(byte[] packet) { checkState(packet.length % 2 == 0); From a58ddcfa2314886ad433e14bc571714b8bcd8ff0 Mon Sep 17 00:00:00 2001 From: datdoantelus Date: Wed, 3 Jan 2024 09:35:46 -0500 Subject: [PATCH 3/5] Update test comment. --- .../androidx/media3/extractor/text/cea/Cea708ParserTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java index c1bbc2fbed..edfcfeb894 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java @@ -127,7 +127,7 @@ public class Cea708ParserTest { 0b0010_0000, // visible=true, row lock and column lock disabled, priority=0 0xF0 | 50, // relative positioning, anchor vertical 50, // anchor horizontal - 1, // anchor point = 0, row count = 10 + 1, // anchor point = 0, row count = 1 30, // column count = 30 0b0000_1001); // window style = 1, pen style = 1 byte[] setCurrentWindow = TestUtil.createByteArray(0x80); // CW0 (set current window to 0) From 04751a1cdb035a3353d2735077fcaa66bac718b0 Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Wed, 3 Jan 2024 17:33:45 +0000 Subject: [PATCH 4/5] Format with google-java-format --- .../java/androidx/media3/extractor/text/cea/Cea708Parser.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java index 9f8002f8de..1b2c2fb7f3 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java @@ -784,7 +784,8 @@ public final class Cea708Parser implements SubtitleParser { 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 should be ignored and assumed to be lock + // ANSI/CTA-708-E S-2023 spec (Section 8.4.7) indicates that rowLock and columnLock should be + // ignored and assumed to be lock captionChannelPacketData.skipBits(2); boolean rowLock = true; boolean columnLock = true; From 0b482ef58c7ada1541184567cc624e4c2e20ed7f Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Wed, 3 Jan 2024 17:43:21 +0000 Subject: [PATCH 5/5] Remove columnLock, columnCount (unimplemented) and rowLock (always true) Also add a release note and simplify the test by removing the garbage prefix and suffix. --- RELEASENOTES.md | 4 ++++ .../extractor/text/cea/Cea708Parser.java | 24 +++++-------------- .../extractor/text/cea/Cea708ParserTest.java | 15 ++++-------- 3 files changed, 14 insertions(+), 29 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 5b32e1bb58..70a405be8e 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -93,6 +93,10 @@ that contained Binder objects` error when using `DefaultExtractorsFactory.setTextTrackTranscodingEnabled` ([#836](https://github.com/androidx/media/issues/836)). + * CEA-708: Ignore `rowLock` value. The CEA-708-E S-2023 spec states that + `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). * Metadata: * Fix bug where `MediaMetadata` was only populated from Vorbis comments with upper-case keys diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java index 1b2c2fb7f3..1c3861c4ee 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java @@ -784,11 +784,9 @@ public final class Cea708Parser implements SubtitleParser { 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 should be - // ignored and assumed to be lock + // 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); - boolean rowLock = true; - boolean columnLock = true; int priority = captionChannelPacketData.readBits(3); // second byte @@ -801,7 +799,8 @@ public final class Cea708Parser implements SubtitleParser { int rowCount = captionChannelPacketData.readBits(4); // fifth byte captionChannelPacketData.skipBits(2); // null padding - int columnCount = captionChannelPacketData.readBits(6); + // TODO: Add support for column count. + captionChannelPacketData.skipBits(6); // column count // sixth byte captionChannelPacketData.skipBits(2); // null padding int windowStyle = captionChannelPacketData.readBits(3); @@ -809,14 +808,11 @@ public final class Cea708Parser implements SubtitleParser { cueInfoBuilder.defineWindow( visible, - rowLock, - columnLock, priority, relativePositioning, verticalAnchor, horizontalAnchor, rowCount, - columnCount, anchorId, windowStyle, penStyle); @@ -977,7 +973,6 @@ public final class Cea708Parser implements SubtitleParser { private int horizontalAnchor; private int anchorId; private int rowCount; - private boolean rowLock; private int justification; private int windowStyleId; private int penStyleId; @@ -1013,7 +1008,6 @@ public final class Cea708Parser implements SubtitleParser { horizontalAnchor = 0; anchorId = 0; rowCount = MAXIMUM_ROW_COUNT; - rowLock = true; justification = JUSTIFICATION_LEFT; windowStyleId = 0; penStyleId = 0; @@ -1047,20 +1041,16 @@ public final class Cea708Parser implements SubtitleParser { public void defineWindow( boolean visible, - boolean rowLock, - boolean columnLock, int priority, boolean relativePositioning, int verticalAnchor, int horizontalAnchor, int rowCount, - int columnCount, int anchorId, int windowStyleId, int penStyleId) { this.defined = true; this.visible = visible; - this.rowLock = rowLock; this.priority = priority; this.relativePositioning = relativePositioning; this.verticalAnchor = verticalAnchor; @@ -1072,14 +1062,12 @@ public final class Cea708Parser implements SubtitleParser { this.rowCount = rowCount + 1; // Trim any rolled up captions that are no longer valid, if applicable. - while ((rowLock && (rolledUpCaptions.size() >= this.rowCount)) + while ((rolledUpCaptions.size() >= this.rowCount) || (rolledUpCaptions.size() >= MAXIMUM_ROW_COUNT)) { rolledUpCaptions.remove(0); } } - // TODO: Add support for column lock and count. - if (windowStyleId != 0 && this.windowStyleId != windowStyleId) { this.windowStyleId = windowStyleId; // windowStyleId is 1-based. @@ -1241,7 +1229,7 @@ public final class Cea708Parser implements SubtitleParser { backgroundColorStartPosition = 0; } - while ((rowLock && (rolledUpCaptions.size() >= rowCount)) + while ((rolledUpCaptions.size() >= rowCount) || (rolledUpCaptions.size() >= MAXIMUM_ROW_COUNT)) { rolledUpCaptions.remove(0); } diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java index edfcfeb894..5eaf6c51a9 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java @@ -139,20 +139,13 @@ public class Cea708ParserTest { Bytes.concat( windowDefinition, setCurrentWindow, - "row1\r\nrow2\r\nrow3\r\nrow4".getBytes(Charsets.US_ASCII))))); - byte[] garbagePrefix = TestUtil.buildTestData(subtitleData.length * 2); - byte[] garbageSuffix = TestUtil.buildTestData(10); - byte[] subtitleDataWithGarbagePrefixAndSuffix = - Bytes.concat(garbagePrefix, subtitleData, garbageSuffix); + "row1\r\nrow2\r\nrow3\r\nrow4".getBytes(Charsets.UTF_8))))); List result = new ArrayList<>(); - cea708Parser.parse( - subtitleDataWithGarbagePrefixAndSuffix, - garbagePrefix.length, - subtitleData.length, - SubtitleParser.OutputOptions.allCues(), - result::add); + cea708Parser.parse(subtitleData, SubtitleParser.OutputOptions.allCues(), result::add); + // Row count is 1 (which means 2 rows should be kept). Row lock is disabled in the media, + // but this is ignored and the result is still truncated to only the last two rows. assertThat(Iterables.getOnlyElement(Iterables.getOnlyElement(result).cues).text.toString()) .isEqualTo("row3\nrow4"); }