Re-apply CEA-708 rowLock/columnLock fix

This change was originally made in 6f8249184b

It was then accidentally lost in when `Cea708Parser` was merged back
into `Cea708Decoder` in 51b4fa2cc8.

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`:
51b4fa2cc8/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java

`Cea608Parser` was also merged back into `Cea608Decoder` in
25498b151b
and so is vulnerable to the same risk of accidental loss of changes. To
be sure, I also checked the history of this file:
25498b151b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea608Parser.java

The only 'decoding logic' change there is 379cb3ba54,
which was also lost in 25498b151b.
I will send a separate change to resolve this.

PiperOrigin-RevId: 635796696
This commit is contained in:
ibaker 2024-05-21 07:13:24 -07:00 committed by Copybara-Service
parent 17bf47ed24
commit e2847b3b80
3 changed files with 50 additions and 19 deletions

View File

@ -91,6 +91,13 @@
* PGS: Fix run-length decoding to resolve `0` as a color index, instead of * PGS: Fix run-length decoding to resolve `0` as a color index, instead of
a literal color value a literal color value
([#1367](https://github.com/androidx/media/pull/1367)). ([#1367](https://github.com/androidx/media/pull/1367)).
* 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).
* This was originally included in the `1.3.0-alpha01` release notes,
but the change was accidentally reverted before the `1.3.0-rc01`
release. This is now fixed, so the change is present again.
* Metadata: * Metadata:
* Fix mapping of MP4 to ID3 sort tags. Previously the 'album sort' * Fix mapping of MP4 to ID3 sort tags. Previously the 'album sort'
(`soal`), 'artist sort' (`soar`) and 'album artist sort' (`soaa`) MP4 (`soal`), 'artist sort' (`soar`) and 'album artist sort' (`soaa`) MP4

View File

@ -778,8 +778,11 @@ public final class Cea708Decoder extends CeaDecoder {
// first byte // first byte
captionChannelPacketData.skipBits(2); // null padding captionChannelPacketData.skipBits(2); // null padding
boolean visible = captionChannelPacketData.readBit(); 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 values in
// the media should be ignored and assumed to be true.
captionChannelPacketData.skipBits(2);
int priority = captionChannelPacketData.readBits(3); int priority = captionChannelPacketData.readBits(3);
// second byte // second byte
boolean relativePositioning = captionChannelPacketData.readBit(); boolean relativePositioning = captionChannelPacketData.readBit();
@ -791,7 +794,8 @@ public final class Cea708Decoder extends CeaDecoder {
int rowCount = captionChannelPacketData.readBits(4); int rowCount = captionChannelPacketData.readBits(4);
// fifth byte // fifth byte
captionChannelPacketData.skipBits(2); // null padding captionChannelPacketData.skipBits(2); // null padding
int columnCount = captionChannelPacketData.readBits(6); // TODO: Add support for column count.
captionChannelPacketData.skipBits(6); // column count
// sixth byte // sixth byte
captionChannelPacketData.skipBits(2); // null padding captionChannelPacketData.skipBits(2); // null padding
int windowStyle = captionChannelPacketData.readBits(3); int windowStyle = captionChannelPacketData.readBits(3);
@ -799,14 +803,11 @@ public final class Cea708Decoder extends CeaDecoder {
cueInfoBuilder.defineWindow( cueInfoBuilder.defineWindow(
visible, visible,
rowLock,
columnLock,
priority, priority,
relativePositioning, relativePositioning,
verticalAnchor, verticalAnchor,
horizontalAnchor, horizontalAnchor,
rowCount, rowCount,
columnCount,
anchorId, anchorId,
windowStyle, windowStyle,
penStyle); penStyle);
@ -967,7 +968,6 @@ public final class Cea708Decoder extends CeaDecoder {
private int horizontalAnchor; private int horizontalAnchor;
private int anchorId; private int anchorId;
private int rowCount; private int rowCount;
private boolean rowLock;
private int justification; private int justification;
private int windowStyleId; private int windowStyleId;
private int penStyleId; private int penStyleId;
@ -1003,7 +1003,6 @@ public final class Cea708Decoder extends CeaDecoder {
horizontalAnchor = 0; horizontalAnchor = 0;
anchorId = 0; anchorId = 0;
rowCount = MAXIMUM_ROW_COUNT; rowCount = MAXIMUM_ROW_COUNT;
rowLock = true;
justification = JUSTIFICATION_LEFT; justification = JUSTIFICATION_LEFT;
windowStyleId = 0; windowStyleId = 0;
penStyleId = 0; penStyleId = 0;
@ -1037,20 +1036,16 @@ public final class Cea708Decoder extends CeaDecoder {
public void defineWindow( public void defineWindow(
boolean visible, boolean visible,
boolean rowLock,
boolean columnLock,
int priority, int priority,
boolean relativePositioning, boolean relativePositioning,
int verticalAnchor, int verticalAnchor,
int horizontalAnchor, int horizontalAnchor,
int rowCount, int rowCount,
int columnCount,
int anchorId, int anchorId,
int windowStyleId, int windowStyleId,
int penStyleId) { int penStyleId) {
this.defined = true; this.defined = true;
this.visible = visible; this.visible = visible;
this.rowLock = rowLock;
this.priority = priority; this.priority = priority;
this.relativePositioning = relativePositioning; this.relativePositioning = relativePositioning;
this.verticalAnchor = verticalAnchor; this.verticalAnchor = verticalAnchor;
@ -1062,14 +1057,12 @@ public final class Cea708Decoder extends CeaDecoder {
this.rowCount = rowCount + 1; this.rowCount = rowCount + 1;
// Trim any rolled up captions that are no longer valid, if applicable. // 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.size() >= MAXIMUM_ROW_COUNT) {
rolledUpCaptions.remove(0); rolledUpCaptions.remove(0);
} }
} }
// TODO: Add support for column lock and count.
if (windowStyleId != 0 && this.windowStyleId != windowStyleId) { if (windowStyleId != 0 && this.windowStyleId != windowStyleId) {
this.windowStyleId = windowStyleId; this.windowStyleId = windowStyleId;
// windowStyleId is 1-based. // windowStyleId is 1-based.
@ -1231,8 +1224,8 @@ public final class Cea708Decoder extends CeaDecoder {
backgroundColorStartPosition = 0; backgroundColorStartPosition = 0;
} }
while ((rowLock && (rolledUpCaptions.size() >= rowCount)) while (rolledUpCaptions.size() >= rowCount
|| (rolledUpCaptions.size() >= MAXIMUM_ROW_COUNT)) { || rolledUpCaptions.size() >= MAXIMUM_ROW_COUNT) {
rolledUpCaptions.remove(0); rolledUpCaptions.remove(0);
} }
} else { } else {

View File

@ -60,7 +60,6 @@ public class Cea708DecoderTest {
public void singleServiceAndWindowDefinition() throws Exception { public void singleServiceAndWindowDefinition() throws Exception {
Cea708Decoder cea708Decoder = Cea708Decoder cea708Decoder =
new Cea708Decoder( new Cea708Decoder(
/* accessibilityChannel= */ Format.NO_VALUE, /* initializationData= */ null); /* accessibilityChannel= */ Format.NO_VALUE, /* initializationData= */ null);
byte[] windowDefinition = byte[] windowDefinition =
TestUtil.createByteArray( TestUtil.createByteArray(
@ -87,6 +86,38 @@ public class Cea708DecoderTest {
assertThat(getOnlyCue(firstSubtitle).text.toString()).isEqualTo("test subtitle"); assertThat(getOnlyCue(firstSubtitle).text.toString()).isEqualTo("test subtitle");
} }
@Test
public void singleServiceAndWindowDefinition_ignoresRowLock() 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
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)
byte[] subtitleData =
encodePacketIntoBytePairs(
createPacket(
/* sequenceNumber= */ 0,
createServiceBlock(
Bytes.concat(
windowDefinition,
setCurrentWindow,
"row1\r\nrow2\r\nrow3\r\nrow4".getBytes(Charsets.UTF_8)))));
Subtitle result = decodeSampleAndCopyResult(cea708Decoder, subtitleData);
// 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(getOnlyCue(result).text.toString()).isEqualTo("row3\nrow4");
}
/** /**
* Queues {@code sample} to {@code decoder} and dequeues the result, then copies and returns it if * Queues {@code sample} to {@code decoder} and dequeues the result, then copies and returns it if
* it's non-null. * it's non-null.