common: rename utils-xiph crate to common

[sscobici]

Adding basic video detection for MKV files, there are more fixes needed for some files, but want to keep those in separate PRs. Please note that I copied the code for AVCDecoderConfigurationRecord and HEVCDecoderConfigurationRecord parsing from the isomp4 to avoid adding additional dependencies to the format crate.

[pdeljanov]

Ah, I hate do this to you, but I actually also have a WIP in commit for MKV that's basically this (including the HEVCDecoderConfigurationRecord) plus a bit more. I thought you were only interested in working on MP4 so I didn't push it. This is my last WIP commit though.. 😆

I'm not sure if a rebase is worthwhile in this case. I can push it later today if that's okay with you.

Thing is, if you are interested in integrating in both MP4 and MKV, we may need to think about code deduplication. While these record parsers are simple right now, I feel like the entire record should be validated which will increase the complexity. Likewise, any NAL parsing that both the demuxer and (possible) future decoders do should be shared. My thought is that we will need a symphonia-utils-mpeg crate just like we have a symphonia-utils-xiph crate.

[sscobici]

:-) no problem at all. I still learn the code. I observed that reading mkv and mp4 is different. For example reading a u16 in mkv crate looks like: pixel_width = Some(it.read_u64()?); which is a bit confusing, even if inside it reads only 2 bytes till the end of element without buffer overrun.

Hope you don't have plans for .ts files. I have some of those too. Movie files need some love for multichannel audio support, demuxers currently cannot identify them.

I will wait for your commit to fix some exceptional cases from my mkv files.

[pdeljanov]

Thanks! I pushed my MKV changes.

For example reading a u16 in mkv crate looks like: pixel_width = Some(it.read_u64()?); which is a bit confusing, even if inside it reads only 2 bytes till the end of element without buffer overrun.

Yeah, this looks a bit weird. It's because MKV integers are variable width, but the maximum width is 64-bit. So the name reflects the largest possible integer it can return.

Hope you don't have plans for .ts files. I have some of those too.

Can't say that one crossed my mind, but I do have a few laying around. In terms of additional formats, I've been particularly interested in seeing AVI and FLV added.

Movie files need some love for multichannel audio support, demuxers currently cannot identify them.

Hmm, that's odd, but perhaps not surprising. Movies usually use DTS/DCA and AC3 codecs which we don't support. I'm also particularly interested in seeing support for those as well.

[sscobici]

My thought is that we will need a symphonia-utils-mpeg crate

It might also contain VP8/VP9 codec configuration box parsers or other non mpeg parsers that might be reused across formats. What do you think about:

symphonia-utils-format
  video
    mod.rs - AVC, HEVC codec configuration code, once it is too long it can be split in files.
  audio (in future)

[pdeljanov]

Hmm, yeah, so there are really two ways this could go:

1. One crate per family of codecs (e.g., MPEG, Xiph, VPX, etc.).
2. One crate for all codec families where each codec can be enabled with feature flags much like the symphonia root crate. In this crate I'd expect each family to be a module, with a sub-module per codec, for organization.

If we go the latter, I'd like to see symphonia-utils-xiph merged into it. Probably could just rename that crate to keep the Git history intact. However, I think we'd want to consider a much bigger name change. I'm not a huge fan of the symphonia-utils-* naming because these crates are mean't to be private to Symphonia and not used by anyone else. I think the current naming may invite other people to use it.

If we go with option 2, then here are some names I think are decent alternatives, but open to others:

• symphonia-internal-common
• symphonia-private-common

If we go option 1, then the names can be adapted per family:

• symphonia-internal-mpeg, etc.
• symphonia-private-vpx, etc.

What do you think?

[sscobici]

I prefer the second option—one crate with multiple modules.

These crates are intended to be private to Symphonia and not used by anyone else.

However, someone might find them useful if they want to implement their own format.

"common" seems too generic as a name. While this might be fine at this stage, when it comes to code reuse, there could be multiple levels:

• One for different formats (e.g., codec configuration structures), which would likely be reused by codec implementations.
• Another for different codec implementations (e.g., software/hardware versions or other common complex code).

The initial intention was to have shared code just for formats, possibly named something like symphonia-format-common. Please let me know if a different name is preferable.

I'd expect each family to be a module, with sub-modules per codec for organization.

Since this library was originally developed for audio, I imagine people will want the option to remove non-audio code during compilation. At the same time, things like Dolby Vision box parsing or HDR detection are common across different codecs like AV2, VP9, HEVC, and VVC but maybe too small for a dedicated module. Please suggest module names. For video, I'd include all related codecs in a single module for now to avoid creating too many small modules, not sure if you want the same for audio.

[pdeljanov]

I prefer the second option—one crate with multiple modules.

These crates are intended to be private to Symphonia and not used by anyone else.

However, someone might find them useful if they want to implement their own format.

Yeah, someone might. Though, in this case, we will need to be mindful of documenting the crate and to keep that use-case in mind while developing it. The current Xiph utils crate is not great in this regard.

"common" seems too generic as a name. While this might be fine at this stage, when it comes to code reuse, there could be multiple levels:

• One for different formats (e.g., codec configuration structures), which would likely be reused by codec implementations.
• Another for different codec implementations (e.g., software/hardware versions or other common complex code).

My thought was that items in the former would be in sub-modules of symphonia-common, while items in the latter could be in codec-specific common crates: symphonia-common-<CODEC>.

But I will agree that "common" is indeed very generic. However, I'm struggling to come up with a good name.

The initial intention was to have shared code just for formats, possibly named something like symphonia-format-common. Please let me know if a different name is preferable.

I don't think a common crate for formats would solve the problem because some stuff will be used by both demuxers and decoders. For example, symphonia-utils-xiph is pulled in by: symphonia-bundle-flac, symphonia-codec-vorbis, symphonia-format-ogg, symphonia-format-isomp4, and symphonia-format-mkv. It would be weird for a codec crate to pull in a format dependency. Therefore, I tried to avoid putting format or codec in the name.

Initially, this was actually why I suggested individual crates per family. It side stepped the problem:

• symphonia-common-mpeg
• symphonia-common-vpx
• symphonia-common-xiph

I'm still partial towards this solution. Unless you see anything significantly wrong with it, we could probably just go with it. We can loop back around before 0.6 is released to determine if we should merge them into one crate or not.

I'd expect each family to be a module, with sub-modules per codec for organization.

Since this library was originally developed for audio, I imagine people will want the option to remove non-audio code during compilation. At the same time, things like Dolby Vision box parsing or HDR detection are common across different codecs like AV2, VP9, HEVC, and VVC but maybe too small for a dedicated module. Please suggest module names. For video, I'd include all related codecs in a single module for now to avoid creating too many small modules, not sure if you want the same for audio.

I was just thinking of a simple hierarchy like this:

• family name (mpeg, vpx, xiph, etc.)
  • audio
    • codec name in family (e.g., flac, opus, vorbis, etc.) for codec specific
  • video

I don't know enough about the video codecs to comment about any deeper levels for those. However, for audio, the above is sufficient. If multiple crates are used, then the family will be namespaced by the crate so it can be omitted from the hierarchy.

[sscobici]

Ok. I just renamed of utils-xiph to common, no cfg options.

I would like to have small PRs for each independent change if you don't mind. Let me know if you prefer bigger PR with several commits.

[pdeljanov]

@sscobici, LGTM, merged!

Thanks for taking care of this. I made a note on my todo to loop back around to this later and add the feature flags. We can also then see if we're still happy with the naming.

For future PRs, I would prefer bigger PRs with a clean commit history that I can apply directly to the dev-0.6 branch. I won't be able to keep pace with smaller PRs, but I can do 1 or maybe 2 in-depth reviews + follow-up a week.