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

Add CMAF closed captions support #3016

Closed

Conversation

victor-at-work
Copy link

This PR will...

Add closed captions support for CMAF streams. This is a modification of @gkolb's work in #2623 for master.

Why is this Pull Request needed?

Closed captions for CMAF are not currently supported.

Are there any points in the code the reviewer needs to double check?

Resolves issues:

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

@victor-at-work victor-at-work mentioned this pull request Sep 3, 2020
3 tasks
@robwalch
Copy link
Collaborator

robwalch commented Sep 3, 2020

Hi @victor-at-work,

Could you provide a sample stream for testing these changes? Thank you.

@victor-at-work
Copy link
Author

@robwalch I'll talk to our team and get back to you on it.

@victor-at-work
Copy link
Author

@robwalch To clarify, do we want the test stream hosted somewhere? Or just the files for it are okay?

@robwalch
Copy link
Collaborator

robwalch commented Sep 15, 2020

@victor-at-work hosted please.

If the stream is ok to host with the hls.js demo page, I can upload and maintain it as part of our test suite, as long as the content is fair use and sfw. Thanks!

@victor-at-work
Copy link
Author

@robwalch Our backend folks found this public test stream https://devstreaming-cdn.apple.com/videos/streaming/examples/img_bipbop_adv_example_fmp4/master.m3u8. Please let me know if this is okay.

@stale
Copy link

stale bot commented Oct 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the Stale label Oct 11, 2020
@victor-at-work
Copy link
Author

Hi @robwalch, by no means trying to rush you but just checking in to see if there are any updates here. Thank you!

@stale stale bot removed the Stale label Oct 12, 2020
@robwalch
Copy link
Collaborator

@victor-at-work The test stream you provided contains a WebVTT subtitles track. As far as I can tell, Safari is displaying the VTT track, not CMAF-608

https://devstreaming-cdn.apple.com/videos/streaming/examples/img_bipbop_adv_example_fmp4/master.m3u8

As an example, if we load only one of the variants in Safari, no captions are displayed:
https://devstreaming-cdn.apple.com/videos/streaming/examples/img_bipbop_adv_example_fmp4/v5/prog_index.m3u8

@robwalch
Copy link
Collaborator

Hi @robwalch, by no means trying to rush you but just checking in to see if there are any updates here. Thank you!

I'm not inclined to include this functionality in a minor update since it doesn't appear present in Safari. If I'm mistaken and you can demonstrate this in a sample stream that does not also contain a VTT subtitles track, I'll prioritize these changes over others.

@victor-at-work
Copy link
Author

Hi @robwalch, I think Safari displays the VTT track because it prefers that to the CEA track? When I manually disabled the former and enabled the latter CEA captions were then rendered, which shows that Safari does indeed support CMAF CEA. I'm not sure what's up with the variant playlists as neither of the tracks showed up when I played any of them.

Let me know if you still want a sample stream without VTT. I can try and ask our backend team for one.

@robwalch
Copy link
Collaborator

Hi @victor-at-work,

When I manually disabled the former and enabled the latter CEA captions were then rendered, which shows that Safari does indeed support CMAF CEA.

OK.

Let me know if you still want a sample stream without VTT. I can try and ask our backend team for one.

Thanks. I would appreciate that. It would help avoid any confusion.

If you could also clarify what you expect to happen when a stream has only CEA caption as well as when a stream has both CEA CCs and VTT subs that would help; Should we display both tracks or are the supposed to be deduplicated based on some property of the MEDIA tags?

@victor-at-work
Copy link
Author

@robwalch This should have just a CEA track https://video.twimg.com/ext_tw_video/1319433806244319232/pu/pl/cBRNj-zqFNAUPQm-.m3u8?tag=10.

The existing behavior is that the VTT is shown and CEA hidden if both tracks are present. This seems reasonable to me and aligns with Safari's behavior as well.

@robwalch robwalch added this to the 0.15.0 milestone Dec 23, 2020
@robwalch robwalch changed the base branch from master to patch/v0.15.x December 23, 2020 18:06
@robwalch robwalch closed this Dec 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants