Open Bug 1682995 (avif-default) Opened 6 months ago Updated 11 days ago

Enable AVIF support by default

Categories

(Core :: ImageLib, task, P1)

Product:
Core
Component:
ImageLib
Type:
task

Tracking

()

Status:
REOPENED
Tracking Flags:
Tracking Status
relnote-firefox --- ?
firefox-esr78 --- wontfix
firefox86 + verified disabled
firefox87 --- disabled
firefox88 --- disabled
firefox89 --- disabled

People

(Reporter: jbauman, Assigned: jbauman)

References

(Depends on 4 open bugs, Blocks 2 open bugs)

Details

(Keywords: dev-doc-needed)

Attachments

(2 files)

Bug 1682995 - Enable AVIF support by default. r=aosmond,jaws
47 bytes, text/x-phabricator-request
Details | Review
Bug 1682995 - Revert Enable AVIF support by default. r=aosmond,jaws,valentin
48 bytes, text/x-phabricator-request
Details | Review

What it says on the tin, basically. This amounts to setting image.avif.enabled to true and letting it ride the trains to release.

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/feef6926b977
Enable AVIF support by default. r=aosmond,preferences-reviewers

Backed out changeset feef6926b977 (bug 1682995) for test_accept_header.html failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=f737176fe4c7c6bb742315b5fcc44acc7e75af3d&tochange=97bb496fd391cd8bb30f65001a637f074a35be94&searchStr=mochitest&selectedTaskRun=EQV68_GGR6-czzsAYRylkA.0

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

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

