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.

Sign up for GitHub

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 - Check parity and valid bits #5086

Closed
wants to merge 1 commit into from
Closed

Cea608 - Check parity and valid bits #5086

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 64 additions & 4 deletions library/core/src/main/java/com/google/android/exoplayer2/text/cea/Cea608Decoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ public final class Cea608Decoder extends CeaDecoder {

private static final int NTSC_CC_FIELD_1 = 0x00;
private static final int NTSC_CC_FIELD_2 = 0x01;
private static final int CC_VALID_608_ID = 0x04;

private static final int CC_MODE_UNKNOWN = 0;
private static final int CC_MODE_ROLL_UP = 1;
Expand Down Expand Up @@ -179,6 +178,43 @@ public final class Cea608Decoder extends CeaDecoder {
0xC5, 0xE5, 0xD8, 0xF8, 0x250C, 0x2510, 0x2514, 0x2518
};

private static final boolean ODD_PARITY_BYTE_TABLE[] = {
false, true, true, false, true, false, false, true, // For values 0 - 7
true, false, false, true, false, true, true, false, // For values 8 - 15
true, false, false, true, false, true, true, false, // For values 16 - 23
false, true, true, false, true, false, false, true, // For values 24 - 31
true, false, false, true, false, true, true, false, // For values 32 - 39
false, true, true, false, true, false, false, true, // For values 40 - 47
false, true, true, false, true, false, false, true, // For values 48 - 55
true, false, false, true, false, true, true, false, // For values 56 - 63
true, false, false, true, false, true, true, false, // For values 64 - 71
false, true, true, false, true, false, false, true, // For values 72 - 79
false, true, true, false, true, false, false, true, // For values 80 - 87
true, false, false, true, false, true, true, false, // For values 88 - 95
false, true, true, false, true, false, false, true, // For values 96 - 103
true, false, false, true, false, true, true, false, // For values 104 - 111
true, false, false, true, false, true, true, false, // For values 112 - 119
false, true, true, false, true, false, false, true, // For values 120 - 127
true, false, false, true, false, true, true, false, // For values 128 - 135
false, true, true, false, true, false, false, true, // For values 136 - 143
false, true, true, false, true, false, false, true, // For values 144 - 151
true, false, false, true, false, true, true, false, // For values 152 - 159
false, true, true, false, true, false, false, true, // For values 160 - 167
true, false, false, true, false, true, true, false, // For values 168 - 175
true, false, false, true, false, true, true, false, // For values 176 - 183
false, true, true, false, true, false, false, true, // For values 184 - 191
false, true, true, false, true, false, false, true, // For values 192 - 199
true, false, false, true, false, true, true, false, // For values 200 - 207
true, false, false, true, false, true, true, false, // For values 208 - 215
false, true, true, false, true, false, false, true, // For values 216 - 223
true, false, false, true, false, true, true, false, // For values 224 - 231
false, true, true, false, true, false, false, true, // For values 232 - 239
false, true, true, false, true, false, false, true, // For values 240 - 247
true, false, false, true, false, true, true, false, // For values 248 - 255

};


private final ParsableByteArray ccData;
private final int packetLength;
private final int selectedField;
Expand Down Expand Up @@ -259,14 +295,20 @@ protected void decode(SubtitleInputBuffer inputBuffer) {
while (ccData.bytesLeft() >= packetLength) {
byte ccDataHeader = packetLength == 2 ? CC_IMPLICIT_DATA_HEADER
: (byte) ccData.readUnsignedByte();
byte ccData1 = (byte) (ccData.readUnsignedByte() & 0x7F); // strip the parity bit
byte ccData2 = (byte) (ccData.readUnsignedByte() & 0x7F); // strip the parity bit

int ccFullByte1 = ccData.readUnsignedByte();
int ccFullByte2 = ccData.readUnsignedByte();

byte ccData1 = (byte) (ccFullByte1 & 0x7F); // strip the parity bit
byte ccData2 = (byte) (ccFullByte2 & 0x7F); // strip the parity bit

// Only examine valid CEA-608 packets
// TODO: We're currently ignoring the top 5 marker bits, which should all be 1s according
// to the CEA-608 specification. We need to determine if the data should be handled
// differently when that is not the case.
if ((ccDataHeader & (CC_VALID_FLAG | CC_TYPE_FLAG)) != CC_VALID_608_ID) {

if ((ccDataHeader & CC_TYPE_FLAG) != 0) {
// do not process anything that is not part of the 608 byte stream
continue;
}

Expand All @@ -284,6 +326,24 @@ protected void decode(SubtitleInputBuffer inputBuffer) {
// If we've reached this point then there is data to process; flag that work has been done.
captionDataProcessed = true;

if ((ccDataHeader & CC_VALID_FLAG) != CC_VALID_FLAG) {
// The screen needs to be cleared when the closed captions are turned off
// (the encoder flips the validity bit)
resetCueBuilders();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following on from the question of resetting when the valid flag is not set:

Isn't it a problem that above on L323, you do not reset if ccData1 == 0 && ccData2 == 0? If I were going to not set the valid flag, the most obvious thing I'd fill the two bytes with would be zeros. This will cause the caption to not be cleared, however you're saying that clearing is necessary.

The same applies to clearing if the parity is wrong. If that's what needs to happen, I think that decision would somehow need to be made before the ccData1 == 0 && ccData2 == 0 case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You arguments are logical, those are completely valid for on-demand content. I tested various order of the checks, and they fail in case of Over-The-Air content, the only use-case when CEA608 should be used.

We had dozens of iterations of ordering of these checks and this current one is the latest that works with real-life 608 streams. I cannot give real arguments listed in the specification, but most probably:

  • The original content provider (who made the content 20 years ago) were the ones flipping the Validity bit, but they only created the content with one specific frame rate.
  • Any later content distributor might change the frame rate. That requires adding extra bytes to the stream without modifying the timing, so the zero bytes pairs must be introduced - but they must not modify anything else, like clearing the screen. So the zero byte pairs simply must restart the decoding loop, and must be checked earlier than the validity flag. (Later changes not in this change set will introduce additional conditions to solve issues introduced by frame rates not existing when 608 was defined.)
  • The stream corruption (not possible for on-demand content) is most probably introduced by the transmission, should be rare and it should not influence any of the additional states I will later introduce to these checks above. So corruption is checked last in this case.

Please run these changes with over-the-air content, and you will see it renders captions with much better accuracy than it was before. Unfortunately, I still have dozens of changes after these ones, and they might depend on the ordering of the checks introduced in this change, so the reason for this current ordering might not be clear at this point. It simply works. If it cases regressions, I might not aware of, or it does not handle some types of content I have not met yet, we can identify those as long as these changes are in the dev branch)

Copy link
Contributor Author

@zsmatyas zsmatyas Nov 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be the hint for the requirement for clearing the screen when valid flag is not set
ANSI / CTA - 608 - ER - 2014 Section C.21:

Display Enable/Disable Logic and Timing (Regulatory)
...
Decoders also should automatically disable the display and erase memory in response to loss of valid data on line 21.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Let's merge this with the order you propose. However note that we can't promise not to change it if we get reports that this is breaking streams being played by other developers / providers.

continue;
}

// When transmission is digital and reliable (TCP) we should not meet any data corruption.
// But any over-the-air content and test streams might give the decoder corrupted data, so we
// need to drop those. Every data byte should have odd parity.
// When corrupt data is met the screen needs to be cleared, so we:
// - reset the cue builders
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address comment on previous pull request about whether resetting is the best thing to do here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, there are conflicting ideas in the legal documents about CEA608. This is from the Sarnoff compliance test document:

When the “NTSC bad parity test” is active, CC1, CC3 and CS1 should not display new captions. CC1and CC3 captions should persist on the screen for approximately one half second of this test condition, then the captions should clear.

In this case the test starts with correct data, then with bad parity after one half second, and they require the screen to be cleared.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking the history of these changes suggest that we did not clear the screen in the first few iterations. And it was not leading previous captions staying on the screen too long. The original 608 documents suggested something like (this is my interpretation of the idea from the 80s)

  1. if the a command is failing the parity check: skip the command, wait for the repetition.
  2. if a repeated command is failing the parity check: decoder acted upon the first one.
  3. if the character fails the parity check, insert a placeholder character.
  4. if the stream is repeatedly failing the parity check, disable the decoding.

With the current over-the-air content and reliable transmission the only one still happening is the last case. The entire stream seems to have random data and we should not let the last caption stay on screen forever. Shall I add a TODO to add additional conditions later?

// - update the screen (by having this check after having captionDataProcessed set above)
if (!ODD_PARITY_BYTE_TABLE[ccFullByte1] || !ODD_PARITY_BYTE_TABLE[ccFullByte2]) {
resetCueBuilders();
continue;
}

// Special North American character set.
// ccData1 - 0|0|0|1|C|0|0|1
// ccData2 - 0|0|1|1|X|X|X|X
Expand Down