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

Support CEA-608 closed-captions in H262/MPEG-TS. #3114

Closed
wants to merge 2 commits into from

Conversation

goffioul
Copy link
Contributor

Issue: #2565

@AquilesCanta
Copy link
Contributor

Looks nice! Thanks. Will get to this soon. Possibly next week or the one after that.

@SVPA-MaxWu
Copy link

any plan to support 708 as well?

@goffioul
Copy link
Contributor Author

AFAIK 708 is supported, although I never tested it myself. However it's not used by default and/or it depends on whether your stream has the proper data in the stream descriptor (in case of MPEG/TS). To have 708 decoder enabled, you have to make sure that the SeiReader object is created with the appropriate mimetype: if your stream does not have appropriate descriptors and you don't provide the list of mimetypes explicitly when creating SeiReader, then it'll default to 608 only.

In all cases, this is independent from how the data is extracted from the stream, which was the point of this pull request. 608 and 708 embeds data similarly in MPEG2 and H264 streams. It's how the data is interpreted afterwards that makes a difference.

@ojw28
Copy link
Contributor

ojw28 commented Sep 4, 2017

@AquilesCanta - Planning to take a look at this?

@@ -41,7 +65,7 @@
* @param seiBuffer The unescaped SEI NAL unit data, excluding the NAL unit start code and type.
* @param outputs The outputs to which any samples should be written.
*/
public static void consume(long presentationTimeUs, ParsableByteArray seiBuffer,
private static void consumeSei(long presentationTimeUs, ParsableByteArray seiBuffer,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we leave the javadocs for consume?

consumeUserData(presentationTimeUs, buffer, outputs);
break;
case MODE_H264:
consumeSei(presentationTimeUs, buffer, outputs);
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering the modes are actually only 2 and they are always known at compile time, why not just have a consumeUserData and consumeSei made public? If so, public methods should be documented.

@okulagin
Copy link

@goffioul any thought when comments left by @AquilesCanta will be addressed?

@goffioul
Copy link
Contributor Author

@okulagin I don't really have the time at the moment. It should be trivial to address @AquilesCanta 's comments, though. Feel free to do it, if you wish.

@ojw28
Copy link
Contributor

ojw28 commented Jul 10, 2018

It looks likely support for this will be added by #4308.

@ojw28 ojw28 closed this Jul 12, 2018
@google google locked and limited conversation to collaborators Nov 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants