Bug 207750 – AVIF decoding support

• Component: Images
• Status: RESOLVED FIXED

[Philippe Normand]

Reference implementation: https://github.com/AOMediaCodec/libavif
Spec: https://aomediacodec.github.io/av1-avif/

[Philippe Normand]

In Firefox they already use dav1d, so it was decided to also use it for AVIF.

[ChangSeok Oh]

Here is a WIP branch. https://github.com/shivamidow/webkit/tree/avif-decoding
Only still images are supported for now.

[ChangSeok Oh]

Created attachment 421855 [details]
Patch

[ChangSeok Oh]

(In reply to ChangSeok Oh from comment #3)
> Created attachment 421855 [details]
> Patch

No review please. This aims to see EWS responses.

[ChangSeok Oh]

Created attachment 421857 [details]
Patch

[ChangSeok Oh]

Created attachment 421987 [details]
Patch

[ChangSeok Oh]

Created attachment 422003 [details]
Patch

[Philippe Normand]

Comment on attachment 422003 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422003&action=review

> Source/WebCore/ChangeLog:8
> +        This patch brings an initial support of AVIF image format to the gtk port.

I suspect supporting WPE wouldn't be much work, can you take a look? Otherwise it can be handled later on.

[ChangSeok Oh]

Comment on attachment 422003 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422003&action=review

>> Source/WebCore/ChangeLog:8
>> +        This patch brings an initial support of AVIF image format to the gtk port.
> 
> I suspect supporting WPE wouldn't be much work, can you take a look? Otherwise it can be handled later on.

Let me look into wpe later. To be honest, I haven't even built it. :P

[Philippe Normand]

It's very easy to build nowadays. But anyway, I can make a first review round on this patch.

[Philippe Normand]

Comment on attachment 422003 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422003&action=review

> Source/WebCore/platform/image-decoders/avif/AVIFImageDecoder.cpp:52
> +

Empty line can be removed.

> Source/WebCore/platform/image-decoders/avif/AVIFImageReader.h:47
> +    AVIFImageDecoder* m_decoder;

Could this be a RefPtr?

> Source/WebCore/platform/image-decoders/avif/AVIFImageReader.h:48
> +    avifDecoder* m_avifDecoder;

Maybe unique_ptr with custom deleter?

[ChangSeok Oh]

Created attachment 422288 [details]
Patch

[ChangSeok Oh]

Comment on attachment 422003 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422003&action=review

Thanks for your comments!

>> Source/WebCore/platform/image-decoders/avif/AVIFImageDecoder.cpp:52
>> +
> 
> Empty line can be removed.

Removed.

>> Source/WebCore/platform/image-decoders/avif/AVIFImageReader.h:47
>> +    AVIFImageDecoder* m_decoder;
> 
> Could this be a RefPtr?

Replaed this line with RefPtr<AVIFImageDecoder>.

>> Source/WebCore/platform/image-decoders/avif/AVIFImageReader.h:48
>> +    avifDecoder* m_avifDecoder;
> 
> Maybe unique_ptr with custom deleter?

AVIFUniquePtr is newly added.

[Philippe Normand]

Comment on attachment 422288 [details]
Patch

Thanks! LGTM.

[ChangSeok Oh]

Comment on attachment 422288 [details]
Patch

Thanks!

[EWS]

Committed r273970: <https://commits.webkit.org/r273970>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422288 [details].

[Radar WebKit Bug Importer]

<rdar://problem/75095248>

[Ewout ter Hoeven]

Thanks a lot for this effort, it's great to see AVIF decoding support landing in WebKit!

I noticed that currently libavif 0.7.3 (from May 2020) and dav1d 0.7.0 (also from  May 2020) are specified in this patch. I don't know if these versions are used in production or only for CI/tooling, but in either case I would be nice having the latest versions. In a nutshell, dav1d added more assembly code for faster decoding and libavif has improved compatibility and features, since the currently used versions.

For updating dav1d from 0.7.0 to 0.8.2 in tools/gtk I submitted a patch: https://bugs.webkit.org/show_bug.cgi?id=222971

Updating libavif (to 0.9.0) could be a little less straight forward, because it has some breaking changes since the current version (which is 0.7.3), mainly around the CICP refactor (which handels color profiles and such) in 0.8.0.

Also libavif 0.8.2 added the amazing avifIO reader API, which allows parsing meta data and start decoding without having the full AVIF payload yet. 
Chromium has implemented this recently, see this issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1136922

[Jon Bauman]

Seconding the call for updating libdav1d. On Firefox we discovered at least one crashing issue in dav1d with AVIF that was fixed between 0.8.1 and 0.8.2: https://bugzilla.mozilla.org/show_bug.cgi?id=1691376.

[Philippe Normand]

Updates handled in https://bugs.webkit.org/show_bug.cgi?id=224177

[Ewout ter Hoeven]

With Animated AVIF support implemented, dav1d and libavif updated and the parsing fixed, would it be time to be considering flipping the flag and enabling AVIF support by default?

[Philippe Normand]

(In reply to Ewout ter Hoeven from comment #21)
> With Animated AVIF support implemented, dav1d and libavif updated and the
> parsing fixed, would it be time to be considering flipping the flag and
> enabling AVIF support by default?

For the GTK and WPE ports I would be in favor.
For the Apple ports, I can't speak.

[Ewout ter Hoeven]

Awesome! I think it now comes down to these two questions:

1) Can we enable AVIF decoding by default for the GTK and WPE ports only (for now)?
2) Who from Apple could give the go-ahead to flip the flag for Apple ports?

[Ewout ter Hoeven]

dav1d 0.9.0 was released today with significant improvements for 10-bit (HDR) decoding on x64 and a new API to signal events: https://code.videolan.org/videolan/dav1d/-/tags/0.9.0

[Sam Sneddon [:gsnedders]]

(In reply to Ewout ter Hoeven from comment #23)
> Awesome! I think it now comes down to these two questions:
> 
> 2) Who from Apple could give the go-ahead to flip the flag for Apple ports?

The Apple ports use the system-provided image decoding capabilities; we're unlikely to deviate from that here, so any support for AVIF depends on underlying OS support. I can't comment as to whether any future OS releases will support AVIF.

[Vladimir Prelovac]

(In reply to Sam Sneddon [:gsnedders] from comment #25)
> (In reply to Ewout ter Hoeven from comment #23)
> > Awesome! I think it now comes down to these two questions:
> > 
> > 2) Who from Apple could give the go-ahead to flip the flag for Apple ports?
> 
> The Apple ports use the system-provided image decoding capabilities; we're
> unlikely to deviate from that here, so any support for AVIF depends on
> underlying OS support. I can't comment as to whether any future OS releases
> will support AVIF.

AVIF is great and it would be cool to be able to use in a custom-built WebKit on Mac. 

Would you be able to point to what would need to be changed for a custom WebKit build on Mac to support AVIF?