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

cc_inserter lacks the proper addition of the ATSC1data marker_bits #115

Closed
enen92 opened this issue Jan 7, 2023 · 3 comments
Closed

cc_inserter lacks the proper addition of the ATSC1data marker_bits #115

enen92 opened this issue Jan 7, 2023 · 3 comments

Comments

@enen92
Copy link

enen92 commented Jan 7, 2023

Dear all,

In Guidelines for Implementation: DASH-IF Interoperability Points v3.1 the following is stated regarding CEA-608/708 in SEI messages:

  • user_structure() – This is a variable length data structure ATSC1_data() defined in section 8.2 of ANSI/SCTE 128 2013-a.

The description is shown below:

image

Unfortunately since 23cb15c#diff-3372455acaff98fed2743e2f609a073a02d36e8edc38bc43adbebc56a03dfef5R83 the cc_data is immediately followed by the rbsp_trailing_bits (the marker_bits (0xFF) might have been unintentionally removed).
Considering the example of the last cc_data packet in one of the SEI elements:

image

Correct sequence should be 0xFC 0x94 0x2F 0xFF 0x80 and not 0xFC 0x94 0x2F 0x80

I've found this while investigating why FFMpeg couldn't extract the CC caption data of the samples provided by the test server on AWS (namely https://livesim.dashif.org/dash/vod/testpic_2s/cea608.mpd). This is not the sole reason for the lack of interoperability but at least seems to be the only issue out of the scope of FFmpeg.

It'd be great if this could be fixed and the public (affected) samples regenerated.

cc @tobbee

@tobbee
Copy link
Contributor

tobbee commented Jan 10, 2023

Thanks for the report and for investigating the issues. It seems indeed that I added the rbsp_trailing bits, but dropped the marker_bits at the same time. I agree that the SEI NAL units still come after the video NALUs. Will try to fix that.

@tobbee
Copy link
Contributor

tobbee commented Jan 12, 2023

This issue should now have been properly fixed and the assets updated.
The updated content has one line in italics, so it can be distinguished from the earlier version:

https://livesim.dashif.org/livesim/testpic_2s/cea608.mpd
https://livesim.dashif.org/dash/vod/testpic_2s/cea608.mpd

The relevant PRs are:

Please report back if everything looks OK or not?

@enen92
Copy link
Author

enen92 commented Jan 13, 2023

Thanks a lot @tobbee I can confirm it works now just fine in mpv/ffmpeg

@tobbee tobbee closed this as completed Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants