Closed Bug 1682995 Opened 1 month ago Closed 7 days ago

Enable AVIF support by default

Categories

(Core :: ImageLib, task, P1)

Product:
Core
Component:
ImageLib
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
86 Branch
Tracking Flags:
Tracking Status
relnote-firefox --- ?
firefox86 + fixed

People

(Reporter: jbauman, Assigned: jbauman)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(1 file)

Bug 1682995 - Enable AVIF support by default. r=aosmond,jaws
47 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: 7 days 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
You need to log in before you can comment on or make changes to this bug.