Bug 207750

Summary: AVIF decoding support
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: ImagesAssignee: ChangSeok Oh <changseok>
Status: RESOLVED FIXED    
Severity: Normal CC: 709922234, agafvv, agomez, andrew, annulen, aperez, bjr.roberts, calvaris, changseok, chi187, clopez, contact+bugs.webkit.org, cyb.ai.815, dev, E.M.terHoeven, ews-watchlist, gsnedders, gyuyoung.kim, jarilittlenen, jbauman+webkit, joehoyle, kai.hollberg, kyle.bavender, ltilve, mathias, mehmetgelisin, ncooper, nich, nidoyle, ryuan.choi, sergio, the.bull, thomas, tomac, vprelovac, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=224663
Bug Depends on: 212964    
Bug Blocks:    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch none

Description Philippe Normand 2020-02-14 01:36:28 PST
Reference implementation: https://github.com/AOMediaCodec/libavif
Spec: https://aomediacodec.github.io/av1-avif/
Comment 1 Philippe Normand 2020-03-10 10:57:26 PDT
In Firefox they already use dav1d, so it was decided to also use it for AVIF.
Comment 2 ChangSeok Oh 2020-06-09 21:28:07 PDT
Here is a WIP branch. https://github.com/shivamidow/webkit/tree/avif-decoding
Only still images are supported for now.
Comment 3 ChangSeok Oh 2021-03-01 12:29:44 PST
Created attachment 421855 [details]
Patch
Comment 4 ChangSeok Oh 2021-03-01 12:30:38 PST
(In reply to ChangSeok Oh from comment #3)
> Created attachment 421855 [details]
> Patch

No review please. This aims to see EWS responses.
Comment 5 ChangSeok Oh 2021-03-01 13:05:59 PST
Created attachment 421857 [details]
Patch
Comment 6 ChangSeok Oh 2021-03-02 13:37:20 PST
Created attachment 421987 [details]
Patch
Comment 7 ChangSeok Oh 2021-03-02 15:19:27 PST
Created attachment 422003 [details]
Patch
Comment 8 Philippe Normand 2021-03-02 23:38:09 PST
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.
Comment 9 ChangSeok Oh 2021-03-03 11:18:51 PST
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
Comment 10 Philippe Normand 2021-03-03 11:42:37 PST
It's very easy to build nowadays. But anyway, I can make a first review round on this patch.
Comment 11 Philippe Normand 2021-03-04 08:46:24 PST
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?
Comment 12 ChangSeok Oh 2021-03-04 14:39:45 PST
Created attachment 422288 [details]
Patch
Comment 13 ChangSeok Oh 2021-03-04 14:42:00 PST
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.
Comment 14 Philippe Normand 2021-03-05 00:29:51 PST
Comment on attachment 422288 [details]
Patch

Thanks! LGTM.
Comment 15 ChangSeok Oh 2021-03-05 08:04:28 PST
Comment on attachment 422288 [details]
Patch

Thanks!
Comment 16 EWS 2021-03-05 08:15:12 PST
Committed r273970: <https://commits.webkit.org/r273970>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422288 [details].
Comment 17 Radar WebKit Bug Importer 2021-03-05 08:16:17 PST
<rdar://problem/75095248>
Comment 18 Ewout ter Hoeven 2021-03-09 13:01:05 PST
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
Comment 19 Jon Bauman 2021-03-09 14:46:25 PST
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.
Comment 20 Philippe Normand 2021-04-05 01:44:46 PDT
Updates handled in https://bugs.webkit.org/show_bug.cgi?id=224177
Comment 21 Ewout ter Hoeven 2021-04-14 01:03:52 PDT
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?
Comment 22 Philippe Normand 2021-04-14 01:10:01 PDT
(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.
Comment 23 Ewout ter Hoeven 2021-04-15 08:29:23 PDT
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?
Comment 24 Ewout ter Hoeven 2021-05-16 11:57:49 PDT
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
Comment 25 Sam Sneddon [:gsnedders] 2021-08-11 04:34:35 PDT
(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.
Comment 26 Vladimir Prelovac 2021-08-24 14:16:04 PDT
(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?