Closed Bug 1459526 Opened 3 years ago Closed 3 months ago

Firefox doesn't play full color range video correctly

Categories

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

defect

Tracking

()

RESOLVED FIXED
91 Branch
Tracking Status
firefox-esr68 --- wontfix
firefox-esr78 --- wontfix
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- wontfix
firefox86 --- wontfix
firefox87 --- wontfix
firefox88 --- wontfix
firefox91 --- fixed

People

(Reporter: human.peng, Assigned: jgilbert)

References

(Blocks 2 open bugs, Regressed 1 open bug)

Details

(Keywords: correctness, parity-chrome)

Attachments

(8 files, 1 obsolete file)

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.
Attached file 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').
I'm assuming that this is not a recent regression. Please let us know if this was working in a Firefox version before.
Rank: 25
Priority: -- → P3
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).
Attached video 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).

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).

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

Duplicate of this bug: 1466408

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

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

OS: Unspecified → All
Hardware: Unspecified → All

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.

(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

Just updated to FF version 80. Problem still present.

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

Flags: needinfo?(bvandyk)

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.

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.

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

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?

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

Flags: needinfo?(matt.woodrow)

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.

Flags: needinfo?(matt.woodrow)

(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).

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

Depends on: 1568745
Flags: needinfo?(bvandyk)
Duplicate of this bug: 1688284

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

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

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!

(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.

I posted this pic comparison in a different bug report:

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

(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.

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

Severity: normal → S3
Priority: P3 → P1
Assignee: nobody → bvandyk

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?

Assignee: bvandyk → jgilbert
Flags: needinfo?(bvandyk)

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

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!

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.

(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.

(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.

Flags: needinfo?(human.peng)

(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)

(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.

Flags: needinfo?(human.peng)
  • Begin to add video tests to ensure we ratchet towards correctness.
Attached image cq_16_127_235.720p.png
Blocks: 1709945

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

  • Begin to add video tests to ensure we ratchet towards correctness.
See Also: → 1711812

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?

Attached video YUV422_709.mp4
Attached video YUV444.mp4

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

I'm on Windows.

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

(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.

Flags: needinfo?(bvandyk)
Attachment #9220676 - Attachment is obsolete: true
See Also: → 1715085

(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!

  • 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
Blocks: 1716093
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/318f32313091
Handle full-range video in Webrender. r=gw,lsalzman
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f7016db57a2e
Handle full-range video in Webrender. r=gw,lsalzman
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/10a229d128c0
Handle full-range video in Webrender. r=gw,lsalzman

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

Push with failures

Failure log

Backout link

Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ab0835f9c267
Handle full-range video in Webrender. r=gw,lsalzman
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch
Regressions: 1718403
Regressions: 1717078

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.

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

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

(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.

Status: REOPENED → RESOLVED
Closed: 3 months ago3 months ago
Flags: needinfo?(jgilbert)
Resolution: --- → FIXED
Duplicate of this bug: 1568745
Flags: qe-verify+

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.

Flags: needinfo?(human.peng)

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

Flags: needinfo?(human.peng)

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.

Flags: qe-verify+
Regressions: 1726186
You need to log in before you can comment on or make changes to this bug.