Bug 207750

Summary: AVIF decoding support
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: ImagesAssignee: ChangSeok Oh <changseok>
Status: RESOLVED FIXED    
Severity: Normal CC: 709922234, 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

Philippe Normand
Reported 2020-02-14 01:36:28 PST
Attachments
Patch (509.01 KB, patch)
2021-03-01 12:29 PST, ChangSeok Oh
ews-feeder: commit-queue-
Patch (525.58 KB, patch)
2021-03-01 13:05 PST, ChangSeok Oh
no flags
Patch (30.20 KB, patch)
2021-03-02 13:37 PST, ChangSeok Oh
no flags
Patch (33.16 KB, patch)
2021-03-02 15:19 PST, ChangSeok Oh
no flags
Patch (35.56 KB, patch)
2021-03-04 14:39 PST, ChangSeok Oh
no flags
Philippe Normand
Comment 1 2020-03-10 10:57:26 PDT
In Firefox they already use dav1d, so it was decided to also use it for AVIF.
ChangSeok Oh
Comment 2 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.
ChangSeok Oh
Comment 3 2021-03-01 12:29:44 PST
ChangSeok Oh
Comment 4 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.
ChangSeok Oh
Comment 5 2021-03-01 13:05:59 PST
ChangSeok Oh
Comment 6 2021-03-02 13:37:20 PST
ChangSeok Oh
Comment 7 2021-03-02 15:19:27 PST
Philippe Normand
Comment 8 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.
ChangSeok Oh
Comment 9 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
Philippe Normand
Comment 10 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.
Philippe Normand
Comment 11 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?
ChangSeok Oh
Comment 12 2021-03-04 14:39:45 PST
ChangSeok Oh
Comment 13 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.
Philippe Normand
Comment 14 2021-03-05 00:29:51 PST
Comment on attachment 422288 [details] Patch Thanks! LGTM.
ChangSeok Oh
Comment 15 2021-03-05 08:04:28 PST
Comment on attachment 422288 [details] Patch Thanks!
EWS
Comment 16 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].
Radar WebKit Bug Importer
Comment 17 2021-03-05 08:16:17 PST
Ewout ter Hoeven
Comment 18 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
Jon Bauman
Comment 19 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.
Philippe Normand
Comment 20 2021-04-05 01:44:46 PDT
Ewout ter Hoeven
Comment 21 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?
Philippe Normand
Comment 22 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.
Ewout ter Hoeven
Comment 23 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?
Ewout ter Hoeven
Comment 24 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
Sam Sneddon [:gsnedders]
Comment 25 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.
Vladimir Prelovac
Comment 26 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?
Note You need to log in before you can comment on or make changes to this bug.