Closed Bug 1788866 Opened 2 years ago Closed 2 months ago

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

Categories

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

x86_64
Windows 11
defect

Tracking

()

RESOLVED FIXED
133 Branch
Tracking Status
firefox133 --- fixed

People

(Reporter: Zaggy1024, Assigned: tnikkel)

References

(Blocks 1 open bug)

Details

Attachments

(8 files, 2 obsolete files)

Attached image 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.

Blocks: AVIF
Summary: Image color space conversion does not respect matrix coefficients for SRGB images. → Image CICP color space conversion does not respect matrix coefficients for sRGB images.

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.

Flags: needinfo?(jmuizelaar)

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

Flags: needinfo?(jmuizelaar) → needinfo?(Zaggy1024)
Attached image modcheck_still.avif

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

Flags: needinfo?(Zaggy1024)
Summary: Image CICP color space conversion does not respect matrix coefficients for sRGB images. → AVIF images from 7tv appear washed out.
Attached file 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.

OS: All → Windows 11
Hardware: All → x86_64
Attached file chrome://gpu
Attached image canvas.png (obsolete) —
Attached image canvas.avif (obsolete) —

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.

Summary: AVIF images from 7tv appear washed out. → Colors on a wide gamut monitor are not correct and cause some AVIF images to be washed out.
Severity: -- → S3
Priority: -- → P3
No longer blocks: AVIF

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?

Flags: needinfo?(aosmond)

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

Depends on: AVIF
Attached image blend.avif
Attachment #9292985 - Attachment is obsolete: true
Attached image 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.

Attachment #9292984 - Attachment is obsolete: true
Blocks: AVIF
No longer depends on: AVIF

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.

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.

Assignee: nobody → tnikkel
Status: NEW → ASSIGNED
Flags: needinfo?(aosmond)
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fbd927642d73 Fix pixel format type for color management of avif images. r=gfx-reviewers,lsalzman

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

[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
Flags: needinfo?(tnikkel)

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

Flags: needinfo?(tnikkel)
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8462d2919400 Fix pixel format type for color management of avif images. r=gfx-reviewers,lsalzman
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 133 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: