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 SEI with resolution payloadType 5 based on H264 specification (Us… #2126

Merged
merged 1 commit into from
May 7, 2019
Merged

Conversation

bruce1i
Copy link
Contributor

@bruce1i bruce1i commented Feb 12, 2019

This PR will parse SEI with resolution payloadType 5 based on H264 specification. Now you can write custom data to H264 and get it from 'Hls.Events.FRAG_PARSING_USERDATA' event.

Why is this Pull Request needed?

A: User can get custom data from SEI. It's especially useful in live streaming.

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

A: No.

Resolves issues:

A: Just add a SEI parsing rule.

Checklist

  • changes have been done against master branch, and PR does not conflict

@michaelcunningham19
Copy link
Member

michaelcunningham19 commented Feb 12, 2019

Hey @bruce1i !
Thanks for the contribution, this looks like it could be very useful!

Would you be able to provide either a test stream containing custom data, or some code snippets of how you're inserting data into a stream?

As an aside, I'd like to hear from other collaborators on their thoughts of incorporating additions like this to the core library. Thinking about bundle size and modularity, it may be beneficial to explore emitting helpers alongside with the main code bundle

@bruce1i
Copy link
Contributor Author

bruce1i commented Feb 13, 2019

Hey @michaelcunningham19 !
Thanks for the reply.

1. Test stream containing custom data.
A: "https://devstreaming-cdn.apple.com/videos/tutorials/App_Store_Connect_Basics/hls_vod_mvp.m3u8" is I found from the apple website and it contains custom data. The screenshot is below.
sei

2. Some code snippets of how you're inserting data into a stream.
A: I can't. It's difficult for me to insert custom into a stream. The back-end colleague does this in the server according to the H264 specification. I just parse this according to the rule from H264 specification. It's work well for me and also works well for all that insert sei according to the rule of H264 specification.

My purpose

1. Why I commit this PR.
A: The first one is my encounter and the user data unregistered SEI message is a high frequency of use.
I have a requirement to get SEI from a live stream, then I found hls.js that has 'Hls.Events.FRAG_PARSING_USERDATA' event which can parse SEI. I'm so glad, but happiness is short. Because the event cannot be triggered. I googled for what I miss, I can't find the answer. Then I saw the source code, I found the TODO comment, so I fork the code and add the parse rule. I thought it's useful for other people who have the same requirement.

2. The bundle size.
A: SEI has a lot of rules. If we add it all, the size will become bigger and bigger. I don't think we will add all rules to the core library. But some high-frequency rule maybe can add it. Or is there a place that people can find how to get SEI and people can reuse the code.

@bruce1i
Copy link
Contributor Author

bruce1i commented Mar 13, 2019

Hi @michaelcunningham19 @johnBartos , Is there any new progress or conclusion?

@iamzhouyi
Copy link

Any further progress for this PR? I encountered same issue with hls.js as well.
Be happy to see it's merged

Copy link
Collaborator

@johnBartos johnBartos left a comment

Choose a reason for hiding this comment

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

@bruce1i @iamzhouyi Sorry for the delay - LGTM

@johnBartos johnBartos added this to the 0.13.0 milestone May 7, 2019
@johnBartos johnBartos added this to In progress in 0.13.0 via automation May 7, 2019
@johnBartos johnBartos merged commit f4792b4 into video-dev:master May 7, 2019
0.13.0 automation moved this from In progress to Done May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
0.13.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants