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

EAI-708 support #1807

Open
zsmatyas opened this issue Sep 8, 2016 · 13 comments
Open

EAI-708 support #1807

zsmatyas opened this issue Sep 8, 2016 · 13 comments
Assignees

Comments

@zsmatyas
Copy link
Contributor

zsmatyas commented Sep 8, 2016

Previous version of the v1 and v2-dev branches had the entire SEI data put into a buffer and then read by the Eia608Parser. The parser had a check that the type and validity bits are set to channel 1 of the 608 content, and everything else was dropped. See: V1
if (ccType != 0) { seiBuffer.skipBits(16); continue; }
Here the ccType values mean:

  • 0 = channel 1 and 2 of EIA-608
  • 1 = channel 3 and 4 of EIA-608
  • 2 = Start of EIA-708 content (channel ID is defined inside the stream)
  • 3 = Chunk of EIA-708 content

As only value 0 was handled, and no additional checks were used to separate channel 1 and 2: a single 608 channel was supported. I added my additional channel and EIA-708 support by updating the Eia608Parser to collect the data instead of dropping it, so channel 3-4 and EIA-708 bits were collected and parsed appropriately.

In the latest v2 code, all these additional data are dropped at SeiReader level:
V2
like:
if (ccValidityAndType != 0x04) { seiBuffer.skipBytes(2); }
The change was: Link

SeiReader seems to be the correct location to filter 608/708 and channel information, but needs serious changes in the architecture I had. Are you guys planning to ever support EIA-708 (as it is an FCC requirement from 2017)? Did you consider supporting the other channels of EIA-608? Any similar changes like the one you made in the v2-dev branch requires lots of changes to support all subtitles correctly, that is why I am interested in the plans about supported formats.

Are you willing to accept pull requests related to these features or are you maybe already working on these issues?

Big Thanks!

@ojw28
Copy link
Contributor

ojw28 commented Sep 8, 2016

@cdrolle is looking at 708 support. Could you comment on the above? Thanks!

@zsmatyas
Copy link
Contributor Author

Any news? 🐔

@zsmatyas
Copy link
Contributor Author

🍯

@zsmatyas
Copy link
Contributor Author

Any comments about your implementation plans? I have the 708 parser implemented, I would gladly contribute if our code changes can be aligned. (As my V1 parsing is not compatible with the current state of V2)

@ojw28
Copy link
Contributor

ojw28 commented Jan 17, 2017

We have some 708 code internally that hasn't been pushed to GitHub yet. @cdrolle - Can you make it happen? Thanks.

ojw28 pushed a commit that referenced this issue Jan 17, 2017
Issue: #1807

-------------
Created by MOE: https:/google/moe
MOE_MIGRATED_REVID=144726542
@ojw28
Copy link
Contributor

ojw28 commented Jan 17, 2017

The change above adds what we've done so far, but it's not yet a complete solution. Some further work is needed on the MediaSource/Extraction side to expose the CEA-708 tracks so that they can be selected for rendering.

@roticv
Copy link

roticv commented Sep 14, 2017

What's the current state of getting the github version up to date with the 708 changes?

@ebr11
Copy link

ebr11 commented Oct 9, 2018

Hi. Has there been any further movement on this 708 support?

@roticv
Copy link

roticv commented Oct 9, 2018

@zsmatyas
Copy link
Contributor Author

zsmatyas commented Oct 9, 2018

@roticv That is for 608, not 708. I think there are a few pull requests pending for quite improved 608 and 708 support. Unfortunately, it seems they wont be merged in this form as reviewing them is quite difficult.

Reworking the pull requests into smaller digestible changes will take some time.

@sneelavara
Copy link
Contributor

@ojw28 is there any plan to add full support for CEA-708. We have requirement to enable CEA-708. It would be helpful if this gets some priority.

@zsmatyas we have tried Sarnoff streams with #4595 changes. It works much better with that. But still there are many outstanding issues. We are happy to help in #4595 merge (and further improvement). Please let us know. Thanks.

@ojw28
Copy link
Contributor

ojw28 commented Nov 30, 2020

We don't have capacity to work on this any time soon. We would welcome high quality external contributions. In terms of what that means, I think we'd be looking for:

  1. Small incremental changes that are relatively easy to reason about, and are amenable to careful code review
  2. Test cases and/or content that (a) allow manual validation of the changes being made, and (b) can be used by tests

sneelavara added a commit to TiVo/ExoPlayer that referenced this issue Dec 16, 2020
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.
sneelavara added a commit to TiVo/ExoPlayer that referenced this issue Dec 17, 2020
Fixed the verticalAnchorType and horizontalAnchorType calculation

The anchorID 0, 1 and 2 should correspond to verticalAnchorType=ANCHOR_TYPE_START, anchorID 3, 4, 5 is ANCHOR_TYPE_MIDDLE, anchorID 6, 7 and 8 is ANCHOR_TYPE_END

The anchorID 0, 3 and 6 should correspond to horizzonatlAnchor=ANCHOR_TYPE_START, anchorID 1, 4, 7 is ANCHOR_TYPE_MIDDLE, anchorID 2, 5 and 8 is ANCHOR_TYPE_END
@sneelavara
Copy link
Contributor

@ojw28 I have sent the test stream links over developer email. Please have a look at them.
We are exploring the way to add unit test cases. If you have an inputs and/or pointers to look at, would be very helpful. Thanks.

@icbaker icbaker assigned icbaker and unassigned ojw28 Dec 17, 2020
psharma676 pushed a commit to psharma676/ExoPlayer that referenced this issue Dec 14, 2023
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.

This is a cherry-pick of one commit from a pending pull request:
  google#8356
And the commit was:
  google@93829f9

The other two commits in the pull request are already included, cherry-picked from Google's 1.15.1

The conflicts resolved are:
  * use previousSequenceNumber over lastSequenceNumber
  * take Google's spacing choices
  * Google's code for the anchorId to Cue.ANCHOR_TYPE to match 2.15.1
psharma676 pushed a commit to psharma676/ExoPlayer that referenced this issue Dec 14, 2023
Fix requires changes to the merged pull request google#8415
that are still part of the unmerged pull request google#8356

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.

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# Author:    sneelavara <[email protected]>
# Date:      Wed Dec 16 13:03:37 2020 -0800
#
# On branch t-fix-cea708Merge
# Your branch and 'tivo-pvt/t-fix-cea708Merge' have diverged,
# and have 1 and 2 different commits each, respectively.
#   (use "git pull" to merge the remote branch into yours)
#
# Changes to be committed:
#	modified:   library/core/src/main/java/com/google/android/exoplayer2/text/cea/Cea708Decoder.java
#
# Untracked files:
#	CCURStream_3000kbps720p29_97fps-1_1661286386_init.cmfv?ccur_keyrot_t=1666096234
#	CCURStream_3000kbps720p29_97fps-1_T1666098015585589~D6006000.cmfv
#	MERGE_COMMIT_MESSAGE
#	VelocixVTP.mp4
#	X
#	test_url
#	tivo-docs/DASH Encoding Guidelines.md
#	tivo-docs/Encoding HLS Renditions.pdf
#	tivo-docs/HANDLING_2.15.1_MERGE_CONFLICTS.pdf
#	tivo-docs/Logging Updates For Audio Position.md
#	tivo-docs/Logging Updates For Audio Position.pdf
#	tivo-docs/TiVoExoPlayerReleaseMapping.md
#	tivo-docs/TiVoExoPlayerReleaseMapping.pdf
#	tivo-docs/Understanding ExoPlayer Position Tracking.md
#	tivo-docs/Understanding ExoPlayer Position Tracking.pdf
#	widevine.cmfv
#
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants