CEA-708 Decoder fixes for issue #1807. #8356
Conversation
In this commit - Handling the sequence number discontinuity in caption channel packet header. Handle the multiple Service Blocks within a single caption channel packet. An outer while loop is added to function processCurrentPacket() to achieve that. The processCurrentPacket returns if the packet length does not match with the currentIndex. That assumption is wrong. As per spec the the packet can end on reception of next cc_type = 0x3. Added a logic to handle multi byte commands. These commands can be within the service block or spread across service blocks. Processing of service block needs to wait until all the command bytes arrive. An ArrayList is used to accumulate the bytes belonging to same service block. In this change the current flow is not altered, i.e. the parsing of service block is not started until the complete caption channel packet arrives. These changes are verified through Sarnoff Test streams. Many tests are still failing after this change. Subsequent changes are planned to address that.
library/core/src/main/java/com/google/android/exoplayer2/text/cea/Cea708Decoder.java
Show resolved
Hide resolved
| // blocks in the current packet. If the previous command spread across the paket, that is | ||
| // accumulated and handled after all the bytes are received | ||
| while (currOffset < currentDtvCcPacket.currentIndex) { | ||
| int serviceNumber = (currentDtvCcPacket.packetData[currOffset] & 0xE0) >> 5; //3 bits |
There was a problem hiding this comment.
It seems far preferable to leverage ParsableBitArray rather than manually bit shifting.
I wonder if it's possible to re-use the serviceBlockPacket one everywhere by some cunning position checking (and rename it allServiceBlocks?), or failing that to introduce a second one called allServiceBlocks to separate the two.
I had a go locally with the first approach (use serviceBlockPacket everywhere) by making edits on top of this change - but I ran into a lack of understanding of exactly how the code is meant to behave (see my next comment) which I think affects how feasible that approach will be.
There was a problem hiding this comment.
Thanks @icbaker. My understanding of ParsableBitArray is that it can not be resized after creation. As per CEA-708 spec, the cc_data() can be split across multiple caption channel packets (section 4.4.1.1 - A DTVCC Caption Channel Packet may continue from one cc_data() structure to the next). In such case, we need to retain the previous data to interpret the commands correctly. But ParsableBitArray.reset() clears everything. If we could extend ParsableBitArray with append feature, I think we can still use ParsableBitArray here. As a simple workaround, I used ArrayList which can be resized easily.
In this change - Handling the sequence number discontinuity in caption channel packet header. The processCurrentPacket returns if the packet length does not match with the currentIndex. That assumption is wrong. As per spec the the packet can end on reception of next cc_type = 0x3.
The handling sequence discontinuity and the packet with size less the packet_size is separated out to another PR google#8415
|
@icbaker any suggestion how can we add unit tests for these files (Cea708Decoder and Cea608Decoder)? |
In this change - Handling the sequence number discontinuity in caption channel packet header. The processCurrentPacket returns if the packet length does not match with the currentIndex. That assumption is wrong. As per spec the the packet can end on reception of next cc_type = 0x3.
|
Closing all PRs on this deprecated project. We are now unable to merge PRs from here. If you would like us to consider this change again please file a new PR on the media3 project: https:/androidx/media/pulls |
This pull request is for issue#1807. In this commit -
Handling the sequence number discontinuity in caption channel packet header.
Handle the multiple Service Blocks within a single caption channel packet. An outer while loop is added to function processCurrentPacket() to achieve that.
The processCurrentPacket returns if the packet length does not match with the currentIndex. That assumption is wrong.
As per spec the the packet can end on reception of next cc_type = 0x3.
Added a logic to handle multi byte commands. These commands can be within the service block or spread across service blocks.
Processing of service block needs to wait until all the command bytes arrive. An ArrayList is used to accumulate the bytes belonging to same service block.
In this change the current flow is not altered, i.e. the parsing of service block is not started until the complete caption channel packet arrives.
These changes are verified through Sarnoff Test streams. Many tests are still failing after this change. Subsequent changes are planned to address them.