[task 2020-12-17T19:53:15.275Z] 19:53:15     INFO - TEST-START | netwerk/test/mochitests/test_accept_header.html
[task 2020-12-17T19:53:15.452Z] 19:53:15     INFO -  test_accept_header /tests/netwerk/test/mochitests/test_accept_header.sjs?iframe
[task 2020-12-17T19:53:15.529Z] 19:53:15     INFO -  test_accept_header /tests/netwerk/test/mochitests/test_accept_header.sjs?get
[task 2020-12-17T19:53:15.550Z] 19:53:15     INFO - TEST-PASS | netwerk/test/mochitests/test_accept_header.html | Expected: iframe 
[task 2020-12-17T19:53:15.552Z] 19:53:15     INFO - TEST-INFO | started process screentopng
[task 2020-12-17T19:53:15.568Z] 19:53:15     INFO -  test_accept_header /tests/netwerk/test/mochitests/test_accept_header.sjs?image
[task 2020-12-17T19:53:15.596Z] 19:53:15     INFO -  test_accept_header /tests/netwerk/test/mochitests/test_accept_header.sjs?get
[task 2020-12-17T19:53:15.624Z] 19:53:15     INFO -  test_accept_header /tests/netwerk/test/mochitests/test_accept_header.sjs?style
[task 2020-12-17T19:53:15.640Z] 19:53:15     INFO -  test_accept_header /tests/netwerk/test/mochitests/test_accept_header.sjs?get
[task 2020-12-17T19:53:15.680Z] 19:53:15     INFO -  test_accept_header /tests/netwerk/test/mochitests/test_accept_header.sjs?worker
[task 2020-12-17T19:53:15.696Z] 19:53:15     INFO -  test_accept_header /tests/netwerk/test/mochitests/test_accept_header.sjs?get
[task 2020-12-17T19:53:15.813Z] 19:53:15     INFO - TEST-INFO | screentopng: exit 0
[task 2020-12-17T19:53:15.813Z] 19:53:15     INFO - TEST-UNEXPECTED-FAIL | netwerk/test/mochitests/test_accept_header.html | Accept header: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8 - got "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8", expected "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8"
[task 2020-12-17T19:53:15.813Z] 19:53:15     INFO -     SimpleTest.is@SimpleTest/SimpleTest.js:500:14
[task 2020-12-17T19:53:15.813Z] 19:53:15     INFO -     test_last_request_and_continue/<@netwerk/test/mochitests/test_accept_header.html:20:7
[task 2020-12-17T19:53:15.813Z] 19:53:15     INFO - TEST-PASS | netwerk/test/mochitests/test_accept_header.html | Expected: image 
[task 2020-12-17T19:53:15.813Z] 19:53:15     INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-12-17T19:53:15.813Z] 19:53:15     INFO - TEST-UNEXPECTED-FAIL | netwerk/test/mochitests/test_accept_header.html | Accept header: image/webp,*/* - got "image/avif,image/webp,*/*", expected "image/webp,*/*"
[task 2020-12-17T19:53:15.813Z] 19:53:15     INFO -     SimpleTest.is@SimpleTest/SimpleTest.js:500:14
[task 2020-12-17T19:53:15.813Z] 19:53:15     INFO -     test_last_request_and_continue/<@netwerk/test/mochitests/test_accept_header.html:20:7
[task 2020-12-17T19:53:15.814Z] 19:53:15     INFO - TEST-PASS | netwerk/test/mochitests/test_accept_header.html | Expected: style 
[task 2020-12-17T19:53:15.814Z] 19:53:15     INFO - TEST-PASS | netwerk/test/mochitests/test_accept_header.html | Accept header: text/css,*/*;q=0.1 
[task 2020-12-17T19:53:15.815Z] 19:53:15     INFO - TEST-PASS | netwerk/test/mochitests/test_accept_header.html | Expected: worker 
[task 2020-12-17T19:53:15.815Z] 19:53:15     INFO - TEST-PASS | netwerk/test/mochitests/test_accept_header.html | Accept header: */* 
[task 2020-12-17T19:53:15.815Z] 19:53:15     INFO - GECKO(3092) | MEMORY STAT | vsize 2564MB | residentFast 155MB | heapAllocated 10MB
[task 2020-12-17T19:53:15.816Z] 19:53:15     INFO - TEST-OK | netwerk/test/mochitests/test_accept_header.html | took 462ms
Flags: needinfo?(jbauman)

The following also seems to start perma failing with the backed out changes: https://treeherder.mozilla.org/logviewer?job_id=324855672&repo=autoland&lineNumber=4508

Backout by csabou@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/26d8d2248fcd
Backed out changeset feef6926b977 for test_accept_header.html failures CLOSED TREE

I'm currently looking at this. The fix to test_accept_header.html is straightforward, but it led me to run a full mochitest suite and that showed some less-obvious errors. In particular image/test/mochitest/test_bug496292.html is failing, but it took me awhile (I got sidetracked by bug 1682709) to track down why.

To hopefully avoid another backout, I've just replaced all the instances of "image/webp,*/*" (our previous Accept header) in our test code with "image/avif,image/webp,*/*". I'll run another try and hopefully that one will be green.

Flags: needinfo?(jbauman)

Sadly there still seem to be some test failures that, while it's not immediately clear why they'd be affected by this, are locally reproducible only with this change. For example: devtools/client/netmonitor/test/browser_net_copy_headers.js.

So, I'm looking into those and won't try to re-land until I've got them sorted.

(In reply to Jon Bauman [:jbauman:] from comment #7)

Sadly there still seem to be some test failures that, while it's not immediately clear why they'd be affected by this, are locally reproducible only with this change. For example: devtools/client/netmonitor/test/browser_net_copy_headers.js.

That test will poll the clipboard until it finds an expected strings. When it can't find the expected string it will time out. https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/test/browser_net_copy_headers.js#45

So, I'm looking into those and won't try to re-land until I've got them sorted.

Thanks Tom, I found that as well (unfortunately before I saw your comment). Not sure how I missed it the first time around, but I'm pretty sure I've addressed all the locations in test code where image/webp appears and if this next try run looks good, I'll try to land this again.

I don't think any of the remaining failures are due to the changes for this issue. However, I don't want to land a change directly before being unavailable during the holiday time, so wait on this until I'm back.

Hi! Excited to see this, but maybe bug 1654461 should be fixed first? Both libavif and squoosh.app produce full range AVIF images by default, and it's better to use full range in image encoding to prevent banding artifact.

https://code.videolan.org/videolan/dav1d/-/blob/master/NEWS
https://code.videolan.org/videolan/dav1d/-/releases

Please update to dav1d 0.8.1 if possible for faster AVIF decoding.

Per bug 1680396, we'll update to the latest dav1d version later this month

Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4267ac191113
Enable AVIF support by default. r=aosmond,preferences-reviewers,necko-reviewers,valentin

Seems like something we'd want to relnote.

relnote-firefox: --- → ?

Indeed! Thanks, Ryan

https://hg.mozilla.org/mozilla-central/rev/4267ac191113

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
status-firefox86: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

Jon, do you have a suggestion for the wording of the release note and maybe an url to an explainer we could link to from the release notes? Thanks

Flags: needinfo?(jbauman)

Hi Pascal, this is adapted from what I suggested back when AVIF was partially implemented, but disabled by default

[Why is this notable]: A new image format with excellent compression and no patent restrictions developed by the Alliance for Open Media
[Suggested wording]: Basic AVIF support is enabled in this release. Some advanced features like animated images and colorspace support is still in development. See https://bugzilla.mozilla.org/show_bug.cgi?id=1682995 and related issue for more detail. It can be disabled by setting image.avif.enabled to false in about:config.
[Links (documentation, blog post, etc)]: https://developer.mozilla.org/en-US/docs/Web/Media/Formats/Image_types#AVIF and https://en.wikipedia.org/wiki/AV1#AV1_Image_File_Format_(AVIF)

I'm not sure if we tend to include links to non-mozilla blog posts in these kinds of notes, but https://jakearchibald.com/2020/avif-has-landed/ is a very good write up on the format generally and includes several examples.

Flags: needinfo?(jbauman)

I went with this note:

Basic AVIF support is enabled in Firefox 86. Some advanced features like animated images and colorspace support are still in development (see bug 1682995 and dependencies)

I am linking to our own documentation and adding the dev-doc-needed keyword to this bug as we will need to also add this information to https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/86 and also update https://developer.mozilla.org/docs/Web/Media/Formats/Image_types#AVIF to indicate that this is no longer behind an about:config flag.

The final release note will likely be rewritten by our marketing team for a more end-user focused wording.

Thanks!

Keywords: dev-doc-needed

FYI Documentation for the FF86 release of this feature is now done/being reviewed. This was mostly already done (we thought this would release in FF83). Essentially this confirms the MIME types, change to HTTP accepts, release note, removes experimental tagging etc.

Full details can be found here: https://github.com/mdn/content/issues/1726#issuecomment-769577479

I've commented over on 1443863 that it will be problematic for AVIF to be enabled by default until bugs around colour accuracy are addressed. Please reconsider enabling AVIF by default until colour is working. This will have long lasting implications for people and services who want to serve AVIF and trust the Accept header.

I agree. Are we sure we want to ship this without colorspace support?

Flags: needinfo?(jbauman)

I'm sorry to keep pushing on this so aggressively but the situation is more problematic than I had initially thought.

With the release of Firefox 86 in two days, this needs urgent attention. I would have pushed on this sooner but I was sideswiped by the fact that AVIF support was enabled by default before all the blockers for bug 1443863 were resolved.

AVIF support, as it stands now is too broken to ship.

To recap the situation:

Any image encoded with libheif (used by ImageMagick for example) will have completely inaccurate colour in Firefox. This is clearly visible in attachment 9202460 [details] and discussed in more detail in bug 1634741. This is not a problem with other browsers with AVIF support.

Colour is broken on all displays, both sRGB and wide gamut, and personally observed in both macOS and Linux X11.

These are not esoteric AVIF images or unusual input images. This happens with a plain old JPEG without accompanying ICC profile (the most common kind) encoded to AVIF with default parameters with one of the most common AVIF codecs (libheif/libaom, as used by ImageMagick for example).

The situation is made worse because this version of Firefox will start advertising AVIF support in the Accept header. In order to properly do content negotiation, servers need to trust the Accept header and assume that support for anything in the Accept header is at the lowest common denominator for browsers that advertise that format. If Firefox 86 ships with AVIF enabled today, with advertising in the Accept header for support, we now must assume that the lowest common denominator has completely inaccurate colour. This means that servers must assume that browsers advertising AVIF support in the Accept header will have broken colour. The only workaround is to use User-Agent sniffing to determine real working AVIF support which is completely antithetical to how content negotiation is supposed to work.

Similar issues made the Accept header useless for years with regards to WebP support in Chrome (around introducing features like transparency, lossless encoding, and animation that older browsers didn't support). Making the same mistake here will have very long lasting repercussions.

Image format "support" with such inaccurate colour is not "support". Rendering an image isn't meaningful if the image is rendered so incorrectly. AVIF in Firefox is not ready.

Look, I'm as excited as anyone for AVIF support in Firefox (it's why I'm pushing so hard here), and can't wait to enable it in our imaging products at Akamai, but enabling it as is today is going to do more harm than good.

I'm a "nobody", I'm not sure my "vote" counts. But still want to say I agree with Nick. I want AVIF to succeed. To do that we must be able to trust a browser saying it can handle AVIF to show expected colors if we choose to server it AVIF instead of a fallback jpg.
I can easily accept lack of f.ex. animation-support, that's a very special case. But missing proper colorspace handling in 2021, that's a no go.

Just to be clear, these incorrect colours happen without colour profiles even being involved.

This happens in the most base of base case: untagged JPEG converted to AVIF with default settings using libheif/libaom (with ImageMagick for example) viewed on any display.

Status update: We talked about not shipping this on Friday, unfortunately it was very late in the release schedule, would have required spinning a new RC and potentially missing the release date. We still have the option of disabling this by a remotely via pref flip and we'll talk about doing that tomorrow.

It, indeed, would've been better if these concerns had been raised earlier instead of after the last RC.

Nick, since Firefox Nightly and Beta have been setting the Accept header for a while, I assume Akamai is not actually serving AVIF based off of that yet?

Flags: needinfo?(njdoyle)

Akamai hasn't publicly exposed serving AVIF by the Accept header yet. Initial support is imminent. We've had to add a block for Firefox in anticipation for the 86 release. Having a misbehaving browser released will complicate matters though because of sensitivity to serving only colour accurate images even to lingering outdated browser versions.

Flags: needinfo?(njdoyle)

Also, to be fair, concerns were raised 11 days ago in bug 1634741 and the impact was shrouded by bug 1443863 being still open also with related open blockers.

Would it make things any better if I can submit a patch to fix the most common cases now?

Depends on: 1694113

Chris, we are not going to ship AVIF in 86 tomorrow, could the dev docs be updated to reflect that? Thanks

Flags: needinfo?(cmills)

Thanks :pascalc, I was reading this thread with interest, and waiting for the call on it.

I'll remove mention of it from the release notes and release blog post immediately, and get Hamish to revert any specific mentions in the docs elsewhere.

Flags: needinfo?(cmills)
Keywords: dev-doc-complete → dev-doc-needed
Flags: needinfo?(jbauman)

FYI, docs work "undone" once the fixes are merged. Info about what I changed in https://github.com/mdn/content/issues/1726#issuecomment-783871008. We can re-enable pretty easily by reverting those changes; though at that point you might have added support for animations etc, which would imply a few addition tidy-ups.

Jon, can you take care of the patch for 87/88? I don't know if we want to keep the feature in nightly and/or early-beta for now, or disabled entirely.

Flags: needinfo?(jbauman)

Disabled in 86 with a backout.

status-firefox86: affected → verified disabled

(In reply to Julien Cristau [:jcristau] from comment #35)

Jon, can you take care of the patch for 87/88? I don't know if we want to keep the feature in nightly and/or early-beta for now, or disabled entirely.

I believe everything is set for now (thanks to you, Julien). AVIF remains enabled for Fx87 nightly/beta, but disabled for Fx87 release. Let me know if I'm missing something since this is my first experience with a back-out like this.

We'll shoot for enabling in Fx88 release, but make sure we have the issues that derailed the Fx86 release plan sorted out first.

Flags: needinfo?(jbauman) → needinfo?(jcristau)

87 is currently on mozilla-beta, so AVIF is set to enabled there. I'll keep the needinfo to turn that off later this week.

Depends on: 1654461
Depends on: 1634741
Depends on: 1696045
Depends on: 1689806

I was unable to figure out why Firefox doesn't accept my transparent AVIF images, which do work in Chrome: https://bugzilla.mozilla.org/show_bug.cgi?id=1696056

Depends on: 1696088

Hi Jon, I just wanted to confirm that we're targeting Fx89 now and want to land the patch from comment 40 on Beta again after 88 merges there on Monday?

Flags: needinfo?(jbauman)

That is correct. Thanks for confirming.

Flags: needinfo?(jbauman)

Backed out from 88 as well for 88.0b7.
https://hg.mozilla.org/releases/mozilla-beta/rev/ddaaeb918e1f

Given that we plan to let this ride Fx89 to release and we've been handling the disabling by backing out the patch which enabled it by default, I think there's no further action required in this bug.

Status: REOPENED → RESOLVED
Closed: 5 months ago → 3 months ago
status-firefox88: fixed → disabled
status-firefox89: affected → fixed
Resolution: --- → FIXED

This would be easier to track for non-Mozilla people if this bug was not resolved until the blockers this bug depends on are also resolved. AVIF is not ready to ship until those bugs are finished.

Hey Nick,

Sorry for the confusion here. The bug is correctly RESOLVED FIXED at the moment because AVIF is currently enabled by default in nightly, and absent intervention will be going through our normal release process to ship in Fx89. Yes, there are incomplete blocking issues and our expectation is that those will be resolved in time for shipping in Fx89. You can rest assured that this bug will get updated in response to any changes to that plan.

Revert this change to turn AVIF off by default for now

Attachment #9215549 - Attachment description: Bug 1682995 - Enable AVIF support by default. r=aosmond,jaws,valentin → Bug 1682995 - Revert Enable AVIF support by default. r=aosmond,jaws,valentin
Pushed by jbauman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6646eb3b62f7
Revert Enable AVIF support by default. r=aosmond,valentin,necko-reviewers,preferences-reviewers,Gijs
Status: RESOLVED → REOPENED
status-firefox89: fixed → disabled
tracking-firefox89: blocking → ---
Resolution: FIXED → ---
Target Milestone: 86 Branch → ---

Revert of this bug seems to have caused bug 1702250 to regress as it relies on optimized avif image to use as background for MR1 new user onboarding flow. Is there an ETA on fix or uplift of blocking issue into Fx89, we were relying on optimized light weight avif images to show up as part of first about:welcome screen. Thanks!

Flags: needinfo?(jbauman)
Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Punam Dahiya [:pdahiya] from comment #50)

Revert of this bug seems to have caused bug 1702250 to regress as it relies on optimized avif image to use as background for MR1 new user onboarding flow. Is there an ETA on fix or uplift of blocking issue into Fx89, we were relying on optimized light weight avif images to show up as part of first about:welcome screen. Thanks!

Uh, if we're disabling avif on nightly by default for 89 (which is what this bug does), I don't think you can rely on it for release, so I would suggest updating the images to not use avif.

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(pdahiya)

(In reply to :Gijs (he/him) from comment #51)

(In reply to Punam Dahiya [:pdahiya] from comment #50)

Revert of this bug seems to have caused bug 1702250 to regress as it relies on optimized avif image to use as background for MR1 new user onboarding flow. Is there an ETA on fix or uplift of blocking issue into Fx89, we were relying on optimized light weight avif images to show up as part of first about:welcome screen. Thanks!

Uh, if we're disabling avif on nightly by default for 89 (which is what this bug does), I don't think you can rely on it for release, so I would suggest updating the images to not use avif.

Started patch to use webp in Bug 1705499, hopefully we see avif turned on by default in 90

Flags: needinfo?(pdahiya)

If even we enable avif by default on release we should not be relying on it in our chrome code until at least one release after the first release with it enabled in case we need to disable it for some critical issue, even if that is unlikely we don't want to have to worry about breaking anything else.

Blocks: 1705799
Alias: avif-default
Flags: needinfo?(jbauman)
Depends on: 1706160
Depends on: 1712368
You need to log in before you can comment on or make changes to this bug.