1459526 - Firefox doesn't play full color range video correctly

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[fireattack [:fireattack]]

Attachment: firefox fullrange video bug.png

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.30 Safari/537.36

Steps to reproduce:

Open any full color range (instead of the common "limited") video.

I found one on Twitch: https://clips.twitch.tv/FaithfulBitterOxSSSsss

I also dumped the video file, which gives the same result if you play it locally in Firefox:

https://www.dropbox.com/s/2wupchv45nncgts/Fullrange.mp4?dl=0

Color space                    : YUV
Chroma subsampling             : 4:2:0
Bit depth                      : 8 bits
Scan type                      : Progressive
Bits/(Pixel*Frame)             : 0.040
Stream size                    : 17.8 MiB (96%)
Color range                    : Full            <-------------
Color primaries                : BT.709
Transfer characteristics       : BT.709
Matrix coefficients            : BT.709



Actual results:

Since the video is full range, it should be played as-is, without any further limited->full conversion on PC.


Expected results:

Firefox incorrectly converts the color range and therefore clips the color. Makes the contrast too high.

It plays flawlessly in any local capable video player, and Chrome (see attachment for comparison).

This bug happens with both hardware acceleration ON and OFF.

Comment 1

[tgn-ff]

Attachment: about_support_linux

Same issue on Linux. There is a color difference between Firefox, and Chromium and mpv. Not sure which one is correct, though. Tested in a new profile. I'm attaching by about:support in case it's needed.

I tested it by setting the color range with xrandr to all three possible values ('Automatic', 'Full' and 'Limited 16:235').

Comment 2

[Nils Ohlmeier [:drno]]

I'm assuming that this is not a recent regression. Please let us know if this was working in a Firefox version before.

Comment 3

[tgn-ff]

I tried versions 38.0.5, 45.0.2 and 52.0.2 from release channel.

I couldn't play the video in reporter's dropbox with 38.

52 looks exactly as current Nightly and Release.

There is a difference between 45 and 52, but still it didn't look like mpv (assuming that's how it should look).

Comment 4

[fireattack [:fireattack]]

Attachment: full range video sample

Attached a full range sample to demonstrate the problem better.

The color of each quater should be rendered as the number noted by the text inside (or close to).

Comment 5

[Michal Soltys]

Just wanted to confirm that this bug is indeed present and somewhat frustrating - especially in streaming case, where one has to use limited range for the sake of those people that watch on firefox (chrome and edge work fine).

Comment 6

[u440107]

Can still reproduce on latest nightly as of 2020-01-30. This is annoying because quite a few twitch streams use full range video.

Comment 8

[Thomas Daede [:TD-Linux]]

Bug https://bugzilla.mozilla.org/show_bug.cgi?id=1466408 has some additional video test cases.

Comment 9

[ennsbq]

So after all these years this is not resolved yet? As already mentioned, some of the streams are literally unwatchable because of this.

Comment 10

[blubbsate.ju]

Created an account to say that this is still not resolved after over 2 years. Some twitch streams are unwatchable wtf is this shit I'm switching to chrome.

Comment 11

[blubbsate.ju]

