-
Notifications
You must be signed in to change notification settings - Fork 6k
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
CEA608: Handling XDS and TEXT modes #5807
Conversation
[Problem] There are 3 services / modes transported on line 21: - Captioning - TEXT (generally not program related) - XDS (eXtended Data Services) Bytes belonging to the unsupported modes are interleaved with the bytes of the captioning mode. See Chapter 7, Chapter 8.5 and Chapter 8.6 of the CEA608 Standard for more details. [Solution] Drop all bytes belonging to unsupported modes. [Test] - All streams containing only captioning services should not be influenced - Test all 4 CEA 608 channels with live over-the-air content and using all available TEXT and XDS streams.
@@ -306,6 +330,38 @@ protected Subtitle createSubtitle() { | |||
return new CeaSubtitle(cues); | |||
} | |||
|
|||
private boolean isCodeForUnsupportedMode(byte cc1, byte cc2) { | |||
// Control codes from 0x01 to 0x0F indicate the beginning of XDS Data | |||
if (0x01 <= cc1 && cc1 <= 0x0F) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Section 8.5 C of the CEA608 Standard:
When any of the control codes from 0x01 to 0x0F is used to begin a control-character pair it indicates the beginning of XDS Data. This XDS data should be considered part of a different data channel and can be used to interrupt the Text data according to the FCC rules for other data channels.
} | ||
|
||
switch (cc2) { | ||
case CTRL_END_OF_CAPTION: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See C.16 of the CEA608 standard.
EDM or ENM command does not instruct the decoder to treat subsequent data as caption data
...
There are six caption-specific commands which do terminate a Text Mode data stream: EOC, RCL, RDC, RU2, RU3, and RU4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in general. Just some code structure comments.
library/core/src/main/java/com/google/android/exoplayer2/text/cea/Cea608Decoder.java
Outdated
Show resolved
Hide resolved
library/core/src/main/java/com/google/android/exoplayer2/text/cea/Cea608Decoder.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
// 2 commands switch to TEXT mode. | ||
if (((cc1 & 0xF7) == 0x14) // first byte must be 0x14 or 0x1C based on channel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move (cc1 & 0xF7) == 0x14 to a private static method isModeSwitchCommand(cc1) for readability? And use the same method on L349 below. Then you only need to add the explaining comment once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could not come up with a better name, but the suggested function signature could be misleading. Usually both bytes are needed to figure out what the command is doing, and there could be a long range of values this function returns true, although there is no mode switch expected.
@@ -363,6 +419,16 @@ protected void decode(SubtitleInputBuffer inputBuffer) { | |||
continue; | |||
} | |||
|
|||
if (isCodeForUnsupportedMode(ccData1, ccData2)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm not mistaken, it seems that this code can move to handleCrtl? That is probably a better place to handle these transitions as it's part of "control code handling" in general.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think so. We need stop processing all data (characters included) in case we are not in CAPTION mode. Moving this down would add the characters of the TEXT and XDS modes to the buffers of our processing of CAPTION mode.
Added the suggested improvements as a new commit, now the comments given to the first commit might point to locations that might be "Outdated" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be able to merge now. I'll do some further formatting changes on our side, but otherwise looks good.
[Problem]
There are 3 services / modes transported on line 21:
Bytes belonging to the unsupported modes are interleaved with the
bytes of the captioning mode.
See Chapter 7, Chapter 8.5 and Chapter 8.6 of the CEA608 Standard for
more details.
[Solution]
Drop all bytes belonging to unsupported modes.
[Test]
using all available TEXT and XDS streams.
[Links]
https://www.law.cornell.edu/cfr/text/47/79.101
https://en.wikipedia.org/wiki/EIA-608#Channels