Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CEA-708: Update current row value when new line is added #1315

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -778,11 +778,9 @@ private void handleDefineWindow(int window) {
// 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();
Expand Down Expand Up @@ -1228,6 +1226,8 @@ public void append(char text) {
|| rolledUpCaptions.size() >= MAXIMUM_ROW_COUNT) {
rolledUpCaptions.remove(0);
}
// update row value after newline
row = rolledUpCaptions.size();
} else {
captionStringBuilder.append(text);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,91 @@ public void singleServiceAndWindowDefinition_ignoresRowLock() throws Exception {
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.
Expand Down