Closed Bug 1933055 Opened 9 months ago Closed 7 months ago

YouTube videos flashing in red or green areas

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Version:
Firefox 133
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
136 Branch
Tracking Flags:
Tracking Status
firefox136 --- fixed

People

(Reporter: bugzilla, Assigned: jnicol)

Details

Attachments

(1 file)

Bug 1933055 - Override colorspace conversion used to render video frames if demuxer disagrees with decoder. r?padenot!,#gfx-reviewers!
48 bytes, text/x-phabricator-request
Details | Review

User Agent: Mozilla/5.0 (Android 15; Mobile; rv:133.0) Gecko/133.0 Firefox/133.0
Firefox for Android

Steps to reproduce:

Start watching a video on YouTube with solid red or green areas.
Switch tasks and switch back again. (Sometimes I had to do this to reproduce, sometimes not.)
Continue watching video.

Actual results:

Red/green areas flash distractingly.
https://youtu.be/l5tdzYO3A_U?si=DUS7CkZrl_Q9zJuu
This is very similar to this bug : https://bugzilla.mozilla.org/show_bug.cgi?id=1845766

Expected results:

No flashing colours.

I am on a Google Pixel 7 Pro Android 15, but this was also an issue on Android 14.

A shorter clearer example: https://youtube.com/shorts/3MNVVALUFIE?si=WpKMlKfFV6Qa4RM3

The Bugbug bot thinks this bug should belong to the 'Fenix::Media' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Media
Product: Firefox → Fenix

I can reproduce it with the example in Comment 2, on a Pixel 7, Android 15, Fx 135.

This is the link to the original video: https://youtu.be/cEU67WLsUHE?feature=shared&t=267

I'll move this bug to Core / Audio/Video, as I think it belongs there.

Status: UNCONFIRMED → NEW
Component: Media → Audio/Video
Ever confirmed: true
Product: Fenix → Core

The severity field is not set for this bug.
:jimm, could you have a look please?

For more information, please visit BugBot documentation.

Flags: needinfo?(jmathies)

This looks gfx related but I could be wrong. The youtube viudeo shows what it looks like, note the read sweater area flicker.

Flags: needinfo?(jmathies) → needinfo?(jnicol)

