Closed Bug 1783458 Opened 4 years ago Closed 1 month ago

MP4 colors are washed out

Categories

(Core :: Graphics, defect, P3)

Firefox 103
Desktop
All
defect

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox-esr91 --- wontfix
firefox-esr115 --- wontfix
firefox-esr140 --- wontfix
firefox104 --- wontfix
firefox105 --- wontfix
firefox106 --- wontfix
firefox152 --- wontfix
firefox153 --- wontfix
firefox154 --- wontfix

People

(Reporter: ben, Assigned: bradwerth)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Attached image source gif

Steps to reproduce:

An MP4 created from an sRGB GIF has washed out colors compared to other browsers. A sample GIF and MP4 are attached. The GIF was converted to an MP4 using ffmpeg 5.1 with

ffmpeg -f gif \
    -i the.gif \
    -r 50 \
    -fps_mode auto \
    -color_primaries bt709 \
    -color_trc iec61966_2_1 \
    -colorspace smpte170m \
    -pix_fmt yuv420p \
    -c:v libx264 \
    -preset medium \
    -profile:v main \
    -crf 20 \
    -movflags +faststart \
    -f mp4 \
    the.mp4



Actual results:

Playing the MP4 will show a background pink quite different from the source GIF and different from Chrome and Safari on the same hardware, a MacBook Pro (16-inch, 2021). 


Expected results:

I expected the color to match other browsers. It appears that firefox is ignoring the colorspace / transfer / primaries tagged on the MP4. Changing the flags in the above ffmpeg command to all use bt709 should change the rendering in browser, but does not.
Attached video gif converted to mp4

Managed to reproduce this on macOS 11.6 and on Ubuntu 20.04, on Windows 10 the issue isn't visible.

Severity: -- → S4
Status: UNCONFIRMED → NEW
Has STR: --- → yes
Component: Untriaged → Graphics
Ever confirmed: true
OS: Unspecified → All
Product: Firefox → Core
Hardware: Unspecified → Desktop

I think this may have been regressed by bug 1768824 ("Set macOS transfer functions independently of colorSpace", landed 2022-05-31, shipped in Firefox 103) — on macOS at least, which fits the 103–106 window in comment 0.

Before bug 1768824, the macOS path attached an IOSurfaceTransferFunction value only on the BT709- and BT2020-matrix branches. Content with a BT601 matrix (smpte170m) — which is what this MP4 uses — had no transfer function attached at all, so CoreAnimation did no transfer color management on it.

Bug 1768824 replaced that with a single unconditional IOSurfaceSetValue(…, kIOSurfaceTransferFunction, …) for all content (in MacIOSurface.cpp). From 103 onward, this stream's frames get IOSurfaceTransferFunction = sRGB attached, and CoreAnimation applies the sRGB EOTF. Applying an sRGB transfer where previously none was applied yields the washed-out result.

That commit's stated goal was an HDR/BT2020 fix; extending the transfer attachment to SDR BT601 content appears to have been incidental?

Windows is unaffected because the standard WMF/<video> decode path sets only the YUV matrix and never attaches a transfer function, so it still renders this content the pre-103 way.

The Ubuntu 20.04 repro in comment 3 is a loose end... As far as I can tell Firefox has no Linux SDR transfer-application path at all (the Linux ffmpeg path sets only the YUV matrix and range, with transfer consulted solely for HDR), so if it still washes out on Linux that's probably a separate cause. Might be worth a re-test on current Firefox/Linux, but I don't have hardware here to test with.

Possible fix: restore the prior behavior for BT601-matrix SDR content (don't attach a transfer function on that branch) or otherwise special-case SDR where attaching the declared transfer diverges from how the stream was authored.

Repro is unchanged from comment 0; still reproduces on current Firefox on macOS.

Keywords: regression
Regressed by: 1768824

Set release status flags based on info from the regressing bug 1768824

:bradwerth, since you are the author of the regressor, bug 1768824, could you take a look?

For more information, please visit BugBot documentation.

Good testcase, thank you for filing. I'll fix this.

Assignee: nobody → bwerth
Flags: needinfo?(bwerth)
Priority: -- → P3

(In reply to ben from comment #3)

I think this may have been regressed by bug 1768824 ("Set macOS transfer functions independently of colorSpace", landed 2022-05-31, shipped in Firefox 103) — on macOS at least, which fits the 103–106 window in comment 0.

Before bug 1768824, the macOS path attached an IOSurfaceTransferFunction value only on the BT709- and BT2020-matrix branches. Content with a BT601 matrix (smpte170m) — which is what this MP4 uses — had no transfer function attached at all, so CoreAnimation did no transfer color management on it.

Not sure we ever handled this case correctly, but Bug 1768824 is not the regressor. The issue is not with the transfer function, but with the ycbcr matrix (which is bt601 for this video). The relevant bit of code in AppleVTDecoder::OutputFrame forces the matrix to match the colorspace. Comment that out, and it looks correct.

It's possible that we can fully trust the Apple decoder, and stop overriding the matrix, color primaries, transfer function of the CVPixelBuffer it emits. Our test coverage is not great for this code path. I'll post a patch and discuss with reviewers the best way forward.

No longer regressed by: 1768824

For historic reasons, the VideoInfo config does not carry information on
the matrix, instead folding it into the colorspace for most cases. When
playing a video with an unusual config of 709 colorspace with 601
matrix, this change ensures color correctness. BT2020 colorspace videos
continue to be forced to BT2020 matrix because without it they render as
SDR BT601 (the default).

This changes the color primaries to be based on the decoder config,
rather than derived from the colorspace.

Blocks: hdr-test

(In reply to ben from comment #0)

An MP4 created from an sRGB GIF has washed out colors compared to other browsers. A sample GIF and MP4 are attached. The GIF was converted to an MP4 using ffmpeg 5.1

with ffmpeg 7.1.1, the resulting mp4 seems to not contain primaries or transfer function. I had to post-process the generated mp4 with

ffmpeg -i input.mp4 -vf setparams=color_primaries=bt709:color_trc=iec61966-2-1 output.mp4

And now it seems to match the properties in attachment 9288654 [details]. I'll try to build a test around this, add that to the patch.

Confirmed on the change to the ffmpeg params.

We're doing it on 7.x with -x264-params colorprim=bt709:transfer=iec61966-2-1:colormatrix=smpte170m during the encode.

Now that I'm trying to add a test, I'm not sure this "fix" is worth landing. The changes in the patch rely on two assumptions for mp4s encoded with BT709 primaries and BT601 matrix coeffeicients:

  1. VideoToolbox will tag the pixel buffer with a BT601 CVImageBufferYCbCrMatrix property for all such videos.

This assumption is not holding. For some input images, including the one in this Bug report, attachment 9288653 [details], the output mp4 matrix is correctly tagged. For the test image we use for color transform tests, when run through the same ffmpeg translation, the output mp4 matrix is not correctly tagged. VideoToolbox decode is a black box, so I can't determine what is happening. It may be because of the square aspect ratio, but that's just a guess. This is an Apple issue that we can't directly do anything about.

  1. When we pass along this property on the video surface, it will appear with correct color.

This isn't true, either. Comparing the image and video attached to this Bug, the colors are improved with correct tagging, but not identical. I created a pink square gif to convert with ffmpeg, and these are the results.

Source: #fd80e9
Firefox: #fba1ef
w/patch: #fc8cec
Chrome: #f57ce3

The colors get closer, but they are still faded from the source image. Given these problems, I think the patch would only add an edge case to already confusing code, for minimal benefit. I'm going to mark this Bug WONTFIX for now.

Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → WONTFIX
Attachment #9614070 - Attachment is obsolete: true

Just a note that the colors will never exactly match due to the YUV color space transformation. We were just hoping to get Firefox to match what we see on safari and chrome on Mac. When we rolled out gif to mp4 transforms originally, we got a ton of pushback on the color wash out from our gif authors, who also skew towards Firefox.

I’m bummed that we can’t fix it, but understand this is pretty complex. If access to our full color fidelity test corpus would help, I’d be happy to open source it.

(In reply to ben from comment #11)

Just a note that the colors will never exactly match due to the YUV color space transformation. We were just hoping to get Firefox to match what we see on safari and chrome on Mac. When we rolled out gif to mp4 transforms originally, we got a ton of pushback on the color wash out from our gif authors, who also skew towards Firefox.

I'll prepare a reduced test application to use while discussing this issue with Apple. It would be nice to know why the VideoToolbox interprets two image-to-mp4s encoded with the same ffpmeg command in two different ways. If we can get an answer for that, maybe we can build robust handling in Firefox.

In the meantime, you will have better success in Firefox if you use BT709 primaries and matrix for these videos. Using BT601 matrix is a case we'll continue to handle poorly for the moment.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: