mozilla / mp4parse-rust

Parser for ISO Base Media Format aka video/mp4 written in Rust.
Mozilla Public License 2.0
404 stars 62 forks source link

Add some support for AVIF sequences. #378

Closed Zaggy1024 closed 1 year ago

Zaggy1024 commented 2 years ago

All properties not related to the pitm box have been moved from mp4parse_avif_get_image to mp4parse_avif_get_info. New fields for the color and alpha tracks are included in Mp4parseAvifInfo. To get the sample indices for those tracks, mp4parse_avif_get_indice_table was added as well.

These changes break C API backwards compatibility due to removing fields from Mp4parseAvifImage, so the version is bumped to 0.16.0.

I've implemented AVIF sequence decoding into Firefox using these changes, so they have been tested a bit at least. However, it's worth noting that I don't have access to the MIAF spec due to the price to buy it, so if anyone is able to confirm that anything related to that spec is sane that would be great.

I've also added the pict and auxv track handlers, which could potentially be fleshed out to verify validity, but I don't think that doing so would provide a tangible benefit to end users since it seems like libavif just uses the logic I've implemented for finding color and alpha tracks here.

I would imagine some new tests for these changes would be in order, but I'm unfortunately not familiar with cargo yet, so if those are desired, any pointers would to documentation would be helpful.

ChunMinChang commented 1 year ago

It seems these will be imported to gecko in BMO 1788119

Zaggy1024 commented 1 year ago

Bumping this since I've updated the tests that are relevant to AVIS support to pass with these changes.

Let me know if there are any new tests I should add as well, I'd be happy to add more coverage.

Zaggy1024 commented 1 year ago

Thanks for the review! I've fixed the issues you mentioned, and squashed the "Moved pitm" commit into the first commit since it was mainly a bit of a fixup of that commit.

Should be good to go if you don't see any more issues!

Zaggy1024 commented 1 year ago

Looks like I missed a couple formatting issues. I fixed those, ran cargo fmt locally and force-pushed again, so hopefully it should be all good now.

Zaggy1024 commented 1 year ago

Sorry for the multiple CI attempts, this is my first time working with cargo so I wasn't aware of clippy. Hopefully I've fixed it so that it passes linting now.

kinetiknz commented 1 year ago

No problem, thanks for the quick follow-ups. Don't worry about the nightly-only clippy failures, I'll sort them out separately.