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

Requires extradata in Matroska-style #2

Open
sdroege opened this issue Feb 25, 2018 · 5 comments
Open

Requires extradata in Matroska-style #2

sdroege opened this issue Feb 25, 2018 · 5 comments

Comments

@sdroege
Copy link
Contributor

sdroege commented Feb 25, 2018

Currently it is required to pass extradata in Matroska-style to the decoder.

It might make sense to allow directly passing the 2/3 Vorbis headers independently, and extending extradata to be a vector of headers instead, and let the Matroska demuxer handle the Matroska-specifics and conversions to any generic format. Otherwise e.g. the Ogg demuxer will have to convert to the Matroska-style extradata, which seems kind of wrong

@est31
Copy link
Contributor

est31 commented Feb 25, 2018

yes, right now it is specific to matroska. This issue is partially blocking ogg demuxer support. So 👍 your suggestion would be a lot cleaner.

@lu-zero
Copy link
Member

lu-zero commented Feb 25, 2018

This part is relatively hairy since supporting multiple formatting without proper signalling is the best way to unwittingly produce non-standard files.

At Context API level would be great to standardize to a single format for extradata and packets, no matter how many variants we might have. That would require to pay a double-conversion in some situations, but would avoid some burden to the API user that does not want to care about this detail.

At Demuxer/Decoder level we could extend the API to support a vector of vectors to have some extra flexibility and introduce earlier than I wished the concept of bitstream formatter and the fact we need to carry the bitstream format information somehow.

If you like the approach probably we could discuss the details in an issue on the main repo.

@sdroege
Copy link
Contributor Author

sdroege commented Feb 26, 2018

Can't you just define that the extradata is always the "canonical" format of the codec (i.e. the 3 Vorbis headers separately), and every muxer/demuxer has to do whatever is necessary for converting that to the format it needs? Nothing other than the Matroska muxer/demuxer would have to convert it to the Matroska format, so generic code in some library for that seems unnecessary.

That's basically how we do it in GStreamer and that worked fine so far, without the need for complicated signalling.

But you mentioned bitstream formatters. This would be more for complicated cases like h264/5 with their various bitstream formats (byte-stream with the start codecs and SPS/PPS inline, avcC and properly packetized, etc)?

@lu-zero
Copy link
Member

lu-zero commented Feb 26, 2018

Having a canonical representation for the higher-level API is something I'd like to have, but each and every demuxer has its own ideas on what is canonical (e.g. the vorbis fundamental headers are actually 2, the last one is generic metadata ogg-way) and sometimes the reference implementation has a muxing-bias, but then the larger world might use a completely different muxing-format with different rules.

A bitstream formatter is mainly useful for h264/hevc/h26x and might be needed for vp9.

We can do without the signaling by forcing a choice (e.g. flatten vorbis extradata vs two buffers extradata vs ogg head verbatim) and in that case I would try to keep the ogg-specific choices away from the user given that mp4 and mkv might have the same style and eventually be more widespread.

@est31
Copy link
Contributor

est31 commented Feb 26, 2018

the vorbis fundamental headers are actually 2, the last one is generic metadata ogg-way

Don't want to be nitpicky but the generic metadata header is the second one. Quoting the spec: "The header packets are, in order, the identification header, the comments header, and the setup header. All are required for decode compliance."

Ogg is quite weird. It can for example support chaining, aka resending the headers mid-stream to allow for reconfigurations. See my lewton issue on it, linking to issues in other projects: RustAudio/lewton#18

I think I don't really have an opinion on how to fix this after all... it is a hard issue, gonna try to help with any fix you come up with :).

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

3 participants