1788119 - (avis) Initial implementation of AVIF image sequence/animation (AVIS) decoding.

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, enhancement, P3)

Description

[Zaggy1024]

Creating a separate bug from Bug 1686338 for a basic (and possibly non-standard) implementation of AVIS decoding.

Comment 1

[Zaggy1024]

Attachment: Bug 1788119 - Part 1 - Keep parser and decoder instances persistent in nsAVIFDecoder. r=tnikkel

Comment 2

[Zaggy1024]

Attachment: Bug 1788119 - Part 2 - Rename dom/media/mp4/Index.h's Index class to MP4SampleIndex to prevent name conflicts. r=alwu

Depends on D156650

Comment 3

[Zaggy1024]

Attachment: Bug 1788119 - Part 3 - Add initial support for animated AVIF sequences. r=tnikkel

Depends on D156651

Comment 4

[Zaggy1024]

Attachment: Bug 1788119 - Part 4 - Update mp4parse-rust for AVIS support. r=kinetik

Depends on D156652

Comment 5

[Tyson Smith [:tsmith]]

Would it possible to also update the fuzzer to support this format? https://searchfox.org/mozilla-central/rev/85cbcecd24554c1fa88360412452ad0f7ed48630/image/test/fuzzing/TestDecoders.cpp#173

Comment 6

[Zaggy1024]

(In reply to Tyson Smith [:tsmith] from comment #5)

Would it possible to also update the fuzzer to support this format? https://searchfox.org/mozilla-central/rev/85cbcecd24554c1fa88360412452ad0f7ed48630/image/test/fuzzing/TestDecoders.cpp#173

It's the same file format, just with different features, so I think judging by what I'm seeing there, it would just be a matter of flipping the "image.avif.sequence.enabled" pref to "true" like it already does with "image.avif.enabled". I don't know where the fuzzing corpora come from, though, and the effectiveness of the fuzzing without animated avif corpora might not be good.

To be honest, though, I don't know much about fuzzing yet, especially in the context of Firefox, so if there's any documentation I can reference that would help.

Comment 7

[Tyson Smith [:tsmith]]

(In reply to Zaggy1024 from comment #6)

It's the same file format, just with different features, so I think judging by what I'm seeing there, it would just be a matter of flipping the "image.avif.sequence.enabled" pref to "true" like it already does with "image.avif.enabled".

That's great, if that is the case :)

I don't know where the fuzzing corpora come from, though, and the effectiveness of the fuzzing without animated avif corpora might not be good.

The fuzzing team maintains a private corpus for this fuzzer. I can update it once this lands. If you have a good source of test case please point me to them.

To be honest, though, I don't know much about fuzzing yet, especially in the context of Firefox, so if there's any documentation I can reference that would help.

Docs can be found here and more specifically for this fuzzer here.

Comment 8

[Zaggy1024]

(In reply to Tyson Smith [:tsmith] from comment #7)

(In reply to Zaggy1024 from comment #6)

It's the same file format, just with different features, so I think judging by what I'm seeing there, it would just be a matter of flipping the "image.avif.sequence.enabled" pref to "true" like it already does with "image.avif.enabled".

That's great, if that is the case :)

Since corpus is elsewhere, that should be all that's needed. Shall I throw that into a commit on this bug?

The fuzzing team maintains a private corpus for this fuzzer. I can update it once this lands. If you have a good source of test case please point me to them.

There's a few animated files used for mp4parse tests in the AVIF spec repository. There are a couple good examples in the Netflix/avis folder, and there may be some others hiding in other folders there.

Some more can be found here, since the Link-U folder in the avif spec tree is out of date. The animated ones have the extension .avifs.

Other than that, the only usage that I can remember finding in the wild is 7tv emotes for Twitch. On an emote page, there's an option to switch to AVIF, and many of the emotes are animated. Those should be using more up-to-date versions of AVIF encoders, so they're more likely to follow spec than the older test files and may be worth using as samples, up to you. :)

Comment 9

[Zaggy1024]

Attachment: Bug 1788119 - Part 5 - Add tests for animated AVIF files. r=tnikkel

Comment 10

[Timothy Nikkel (:tnikkel)]

I pushed the patches to try

https://treeherder.mozilla.org/jobs?repo=try&revision=fd298ebce057c8e1cf72ed524cd192daaba56bed

Looks like there is a leak of the decoder and a failure in browser/components/firefoxview/tests/browser/browser_feature_callout_position.js

Comment 11

[Zaggy1024]

(In reply to Timothy Nikkel (:tnikkel) from comment #10)

I pushed the patches to try

https://treeherder.mozilla.org/jobs?repo=try&revision=fd298ebce057c8e1cf72ed524cd192daaba56bed

Looks like there is a leak of the decoder and a failure in browser/components/firefoxview/tests/browser/browser_feature_callout_position.js

Leak should be resolved now, but I'm not familiar with that test I'll have to look into it more. The log output doesn't really make sense to me without looking closer at it :(

Comment 12

[Timothy Nikkel (:tnikkel)]

(In reply to Zaggy1024 from comment #11)

(In reply to Timothy Nikkel (:tnikkel) from comment #10)

I pushed the patches to try

https://treeherder.mozilla.org/jobs?repo=try&revision=fd298ebce057c8e1cf72ed524cd192daaba56bed

Looks like there is a leak of the decoder and a failure in browser/components/firefoxview/tests/browser/browser_feature_callout_position.js

Leak should be resolved now, but I'm not familiar with that test I'll have to look into it more. The log output doesn't really make sense to me without looking closer at it :(

Thanks for fixing the leak.

Perhaps that other failure is not caused by your patches. I will push the new patches to try tomorrow to check and we'll see if we need to investigate it afterall.

Comment 13

[Zaggy1024]

(In reply to Timothy Nikkel (:tnikkel) from comment #12)

Perhaps that other failure is not caused by your patches. I will push the new patches to try tomorrow to check and we'll see if we need to investigate it afterall.

I'm running another one already: https://treeherder.mozilla.org/jobs?repo=try&revision=0fc38f65a7013afff80cf3aca6df96bbde83a537&selectedTaskRun=cdONI1j3SWuxNu7rrgSyyg.0

Not sure if the browser chrome test group numbers match up, but it seems like at least some have passed where they were failing repeatedly on your link.

(Looks like I have a GTest failure to look into tomorrow, though..)

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8dce9350abb8
Part 1 - Keep parser and decoder instances persistent in nsAVIFDecoder. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/eeb1461933aa
Part 2 - Rename dom/media/mp4/Index.h's Index class to MP4SampleIndex to prevent name conflicts. r=media-playback-reviewers,alwu
https://hg.mozilla.org/integration/autoland/rev/4ba88410cdf6
Part 3 - Add initial support for animated AVIF sequences. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/00d82acfaa2f
Part 4 - Update mp4parse-rust for AVIS support. r=kinetik,glandium,supply-chain-reviewers
https://hg.mozilla.org/integration/autoland/rev/56abc66f7e84
Part 5 - Add tests for animated AVIF files. r=tnikkel

Comment 15

[Norisz Fay [:noriszfay]]

Backed out for causing build bustages on DecoderData.cpp

Backout link

Push with failures

Failure log

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f8eacf96c71
Part 1 - Keep parser and decoder instances persistent in nsAVIFDecoder. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/3eaac75dde3c
Part 2 - Rename dom/media/mp4/Index.h's Index class to MP4SampleIndex to prevent name conflicts. r=media-playback-reviewers,alwu
https://hg.mozilla.org/integration/autoland/rev/7d5cc211e3f5
Part 3 - Add initial support for animated AVIF sequences. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/9493029c76b9
Part 4 - Update mp4parse-rust for AVIS support. r=kinetik,glandium,supply-chain-reviewers
https://hg.mozilla.org/integration/autoland/rev/430e71478ff5
Part 5 - Add tests for animated AVIF files. r=tnikkel

Comment 17

[Atila Butkovits]

Backed out for causing bustages complaining about AVIFDecodedData.

Backout link: https://hg.mozilla.org/integration/autoland/rev/da557d1806dd9e1e519e00a4481b2e075f722ae8

Push wheres failures started: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=linux%2Cx64%2Cdebug%2Cbuild-linux64-base-toolchains%2Fdebug%2Cbb&revision=72da2fb0aab1c5cd50aee0b7759eb4678a2d84b8&selectedTaskRun=L-kgAIqESYCOd0U3jou2ig.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=403811131&repo=autoland&lineNumber=19569

Comment 18

[Zaggy1024]

(In reply to Atila Butkovits from comment #17)

Backed out for causing bustages complaining about AVIFDecodedData.
Push wheres failures started: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=linux%2Cx64%2Cdebug%2Cbuild-linux64-base-toolchains%2Fdebug%2Cbb&revision=72da2fb0aab1c5cd50aee0b7759eb4678a2d84b8&selectedTaskRun=L-kgAIqESYCOd0U3jou2ig.0

Odd that this error only happens with one build type, but I'll move the classes over to the header to get around this.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/051e536a5a65
Part 1 - Keep parser and decoder instances persistent in nsAVIFDecoder. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/b18d676231de
Part 2 - Rename dom/media/mp4/Index.h's Index class to MP4SampleIndex to prevent name conflicts. r=media-playback-reviewers,alwu
https://hg.mozilla.org/integration/autoland/rev/be008ddb2418
Part 3 - Add initial support for animated AVIF sequences. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/a651d4323066
Part 4 - Update mp4parse-rust for AVIS support. r=kinetik,glandium,supply-chain-reviewers
https://hg.mozilla.org/integration/autoland/rev/3c8cacc1d81b
Part 5 - Add tests for animated AVIF files. r=tnikkel

Comment 20

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/051e536a5a65
https://hg.mozilla.org/mozilla-central/rev/b18d676231de
https://hg.mozilla.org/mozilla-central/rev/be008ddb2418
https://hg.mozilla.org/mozilla-central/rev/a651d4323066
https://hg.mozilla.org/mozilla-central/rev/3c8cacc1d81b

Comment 21

[Mike Hommey [:glandium]]

For some reason, if I run mach vendor rust, I get carriage return differences in mp4parse/link-u-avif-sample-images. I suppose you did this on windows?

Comment 22

[Zaggy1024]

(In reply to Mike Hommey [:glandium] from comment #21)

For some reason, if I run mach vendor rust, I get carriage return differences in mp4parse/link-u-avif-sample-images. I suppose you did this on windows?

I did, yeah. Is there anything specific I should look for on my system that would normally take care of line endings?

Comment 23

[Mike Hommey [:glandium]]

(In reply to Zaggy1024 from comment #22)

(In reply to Mike Hommey [:glandium] from comment #21)

For some reason, if I run mach vendor rust, I get carriage return differences in mp4parse/link-u-avif-sample-images. I suppose you did this on windows?

I did, yeah. Is there anything specific I should look for on my system that would normally take care of line endings?

The git config related to crlf would be my first guess. (core.eol and core.autocrlf)