Skip to content

Commit e2847b3

Browse files
icbakercopybara-github
authored andcommitted
Re-apply CEA-708 rowLock/columnLock fix
This change was originally made in 6f82491 It was then accidentally lost in when `Cea708Parser` was merged back into `Cea708Decoder` in 51b4fa2. This is the only change made to the actual 'decoding' logic in `Cea708Parser` between it being split from `Cea708Decoder` and merged back in again, all the other changes in this period relate to the implementation of the `SubtitleParser` interface, so don't need to be preserved in `Cea708Decoder`: https://github.com/androidx/media/commits/51b4fa2cc83b60fcb313fd0e6afd2d45fe64e535/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java `Cea608Parser` was also merged back into `Cea608Decoder` in 25498b1 and so is vulnerable to the same risk of accidental loss of changes. To be sure, I also checked the history of this file: https://github.com/androidx/media/commits/25498b151ba298ef359f245e2ed80718b4adf556/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea608Parser.java The only 'decoding logic' change there is 379cb3b, which was also lost in 25498b1. I will send a separate change to resolve this. PiperOrigin-RevId: 635796696
1 parent 17bf47e commit e2847b3

File tree

3 files changed

+50
-19
lines changed

3 files changed

+50
-19
lines changed

RELEASENOTES.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,13 @@
9191
* PGS: Fix run-length decoding to resolve `0` as a color index, instead of
9292
a literal color value
9393
([#1367](https://github.com/androidx/media/pull/1367)).
94+
* CEA-708: Ignore `rowLock` value. The CEA-708-E S-2023 spec states that
95+
`rowLock` and `columnLock` should both be assumed to be true, regardless
96+
of the values present in the stream (`columnLock` support is not
97+
implemented, so it's effectively assumed to always be false).
98+
* This was originally included in the `1.3.0-alpha01` release notes,
99+
but the change was accidentally reverted before the `1.3.0-rc01`
100+
release. This is now fixed, so the change is present again.
94101
* Metadata:
95102
* Fix mapping of MP4 to ID3 sort tags. Previously the 'album sort'
96103
(`soal`), 'artist sort' (`soar`) and 'album artist sort' (`soaa`) MP4

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

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -778,8 +778,11 @@ private void handleDefineWindow(int window) {
778778
// first byte
779779
captionChannelPacketData.skipBits(2); // null padding
780780
boolean visible = captionChannelPacketData.readBit();
781-
boolean rowLock = captionChannelPacketData.readBit();
782-
boolean columnLock = captionChannelPacketData.readBit();
781+
782+
// ANSI/CTA-708-E S-2023 spec (Section 8.4.7) indicates that rowLock and columnLock values in
783+
// the media should be ignored and assumed to be true.
784+
captionChannelPacketData.skipBits(2);
785+
783786
int priority = captionChannelPacketData.readBits(3);
784787
// second byte
785788
boolean relativePositioning = captionChannelPacketData.readBit();
@@ -791,22 +794,20 @@ private void handleDefineWindow(int window) {
791794
int rowCount = captionChannelPacketData.readBits(4);
792795
// fifth byte
793796
captionChannelPacketData.skipBits(2); // null padding
794-
int columnCount = captionChannelPacketData.readBits(6);
797+
// TODO: Add support for column count.
798+
captionChannelPacketData.skipBits(6); // column count
795799
// sixth byte
796800
captionChannelPacketData.skipBits(2); // null padding
797801
int windowStyle = captionChannelPacketData.readBits(3);
798802
int penStyle = captionChannelPacketData.readBits(3);
799803

800804
cueInfoBuilder.defineWindow(
801805
visible,
802-
rowLock,
803-
columnLock,
804806
priority,
805807
relativePositioning,
806808
verticalAnchor,
807809
horizontalAnchor,
808810
rowCount,
809-
columnCount,
810811
anchorId,
811812
windowStyle,
812813
penStyle);
@@ -967,7 +968,6 @@ private static final class CueInfoBuilder {
967968
private int horizontalAnchor;
968969
private int anchorId;
969970
private int rowCount;
970-
private boolean rowLock;
971971
private int justification;
972972
private int windowStyleId;
973973
private int penStyleId;
@@ -1003,7 +1003,6 @@ public void reset() {
10031003
horizontalAnchor = 0;
10041004
anchorId = 0;
10051005
rowCount = MAXIMUM_ROW_COUNT;
1006-
rowLock = true;
10071006
justification = JUSTIFICATION_LEFT;
10081007
windowStyleId = 0;
10091008
penStyleId = 0;
@@ -1037,20 +1036,16 @@ public boolean isVisible() {
10371036

10381037
public void defineWindow(
10391038
boolean visible,
1040-
boolean rowLock,
1041-
boolean columnLock,
10421039
int priority,
10431040
boolean relativePositioning,
10441041
int verticalAnchor,
10451042
int horizontalAnchor,
10461043
int rowCount,
1047-
int columnCount,
10481044
int anchorId,
10491045
int windowStyleId,
10501046
int penStyleId) {
10511047
this.defined = true;
10521048
this.visible = visible;
1053-
this.rowLock = rowLock;
10541049
this.priority = priority;
10551050
this.relativePositioning = relativePositioning;
10561051
this.verticalAnchor = verticalAnchor;
@@ -1062,14 +1057,12 @@ public void defineWindow(
10621057
this.rowCount = rowCount + 1;
10631058

10641059
// Trim any rolled up captions that are no longer valid, if applicable.
1065-
while ((rowLock && (rolledUpCaptions.size() >= this.rowCount))
1066-
|| (rolledUpCaptions.size() >= MAXIMUM_ROW_COUNT)) {
1060+
while (rolledUpCaptions.size() >= this.rowCount
1061+
|| rolledUpCaptions.size() >= MAXIMUM_ROW_COUNT) {
10671062
rolledUpCaptions.remove(0);
10681063
}
10691064
}
10701065

1071-
// TODO: Add support for column lock and count.
1072-
10731066
if (windowStyleId != 0 && this.windowStyleId != windowStyleId) {
10741067
this.windowStyleId = windowStyleId;
10751068
// windowStyleId is 1-based.
@@ -1231,8 +1224,8 @@ public void append(char text) {
12311224
backgroundColorStartPosition = 0;
12321225
}
12331226

1234-
while ((rowLock && (rolledUpCaptions.size() >= rowCount))
1235-
|| (rolledUpCaptions.size() >= MAXIMUM_ROW_COUNT)) {
1227+
while (rolledUpCaptions.size() >= rowCount
1228+
|| rolledUpCaptions.size() >= MAXIMUM_ROW_COUNT) {
12361229
rolledUpCaptions.remove(0);
12371230
}
12381231
} else {

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

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ public void setupLogging() {
6060
public void singleServiceAndWindowDefinition() throws Exception {
6161
Cea708Decoder cea708Decoder =
6262
new Cea708Decoder(
63-
6463
/* accessibilityChannel= */ Format.NO_VALUE, /* initializationData= */ null);
6564
byte[] windowDefinition =
6665
TestUtil.createByteArray(
@@ -87,6 +86,38 @@ public void singleServiceAndWindowDefinition() throws Exception {
8786
assertThat(getOnlyCue(firstSubtitle).text.toString()).isEqualTo("test subtitle");
8887
}
8988

89+
@Test
90+
public void singleServiceAndWindowDefinition_ignoresRowLock() throws Exception {
91+
Cea708Decoder cea708Decoder =
92+
new Cea708Decoder(
93+
/* accessibilityChannel= */ Format.NO_VALUE, /* initializationData= */ null);
94+
byte[] windowDefinition =
95+
TestUtil.createByteArray(
96+
0x98, // DF0 command (define window 0)
97+
0b0010_0000, // visible=true, row lock and column lock disabled, priority=0
98+
0xF0 | 50, // relative positioning, anchor vertical
99+
50, // anchor horizontal
100+
1, // anchor point = 0, row count = 1
101+
30, // column count = 30
102+
0b0000_1001); // window style = 1, pen style = 1
103+
byte[] setCurrentWindow = TestUtil.createByteArray(0x80); // CW0 (set current window to 0)
104+
byte[] subtitleData =
105+
encodePacketIntoBytePairs(
106+
createPacket(
107+
/* sequenceNumber= */ 0,
108+
createServiceBlock(
109+
Bytes.concat(
110+
windowDefinition,
111+
setCurrentWindow,
112+
"row1\r\nrow2\r\nrow3\r\nrow4".getBytes(Charsets.UTF_8)))));
113+
114+
Subtitle result = decodeSampleAndCopyResult(cea708Decoder, subtitleData);
115+
116+
// Row count is 1 (which means 2 rows should be kept). Row lock is disabled in the media,
117+
// but this is ignored and the result is still truncated to only the last two rows.
118+
assertThat(getOnlyCue(result).text.toString()).isEqualTo("row3\nrow4");
119+
}
120+
90121
/**
91122
* Queues {@code sample} to {@code decoder} and dequeues the result, then copies and returns it if
92123
* it's non-null.

0 commit comments

Comments
 (0)