(In reply to blubbsate.ju from comment #10)

Created an account to say that this is still not resolved after over 2 years. Some twitch streams are unwatchable wtf is this shit I'm switching to chrome.

Example: https://i.imgur.com/EHyButE.jpg

Comment 12

[ennsbq]

Just updated to FF version 80. Problem still present.

Comment 13

[guillaumechabot1000]

The Severity should be raised from S3 (Normal) to something more important. Consider also raising the priority. This issue makes some twitch streamers with 1.4M followers unwatchable, making the users claim it'll probably never get fixed, that Firefox is a "Dogshit browser". The streamer's proposed solution is to "stop using firefox in 2020", which is not a good look.

Here's an example of unwatchable content from 20 hours ago: https://clips.twitch.tv/AnimatedBelovedPigeonPRChase

Comment 14

[fireattack [:fireattack]]

If anything, I've tried my best to raise awareness of this issue at multiple social media (Reddit, Hacker News, etc.).
I knew A/V stuff is pretty hard to get right, but still hope this can be solved ASAP considering it's severity in this streaming era.

Comment 15

[darktori]

I would like to propose raising the severity, as currently it's almost impossible to watch streaming video content (like on twitch.tv) depending on the content - if the game being streamed is in a dark setting, I mostly see a black window.

Comment 16

[ennsbq]

I CAN'T WATCH FORSENS MINECRAFT STREAMS FFS ITS TOO DARK!!!!!! PLS SOMEONE FIX THIS SHIT ITS BEEN 3 YEARS WTF IS TAKING SO LONG XD

Comment 17

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

This is being investigated. I appreciate it is frustrating to run into this issue, and comments that provide information on cases where this occurs and where it can be tested are useful. Aggressive comments and/or demands are not going to increase peoples motivation, and are not welcome. Please refer to the etiquette guidelines if you're uncertain.

---

On the dev side this seems like the check needs to be per codec or container. In some cases the codec string will contain info on if the colours are full range, but codec strings are going to be less reliable than info in the codec or container.

The h264 case sounds like the most pressing case. Annex E of the h264 spec discusses the 'Video usability information' structure, which includes the video_full_range_flag flag. This flag seems like the governing info for if we consider the colour space we're dealing with is full range or not. Annex E defines the specific values for use with different matrix coefficients/colour spaces.

We appear to have handling for parsing this information already[0], this is used to populate metadata elsewhere[1]. It seems like there's gfx support for 8bit colour (i.e. not HDR) with full range. However, there appears to be a missing link in the chain somewhere. I'll see if I can figure out where that is.

Holding NI while I investigate further.

[0] https://searchfox.org/mozilla-central/rev/f82d5c549f046cb64ce5602bfd894b7ae807c8f8/dom/media/platforms/agnostic/bytestreams/H264.cpp#870
[1] https://searchfox.org/mozilla-central/rev/f82d5c549f046cb64ce5602bfd894b7ae807c8f8/dom/media/platforms/wrappers/MediaChangeMonitor.cpp#134-136

Comment 18

[Thomas Andre]

To be honest videos, in chrome, and Firefox looks kind of the same (I still see differences in color unless I set force color-profile) to me when looking at forsens stream on twitch, it could be possible that the reason why chrome looks different is because they are doing color management on videos?

Comment 19

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Looking at the Windows case and tracking our setting of full range flags.

• With HW decoding my WMFMediaDataDecoder is returning media data wrapping a D3D11ShareHandleImage that has the full range flag set.
• By the time the data has reached MediaFormatReader::NotifyNewOutput the VideoData contains a GPUVideoImage which doesn't appear to store full range data. It looks like the imagine instead stores reference to structures related to a texture and those should persist the full range flag (I'm not familiar with the gfx code, but it looks like the TextureHost (and subclasses) is at the end of the chain that is responsible for tracking this).
• Texture hosts can report the range via GetColorRange. It looks like that information is used in CreateTexturedEffect[0].
  • I've tried breakpointing around CreateTexturedEffect, but am not having any luck hitting these breakpoints.

Matt, do you have any suggestions for tracing this? Is my expectation that TextureHost should be tracking that flag correct? Is it CreateTexturedEffect that should be consuming that info?

[0] https://searchfox.org/mozilla-central/rev/f82d5c549f046cb64ce5602bfd894b7ae807c8f8/gfx/layers/Effects.h#239

Comment 20

[Matt Woodrow (:mattwoodrow)]

Are you using WebRender? CreateTexturedEffect is only used for non-WR.

This looks to be mostly bug 1568745, where the various backends don't actually use the colour range information yet.

Comment 21

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

(In reply to Matt Woodrow (:mattwoodrow) from comment #20)

Are you using WebRender? CreateTexturedEffect is only used for non-WR.

This looks to be mostly bug 1568745, where the various backends don't actually use the colour range information yet.

I was primarily testing with WR, so that would make sense. I did a little testing with WR disabled and wasn't hitting my breakpoints either. Since WR is going to be the more utilized scenario going forward, I'll look into what needs to happen there (if you've got an idea on what's needed I'd appreciate an outline).

Comment 22

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Clearing NI. As comment 20 indicates, this is blocked on (or will be resolved by) bug 1568745.

Comment 24

[Niemeyer]

Are there any hopes this is going to be fixed? Some Twitch streams are unwatchable with Firefox.

Comment 25

[maciej.dariusz.gorczyca]

This should really be high priority. It feels like a crucial part of browser isn't working correctly...

Comment 26

[fireattack [:fireattack]]

I personally no longer use FIrefox as my main browser myself due to this bug. But for what it's worth, I've tried my best to raise awareness of this bug in various communities, such as Reddit, Hacker News, etc.

Hope it can get resolved sooner than later, anyway!

Comment 27

[danceflooR]

(In reply to fireattack from comment #4)

Attached a full range sample to demonstrate the problem better.

The color of each quater should be rendered as the number noted by the text
inside (or close to).

This alone should make VERY clear that there is a serious bug in the ff video player, just open the same video in chrome and see it for yourself.
I knew this was big but I didn't know it was this big, HOLY.
Really hope this gets fixed.

Comment 28

[maciej.dariusz.gorczyca]

I posted this pic comparison in a different bug report:

https://bugzilla.mozilla.org/show_bug.cgi?id=1688284

Comment 29

[Kacper Michajłow [:kasper93]]

(In reply to fireattack from comment #26)

I personally no longer use FIrefox as my main browser myself due to this bug.

Yes, bug like this one may seem insignificant, but in reality millions of people watch live streams everyday. Of course there is very small number of affected streams as most are in limited range, but still this produce mind share among users that Firefox is unusable and other browsers "just works". It is hard to recommend browser that have easily visible flaws, and frankly streaming community is huge among young ppl. This then produces banners like "works best in C" or "Firefox is not supported"... than again I guess everyone knows that given how small Firefox market share is currently.

Sorry for spam, as this message doesn't bring anything important, but message from fireattack really gave me some thoughts.

Comment 30

[Kelsey Gilbert [:jgilbert]]

This is low-hanging fruit to fix, and it affects a ton of users. Let's handle it

Comment 31

[Kelsey Gilbert [:jgilbert]]

I have WR setup to handle full-range properly now, but WR's only being fed narrow-tagged frames.
It looks like while MediaChangeMonitor correctly sees the frame as full-range, but by the time we get to WMFVideoMFTManager::CreateD3DVideoFrame (via WMFMediaDataDecoder::ProcessDecode), MediaRawData has lost its mTrackInfo data, which ends up causing it to be treated with default space/range. (709narrow)

I beliiiieve that this is dropped when serializing MediaRawData across IPC, in particular in ArrayOfRemoteMediaRawData::Fill. The mExtraData does seem to make it across IPC, and mExtraData is actually what MediaChangeMonitor had used to identify full-range. Does WMFVideoMFTManager need to re-do the extra-data-decoding, or do we need to serialize (some of?) mTrackInfo across the wire?

Comment 32

[Kelsey Gilbert [:jgilbert]]

Well, rather by WMFVideoMFTManager::CreateD3DVideoFrame, MediaRawData has lost its mTrackInfo data, which leaves us unable to pick up the correct narrow/full tagging.

Comment 33

[Kelsey Gilbert [:jgilbert]]

WR does handle narrow/full in at least some cases, like av1-webm looks ok before my changes, so maybe I just didn't trust it enough!

Comment 34

[Kelsey Gilbert [:jgilbert]]

My testcase was wrong, it does not.

I now have a av1-webm 709full that repros the issue of this bug in Nightly, and is better with my patches to WR. I think that's a good place to start, and then we'll tackle the h264 decoding issue separately.

Comment 35

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

(In reply to Jeff Gilbert [:jgilbert] from comment #31)

<snip>
Does WMFVideoMFTManager need to re-do the extra-data-decoding, or do we need to serialize (some of?) mTrackInfo across the wire?

We have some serialization traits for audio/video track info, so my expectation is that it would be sent across the IPC already, I'll investigate it not being sent (it may take me a little while clear backlog before I do).

Because different decoders may handle inputs differently, I don't know off the top of my head how much we need to feed the WMF one to get it to behave. However, if we're already figuring out some data earlier in the pipeline and failing to propogate it, it seems like we should be plumbing that down rather than re-decoding it.

It sounds like you've got this working for some cases and we can break h264 case out and look at that further?

Holding NI to remind me to investigate plumbing problems when I have time.

Comment 36

[Kelsey Gilbert [:jgilbert]]

(In reply to fireattack from comment #4)

Attached a full range sample to demonstrate the problem better.

The color of each quater should be rendered as the number noted by the text
inside (or close to).

What's the license for this? I would love to use it in our testing.

Comment 37

[Kelsey Gilbert [:jgilbert]]

(In reply to Bryce Seager van Dyk (:bryce) from comment #35)

(In reply to Jeff Gilbert [:jgilbert] from comment #31)

<snip>
Does WMFVideoMFTManager need to re-do the extra-data-decoding, or do we need to serialize (some of?) mTrackInfo across the wire?

We have some serialization traits for audio/video track info, so my expectation is that it would be sent across the IPC already, I'll investigate it not being sent (it may take me a little while clear backlog before I do).

Because different decoders may handle inputs differently, I don't know off the top of my head how much we need to feed the WMF one to get it to behave. However, if we're already figuring out some data earlier in the pipeline and failing to propogate it, it seems like we should be plumbing that down rather than re-decoding it.

It sounds like you've got this working for some cases and we can break h264 case out and look at that further?

Holding NI to remind me to investigate plumbing problems when I have time.

Yeah, sounds good. I have an av1-webm testcase that works with full-range now, so I'd like to take this bug for that, and I'll file a follow-up for h264-mp4 testcase, which still fails. (I think it's just the info propagation problem)

Comment 38

[fireattack [:fireattack]]

(In reply to Jeff Gilbert [:jgilbert] from comment #36)

(In reply to fireattack from comment #4)

Attached a full range sample to demonstrate the problem better.

The color of each quater should be rendered as the number noted by the text
inside (or close to).

What's the license for this? I would love to use it in our testing.

I generated this video myself.
I release it to the Public Domain if that helps.

Comment 39

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1459526 - Handle full-range video in Webrender.

• Begin to add video tests to ensure we ratchet towards correctness.

Comment 40

[Kelsey Gilbert [:jgilbert]]

Attachment: full-range av1-webm (cq_16_127_235.720p.png.bt709.bt709.pc.yuv420p.av1.webm)

Comment 41

[Kelsey Gilbert [:jgilbert]]

Attachment: cq_16_127_235.720p.png

Comment 42

[Jason]

Appreciate you guys working on this. Been wondering about this for awhile and haven’t had any luck searching online.

Comment 43

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1459526 - Handle full-range video in Webrender.

• Begin to add video tests to ensure we ratchet towards correctness.

Comment 44

[fireattack [:fireattack]]

Well, Firefox can't play YUV422 or YUV444 videos either, no matter full range or limited range. Chrome can.

Should we add tickets for them too?

Comment 45

[fireattack [:fireattack]]

Attachment: YUV422_709.mp4

Comment 46

[fireattack [:fireattack]]

Attachment: YUV444.mp4

Comment 47

[Martin Stránský [:stransky] (ni? me)]

YUV444 was explicitly disabled on Linux, see Bug 1368063.
I see the YUV422 clip played correctly on Firefox/Linux by system installed ffmpeg.

Comment 48

[fireattack [:fireattack]]

I'm on Windows.

YUV444 one errors out, YUV422 one "plays" but doesn't show anything.

Comment 49

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

(In reply to fireattack from comment #44)

Well, Firefox can't play YUV422 or YUV444 videos either, no matter full range or limited range. Chrome can.

Should we add tickets for them too?

For Windows and MacOS we rely on decoders provided by the operating system that do not provide support (to the best of my knowledge) for non-420 subsampling configurations. Bug 1368063 tracks the 444 case. I don't know if we have something to track 422. However, my understanding is that until those platforms provide support, it's not something we can address as licensing/patent issues prevent us providing our own decoders. So we can track it, but cannot fix the issue, in the sense we won't be able to play the media.

---

Clearing NI as I believe I've answered the query from comment 31 in bug 1709945.

Comment 50

[Kelsey Gilbert [:jgilbert]]

(In reply to fireattack from comment #44)

Well, Firefox can't play YUV422 or YUV444 videos either, no matter full range or limited range. Chrome can.

Should we add tickets for them too?

New formats, new tickets, please!

Comment 51

[Kelsey Gilbert [:jgilbert]]

• OSX h264s display 127,127,127 as 138,138,138
  • Same as Chrome. Safari refuses to play the video.
• Android is in generally rough shape
  • vp9 and h264
    • render as black for yuv8
    • timeout for yuv10
  • av1 renders as green for yuv10

Comment 52

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/318f32313091
Handle full-range video in Webrender. r=gw,lsalzman

Comment 53

[Atila Butkovits]

Backed out for causing wrench bustages.

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

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=c8wjot_7SCKkbJv8tOrBxw.0&revision=318f32313091bb3e184a8102aab8aaed671b56a7&searchStr=os%2Cx%2C10.14%2Cwebrender%2Copt%2Cwebrender%2Cstandalone%2Cwebrender-macos-release%2Cwrench

Failure log: https://treeherder.mozilla.org/logviewer?job_id=343026655&repo=autoland&lineNumber=462
https://treeherder.mozilla.org/logviewer?job_id=343026430&repo=autoland&lineNumber=37464

Comment 54

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f7016db57a2e
Handle full-range video in Webrender. r=gw,lsalzman

Comment 55

[Alexandru Michis [:malexandru]]

Backed out changeset f7016db57a2e (Bug 1459526) for causing reftest failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/e0f78a14ea6d1b1646a265c7961e2a6d8dabc512
Push with failures, failure log.

Comment 56

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/10a229d128c0
Handle full-range video in Webrender. r=gw,lsalzman

Comment 57

[Iulian Moraru]

Backed out for causing reftest failures on short.mp4.lastframe.html.

Push with failures

Failure log

Backout link

Comment 58

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab0835f9c267
Handle full-range video in Webrender. r=gw,lsalzman

Comment 59

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/ab0835f9c267

Comment 60

[Kacper Michajłow [:kasper93]]

Original sample from this bug attachment 8984703 [details],samples from Bug 1466408 and twitch clip if first comment still doesn't work correctly. Looks like you are missing yuvj420p case.

Comment 61

[fireattack [:fireattack]]

Can confirm they're still broken on my end too, Windows + software decoding.

Comment 62

[Kelsey Gilbert [:jgilbert]]

(In reply to Kacper Michajłow [:kasper93] from comment #60)

Original sample from this bug attachment 8984703 [details],samples from Bug 1466408 and twitch clip if first comment still doesn't work correctly. Looks like you are missing yuvj420p case.

This is bug 1709945.

(In reply to fireattack from comment #61)

Can confirm they're still broken on my end too, Windows + software decoding.

This is bug 1716093.

Comment 64

[Ciprian Georgiu, Desktop QA]

Hi fireattack! Are you still experience this issue that you originally reported in comment 0. Can you please help us verify this issue on 91 RC2?

I've tried to reproduce the issue, but I cannot seem to observe any difference between an affected Firefox build (e.g. Release 84) and Chrome.

Comment 65

[fireattack [:fireattack]]

Hi,

Most of use cases, including the two I posted in this ticket, actually require the patch for bug 1709945 to land to have it fixed.

So, no, 91 RC2 did NOT fix it (tested). The newest Nightly did fixed it.

https://i.imgur.com/jZJwIrc.png

Comment 66

[Ciprian Georgiu, Desktop QA]

Thanks for your quick testing! I don't think there's anything left for QA to verify here, given the fact that bug 1709945 has the wontfix status for 91.

I'll remove the qe+ flag.