This seems related to bug 1866020: it also affects a Pixel 7 (haven't tested 6 or 8 but assume they are affected too) from Android 14 onwards. Just like in bug 1866020, I cannot reproduce in Chrome by default, but when you swipe up into the app switcher the chrome preview is affected. This is because chrome composites video using SurfaceControl, which typically would use hardware composer, which is unaffected. However, when the app switcher is active surfaceflinger will use GLES to composite chrome's video. The bug is in the GLES driver. Firefox always uses GLES to composite the video so is always affected.

I can only seem to reproduce in 360p quality and lower. According to the logcat the difference in the video format is

480p (no flickering):

   CCodec  :   int32_t android._dataspace = 260
   CCodec  :   int32_t color-standard = 1

vs 360p (flickering):

   CCodec  :   int32_t android._dataspace = 259
   CCodec  :   int32_t color-standard = 4

color-standard == 1 is COLOR_STANDARD_BT709, and color-standard == 4 is COLOR_STANDARD_BT601_NTSC. I'm not sure whether BT601 is broken for all videos on this device, however, or whether there's something special about the reported videos.

Flags: needinfo?(jnicol)

Just for the record I can definitely reproduce this bug at 1080p on YouTube.

Hmm, that's interesting. I can definitely only reproduce in 360p or less. Can you reproduce in this video in 1080p? (I figured a football pitch has lots of green). I can repro in 360p or less, but not higher.

Could you capture a logcat when reproducing the flicker, and attach it to this bug? Could you also hit the settings gear icon in the youtube video, click the "stats for nerds" button, then take a screenshot?

Flags: needinfo?(bugzilla)

Two nights ago I definitely had a video set at 1080p and saw the flashing, but I haven't been able to reproduce it since. I can see how reliably reproduceable it is at 320p. I'll keep an eye out and get some info if I see it happen again.

Flags: needinfo?(bugzilla)

Few more bits of information: This affects the Pixel 6a on Android 14 and 15, but not Android 13. It affects the Pixel 7 on Android 13, 14, and 15. It doesn't affect the Pixel 8 at all. Nor does it affect an exynos samsung S24. I haven't tested other devices yet.

Looking at the video I posted a link to in comment 9. The "stats for nerds" says Codecs: av01.0.01M.08 (396) and Color: bt709 / bt709.

If I download the exact same video (id 396) to my desktop and run ffprobe -show-frames I get:

color_range=tv
color_space=bt709
color_primaries=bt709
color_transfer=bt709

So it seems conclusive that the video in question has a BT709 colorspace.

If play this video in firefox on youtube on an unaffected android device, the logcat only ever shows a color-standard of 1, ie kColorStandardBT709.

But on affected devices we get a flurry of output format changes alternating between 1 and 4, 4 being kColorStandardBT601_525. While this quickly settles, the flickering persists. My theory is that certain buffers have been incorrectly marked as having the incorrect colorspace, while some have the correct one. The means when rendering through the frames the driver alternates between which color conversion it uses, resulting in the flickering.

To verify this I used the EXT_YUV_target glsl extension to manually make us switch between bt709 and bt601 conversions, and it indeed looks very similar.

In order to fix this, we already have a mechanism to force the GPU to use bt709 conversion, from bug 1866020. So that part is trivial. The difficulty is deciding when to force this conversion. Presumably we wouldn't want to force all bt601 frames to use bt709 conversion, as they would look incorrect.

Paul, do you know whether we have any other mechanism of determining the color space of decoded frames other than relying on what the MediaCodec tells us in the onOutputFormatChanged() callback?

Flags: needinfo?(padenot)

We can inspect the compressed byte stream ourselves, if we feel that the video is correct, but somehow Android gets it wrong. Then it's probably possible to override as you say. Do we feel this is restricted to AV1 ?

For this, we would take the extradata for the AV1 video, and parse it. https://searchfox.org/mozilla-central/source/dom/media/platforms/android/RemoteDataDecoder.cpp#173, here we can get it in aConfig.mCodecSpecificConfig, this is a blob that you can get like so: https://searchfox.org/mozilla-central/source/dom/media/platforms/android/RemoteDataDecoder.cpp#601-602.

We can then parse it ourselves: we can reuse libdav1d's code (software decoder), here's how ffmpeg does it, as an example: https://searchfox.org/mozilla-central/source/media/ffvpx/libavcodec/libdav1d.c#175-212. This parses the sequence header, and that has the info we're looking for to confirm or infirm what MediaCodec has found, directly from the byte stream, here's the list of relevant attributes: https://searchfox.org/mozilla-central/source/third_party/dav1d/include/dav1d/headers.h#216-233.

It's probably trivially cheap to always run this libdav1d code on decoder init, and then we can compare / telemetry / whatever.

Flags: needinfo?(padenot) → needinfo?(jnicol)

Yes, this does seem restricted to AV1.

Paul helped me on matrix: this is in fact even easier to detect than he wrote above. The demuxer already provides us with the colorspace, which we can access from RemoteVideoDecoder::mConfig::mColorPrimaries. For the affected videos, this is always Some(gfx::ColorSpace2::BT709) which matches what we believe the video is in reality, even when the android decoder sometimes tells us the color space is kColorStandardBT601_525.

So to solve this we can simply set the forceBT709ColorSpace flag on the SurfaceTextureImage if the demuxer tells us ColorSpace2::BT709 and the android decoder tells us kColorStandardBT601_525, eg mConfig.mColorPrimaries == Some(gfx::ColorSpace2::BT709) && mColorSpace == Some(4)

Flags: needinfo?(jnicol)

On the Pixel 6 and 7 families of devices, some AV1 video frames are
misreported as having BT609 colorspace rather than BT701. This results
in a flickering effect during playback as the GPU alternates between
which conversion to use.

On affected devices, this patch makes us force the GPU to use BT701
conversion for all frames which the android decoder has reported as
BT609, as the long as the demuxer has also reported the stream as
containing BT701.

Assignee: nobody → jnicol
Status: NEW → ASSIGNED
Pushed by jnicol@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/46ab78a4007b Override colorspace conversion used to render video frames if demuxer disagrees with decoder. r=padenot,gfx-reviewers,nical

Backed out for causing multiple failures.

  • Push with failures - reftests failures
  • Failure Log
  • Failure line: REFTEST ERROR | TEST-UNEXPECTED-FAIL | dom/media/test/reftest/color_quads/720p.png.bt709.bt709.tv.yuv420p.vp9.webm == dom/media/test/reftest/color_quads/720p.png.bt709.bt709.tv.yuv420p.av1.webm | application timed out after 370 seconds with no output
Flags: needinfo?(jnicol)

Ah whoops. os.Build.SOC_MODEL is only available in SDK 31 onwards

Flags: needinfo?(jnicol)

This only affects Android 13 (SDK 33) onwards so I'll put the check for SOC_MODEL inside a check for SDK level >= 33

Pushed by jnicol@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2acddf862dab Override colorspace conversion used to render video frames if demuxer disagrees with decoder. r=padenot,gfx-reviewers,nical

https://hg.mozilla.org/mozilla-central/rev/2acddf862dab

Status: ASSIGNED → RESOLVED
Closed: 7 months ago
status-firefox136: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 136 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: