1788866 - Colors on a wide gamut monitor are not correct and cause some AVIF images to be washed out.

Status: RESOLVED FIXED

(Core :: Graphics: Color Management, defect, P3)

Description

[Zaggy1024]

Attachment: firefox_vs_chrome.png

Certain AVIF files may have CICP color space set as follows:

• Color primaries: BT709
• Transfer characteristics: SRGB
• Matrix coefficients: BT601

An example is emotes for Twitch hosted by 7tv, where any AVIF-encoded emotes will display brightened and slightly washed out. Note that the site will not serve AVIF images to Firefox by default, a UA switcher or Chrome may be needed to download them.

Attached is a screenshot comparing the emote modCheck (direct link) in Firefox on the left and Chrome on the right.

Comment 1

[Zaggy1024]

It looks like according to the libavif wiki, the default CICP for encodes is 1/13/6 (BT.709, sRGB, BT.601), so this issue may be quite prevalent in AVIF images around the web.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Do you have a non-animated example that shows the difference?

Comment 3

[Zaggy1024]

Attachment: modcheck_still.avif

I've edited the major_brand to avif so that the example image doesn't animate to make comparison easier.

Comment 4

[Zaggy1024]

Attachment: about_support.json

Since the issue isn't occurring for :jrmuizel, attaching my about:support from my main browser. Note that this is a local build from a recent revision but with AVIF sequence decoding enabled. The issue is also present on stable and in my dev environment.

Comment 5

[Zaggy1024]

Attachment: about_support_ff_104_stable

Comment 6

[Zaggy1024]

Attachment: chrome://gpu

Comment 7

[Zaggy1024]

Attachment: canvas.png

Comment 8

[Zaggy1024]

Attachment: canvas.avif

Comment 9

[Zaggy1024]

I've attached a source PNG file generated here and one converted to AVIF. Both display differently in Firefox when compared with Chrome on my system.

Comment 10

[Zaggy1024]

I noticed this while dealing with some new AVIF gtests that break with my monitor's color profile. It appears that the culprit is these lines that are copy/pasted between nsPNGDecoder and nsAVIFDecoder:

https://searchfox.org/mozilla-central/rev/e9dc497c47107c35bf8d9fc9d683ea4191380927/image/decoders/nsPNGDecoder.cpp#679-682
https://searchfox.org/mozilla-central/rev/1fbcf87b115cec71e82a3c513360c9728ee1dcee/image/decoders/nsAVIFDecoder.cpp#1305-1308

If the check for the presence from alpha is removed and only the else block remains, the color management works correctly and all colors appear correct.

It looks like this code was added by :aosmond, so ni to you: I assume that the check is necessary, due to the comment there, but I can't say I understand the entire scope of this issue. Is there a way to make this work correctly without breaking what I assume is some sort of optimization?

Comment 11

[Zaggy1024]

Also, re-adding the AVIF bug's reference to this, as this issue appears to only happen with PNG and AVIF.

Comment 12

[Zaggy1024]

Attachment: blend.avif

Comment 13

[Zaggy1024]

Attachment: blend_0.png

(In reply to Zaggy1024 from comment #10)

I noticed this while dealing with some new AVIF gtests that break with my monitor's color profile. It appears that the culprit is these lines that are copy/pasted between nsPNGDecoder and nsAVIFDecoder:

After revisiting my test cases, it seems as though comparing a PNG file with alpha and without, the colors are the same. This issue must belimited to AVIF with alpha. The comparison to Chrome was seemingly pointless, since the color management system in it works differently from Firefox.

I've attached new test cases that illustrate the issue. The AVIF displays with a more saturated red than the PNG when using my wide-gamut display profile.

Comment 14

[Adam Fontenot]

Since I don't think anyone else has confirmed this - I do see this issue with the test images on Linux with a WCG (non-HDR) monitor, and color profiles set appropriately. I can also reproduce it by adding an (opaque) alpha channel to an AVIF photo.

Comment 15

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1788866. Fix pixel format type for color management of avif images. r?#gfx-reviewers

This code was copied from the png decoder but it is only applicable to the png decoder because it produces output in rgb order. Whereas the avif decoder produces data in BGR order.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbd927642d73
Fix pixel format type for color management of avif images. r=gfx-reviewers,lsalzman

Comment 17

[tszentpeteri]

Backed out for causing for gtest failures related to ImageFrameAnimator.BlendAVIFWithFilter.

• Backout link
• Push with failures
• Failure Log:

[task 2024-10-10T06:37:33.873Z] 06:37:33     INFO -  TEST-START | ImageFrameAnimator.BlendAVIFWithFilter
[task 2024-10-10T06:37:33.873Z] 06:37:33     INFO -  [Parent 4736, Main Thread] WARNING: '!manager || !manager->CanSend()', file /builds/worker/checkouts/gecko/gfx/layers/ipc/SharedSurfacesChild.cpp:177
[task 2024-10-10T06:37:33.874Z] 06:37:33     INFO -  [Parent 4736, Main Thread] WARNING: '!manager || !manager->CanSend()', file /builds/worker/checkouts/gecko/gfx/layers/ipc/SharedSurfacesChild.cpp:177
[task 2024-10-10T06:37:33.874Z] 06:37:33     INFO -  [Parent 4736, Main Thread] WARNING: '!manager || !manager->CanSend()', file /builds/worker/checkouts/gecko/gfx/layers/ipc/SharedSurfacesChild.cpp:177
[task 2024-10-10T06:37:33.875Z] 06:37:33  WARNING -  TEST-UNEXPECTED-FAIL | ImageFrameAnimator.BlendAVIFWithFilter | Expected equality of these values:
[task 2024-10-10T06:37:33.875Z] 06:37:33     INFO -    expectedPixel
[task 2024-10-10T06:37:33.875Z] 06:37:33     INFO -      Which is: 4278320896
[task 2024-10-10T06:37:33.875Z] 06:37:33     INFO -    gotPixel
[task 2024-10-10T06:37:33.876Z] 06:37:33     INFO -      Which is: 4278255362
[task 2024-10-10T06:37:33.876Z] 06:37:33     INFO -  Color mismatch for rectangle from (0,0) to (50,50): got rgba(0, 255, 2, 255), expected rgba(1, 255, 0, 255)
[task 2024-10-10T06:37:33.876Z] 06:37:33     INFO -   @ /builds/worker/checkouts/gecko/image/test/gtest/Common.cpp:217
[task 2024-10-10T06:37:33.877Z] 06:37:33  WARNING -  TEST-UNEXPECTED-FAIL | ImageFrameAnimator.BlendAVIFWithFilter | Value of: RectIsSolidColor(aSurface, aRect, aInnerColor, aFuzz)
[task 2024-10-10T06:37:33.877Z] 06:37:33     INFO -    Actual: false
[task 2024-10-10T06:37:33.877Z] 06:37:33     INFO -  Expected: true
[task 2024-10-10T06:37:33.877Z] 06:37:33     INFO -   @ /builds/worker/checkouts/gecko/image/test/gtest/Common.cpp:321
[task 2024-10-10T06:37:33.878Z] 06:37:33  WARNING -  TEST-UNEXPECTED-FAIL | ImageFrameAnimator.BlendAVIFWithFilter | test completed (time: 9ms)
[task 2024-10-10T06:37:33.878Z] 06:37:33     INFO -  TEST-START | ImageLoader.DetectGIF

Comment 18

[Timothy Nikkel (:tnikkel)]

Not too surprising that correct color management changes a pixel value by 2 units. I'll bump the fuzz.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8462d2919400
Fix pixel format type for color management of avif images. r=gfx-reviewers,lsalzman

Comment 20

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/8462d2919400