Closed Bug 1849680 Opened 6 months ago Closed 5 months ago

YouTube Video Payback does not use video overlay in normal playing mode

Categories

(Core :: Graphics: WebRender, defect)

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
120 Branch
Tracking Status
firefox120 --- fixed

People

(Reporter: sotaro, Assigned: gw)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

(Keywords: perf-alert)

Attachments

(3 files)

I noticed it during looking into Bug 1846165. It seems like recent regression.

can_promote_to_surface() was failed at if prim_clip_chain.needs_mask {.

Blocks: 1846165

:gw, do you have any idea about the regression?

Flags: needinfo?(gwatson)
Summary: YouTube Video Payback does not use video overlays other than full screen → YouTube Video Payback does not use video overlay other than full screen
Summary: YouTube Video Payback does not use video overlay other than full screen → YouTube Video Payback does not use video overlay except in full screen

I have tried on a Linux machine with various videos and resolutions, and they all seem to be getting promoted correctly to a compositor surface, so I couldn't repro. Perhaps it depends on platform, or theme, or whether signed in, or something like that?

Flags: needinfo?(gwatson) → needinfo?(sotaro.ikeda.g)
Flags: needinfo?(sotaro.ikeda.g)
OS: Unspecified → Windows
Summary: YouTube Video Payback does not use video overlay except in full screen → YouTube Video Payback does not use video overlay except in full screen on Windows

When I tested today, video rendering used overlay. Hmm.
Build with yesterday m-c caused the problem. Build with today m-c did not cause the problem. change to m-c seemed to addresses the problem.

https://www.youtube.com/watch?v=zCLOJ9j1k2Y

Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → WORKSFORME

Hmm, gecko still seems to have a problem of using overlay.

Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---

:gw, can you check again? When I tried the following URL, Comment 1 happened on Linux with latest m-c local build.
https://www.youtube.com/watch?v=zCLOJ9j1k2Y

It seemed that there were cases that can_promote_to_surface() succeeded or failed. I am not sure yet what causes the difference.
With latest m-c, the problem did not happen on Windows for me today. But the problem happened with latest m-c on Linux.

Flags: needinfo?(gwatson)
Summary: YouTube Video Payback does not use video overlay except in full screen on Windows → YouTube Video Payback does not use video overlay in normal playing mode

I tested that video with current m-c on Linux, and it still promotes it to a compositor surface for me locally. Could you add some logs to your repro with the flags passed to can_promote_to_surface and also the image keys / image format (rgb/yuv)? Perhaps that will help us understand what is going on?

Flags: needinfo?(gwatson)
Attached patch patch - add logSplinter Review

With Attachment 9350241 [details] [diff], I got the following log out on my Linux PC.
:gw, do you have any idea about what is going on?


PrimitiveInstanceKind::YuvImage 1 handle Handle { index: 1, epoch: Epoch(584), _marker: PhantomData<webrender::prim_store::image::YuvImage> } flags IS_BACKFACE_VISIBLE | PREFER_COMPOSITOR_SURFACE prim_clip_chainClipChainInstance { clips_range: ClipNodeRange { first: 9, count: 1 }, local_clip_rect: Box2D((0.0, 0.0), (1280.0, 720.0)), has_non_local_clips: true, needs_mask: true, pic_coverage_rect: Box2D((135.0, 80.0), (1415.0, 800.0)), pic_spatial_node_index: SpatialNodeIndex(8) }
can_promote_to_surface()_X2
PrimitiveInstanceKind::YuvImage 2

Flags: needinfo?(gwatson)

It seemed that when the problem happened, corners of video area were rounded.

With the following URL, there were cases that corners of video were rounded or not rounded.
https://www.youtube.com/watch?v=zCLOJ9j1k2Y

I am not sure yet what causes the difference between rounded corners and rectangle area. Today, on my Win11PC, latest nightly showed rectangle video area, and local build showed rounded corners area.

It should be possible to use overlay even when corners of video were rounded.

https://www.theverge.com/2023/8/22/23841737/youtube-rounded-corners-desktop
https://9to5google.com/2023/08/22/youtube-video-player-rounded-corners/

Looks like rounded corners of videos in default viewing mode on youtube will become the default on youtube. So fixing this bug will probably become a higher-than-normal priority task.

Seems like something we'll need to add support for to WR's compositing interfaces soon then, given that they are starting to roll this out.

Flags: needinfo?(gwatson)

Sotaro, do you know what method chrome is using to support rounded corners with video overlays?

Flags: needinfo?(sotaro.ikeda.g)

Looks like it's support on CALayer too, which I think will provide what we want on macos:

https://developer.apple.com/documentation/quartzcore/calayer/1410818-cornerradius

(In reply to Glenn Watson [:gw] from comment #15)

Sotaro, do you know what method chrome is using to support rounded corners with video overlays?

It seems that chromium does not use IDCompositionRectangleClip for videos with rounded corners. It seemed to use underlays

Flags: needinfo?(sotaro.ikeda.g)
Duplicate of this bug: 1850386

Hello, sorry for the dupe bug. I should have looked more thoroughly for similar bugs.

After testing a few other websites, I've found that Reddit, Instagram and Twitter/X also show similar behavior whenever the video frame is rounded, so this isn't isolated to just YouTube's new UI.

Here are Twitter and Instagram links to a video that was posted on both websites, to keep things consistent for testing:

  1. Twitter/X: https://twitter.com/instagram/status/1695100074819305902 (Overlay not active unless the video is played in fullscreen, also note that the video doesn't fit the rounded frame yet is still affected somehow)
  2. Instagram: https://www.instagram.com/reels/CwX31XMMYAV/ (Rounded corners) and https://www.instagram.com/reel/CwX31XMMYAV/ (Squared corners)

Unfortunately you will need to be logged in to open the Instagram link that has the rounded corners.

For Reddit, I've found this issue to only occur for videos that cover the entire rounded frame of the video, or at least if the video reaches really close to the rounded edges of the frame. Video overlay seems to engage even with rounded corners, but only if the video has enough "space" around it to not be affected by the rounded edges. Somehow this isn't the case with Twitter, where just having the rounded frame is enough for the video overlay to stop working, even if the video itself is squared.

Here are two Reddit links that show this phenomenon, open these in private windows without logging in if you don't see rounded corners:

  1. https://www.reddit.com/r/aww/comments/v7x39u/man_stops_to_rescue_kitten_gets_ambushed_by/ (Overlay activates even with rounded corners, and the video does not fit the entire frame)
  2. https://www.reddit.com/r/aww/comments/buzdy7/every_year_a_bobcat_mama_gives_birth_to_a_litter/ (Overlay doesn't work at all unless in fullscreen mode, and the video fits the entire frame)

After testing videos with different aspect ratios in YouTube, I found out that YouTube behaves similarly to Reddit, video overlay can activate even with YouTube's new rounded corners UI, but only for videos that don't fully cover the entire video frame. Example video: https://youtu.be/zKSsP2084nU?si=HNgyQdxStfYALlxv.

As that video's aspect ratio is 1:1, it doesn't cover the entire frame and hence the overlay works here. You can actually make the video "fit" the rounded frame by zooming out a few notches using Ctrl +/-, and the overlay stops working immediately. Conversely, try zooming in if the video fits the frame for you due to different scaling factors depending on your display's DPI.

I can't really guess why Twitter is the odd one out here, but I still hope this helps to understand the issue better. Understanding how chromium handles this will definitely be helpful, as Glenn and Sotaro are already doing.

I just stumbled across this bug in Firefox 117.0 on Linux. On systems with a weak GPU this bug makes even 480p30fps video basically unwatchable so this is pretty severe.

If you want a reliable/minimal reproducer then you can use this HTML with any MP4 video.

<div style="overflow: hidden; border-top-left-radius: 12px">
<video controls><source src="whatever.mp4" type="video/mp4"></video>
</div>

(In reply to David Turner from comment #21)

I just stumbled across this bug in Firefox 117.0 on Linux. On systems with a weak GPU this bug makes even 480p30fps video basically unwatchable so this is pretty severe.

If you want a reliable/minimal reproducer then you can use this HTML with any MP4 video.

<div style="overflow: hidden; border-top-left-radius: 12px">
<video controls><source src="whatever.mp4" type="video/mp4"></video>
</div>

This shouldn't be the case. Can you file a separate bug for what you're seeing including the gfx section of about:support?

Flags: needinfo?(david.turner)

I have created Bug 1850754 with my specific issue and about:support

Flags: needinfo?(david.turner)

Adding Sotaro's power-consumption metrics for later reference.

  • Win11PC1

    • Firefox
      • square: 22W-24W
      • rounded: 25W-27W
    • Edge
      • square: 21W-23W
      • rounded: 21W-23W
    • Chrome
      • square: 31W-36W
      • rounded: 31W-36W
  • Win11PC2

    • Firefox
      • square:23W-24W
      • rounded:25W-27W
    • Edge
      • square: 21W-22W
      • rounded: 21W-22W
    • Chrome
      • square: 19W-21W
      • rounded: 19W-21W

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

For more information, please visit BugBot documentation.

Flags: needinfo?(gwatson)
Severity: -- → S3
Flags: needinfo?(gwatson)
Blocks: 1850118
Assignee: nobody → gwatson

Sheriffs, I will try to land this today, but have limited availability over the next few days to keep an eye out for regressions. If you see any regressions or bug reports related to it, please feel free to back out immediately (as it's likely any regressions will affect a lot of video related pages).

Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/14877179e1a7
Add support for masks on opaque compositor surfaces r=gfx-reviewers,lsalzman

test_video_low_power_telemetry.html is my test so I'll guess about what's happening here. The failure in that test is noting that, in fullscreen, there is some layer on top of the video layer, which will (likely) prevent the window from entering its low-power mode. Since YouTube isn't rounding corners in fullscreen, I assume this is either:

  1. an artifact of the opaque cutout being in a lower layer stacking
  2. perhaps the rounded corner overlays remain as a fully transparent layer on top of the video

Either way, we can probably fix it / account for it in the macOS layer code. Glenn, feel free to disable this test to land your fix and I'll work through the fixups for the macOS layers in a new Bug.

No longer blocks: 1846165
See Also: → 1846165

I've disabled that test in the patch and will re-land shortly.

Flags: needinfo?(gwatson)
Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4333e71e3754
Add support for masks on opaque compositor surfaces r=gfx-reviewers,lsalzman

Friendly ni so we remember to file the mentioned follow-up bug.

Flags: needinfo?(bwerth)

I think those failures are likely to be a temporary unrelated infrastructure failure. I added android try jobs to my try run with that patch and they passed OK (https://treeherder.mozilla.org/jobs?repo=try&revision=fd54542675db1b9d91e6f854542224c83a43230c). Would you be able to try re-land it?

Flags: needinfo?(gwatson) → needinfo?(nerli)
Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c369e42eb572
Add support for masks on opaque compositor surfaces r=gfx-reviewers,lsalzman

Relanded as requested in previous comment.

Flags: needinfo?(nerli)
Status: REOPENED → RESOLVED
Closed: 6 months ago5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 119 Branch
Regressions: 1853691

(In reply to Timothy Nikkel (:tnikkel) from comment #33)

Friendly ni so we remember to file the mentioned follow-up bug.

Bug 1850118, already blocked by this Bug, will be the Bug for fixing up the macOS layer presentation. I'll also use it to turn back on the test.

Flags: needinfo?(bwerth)

Backed out changeset c369e42eb572 (bug 1849680) for causing Bug 1853691.

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/0851ebe92581

Status: RESOLVED → REOPENED
Flags: needinfo?(gwatson)
Resolution: FIXED → ---
Target Milestone: 119 Branch → ---
Flags: needinfo?(gwatson)

Updated the patch with a fix and new test for what was causing this to be broken on youtube when ambient mode is enabled. I'll re-land after the soft freeze, and then we can consider an uplift to beta in a week or so.

See Also: → 1854920
Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0cca5d3b5a43
Add support for masks on opaque compositor surfaces r=gfx-reviewers,lsalzman
Flags: needinfo?(gwatson)

Android failure looks like some kind of infra issue, I adjusted the fuzziness on the new test for the win11 failure.

Flags: needinfo?(gwatson)
Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d3828ca7337d
Add support for masks on opaque compositor surfaces r=gfx-reviewers,lsalzman
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/243096f6ead7
Backed out changeset d3828ca7337d for causing android bustages on underlay.yaml.
https://hg.mozilla.org/integration/autoland/rev/93cb4f911941
Add support for masks on opaque compositor surfaces. r=gfx-reviewers,lsalzman
Status: REOPENED → RESOLVED
Closed: 5 months ago5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 120 Branch
Blocks: 1855573
Duplicate of this bug: 1850118

(In reply to Brad Werth [:bradwerth] from comment #39)

Bug 1850118, already blocked by this Bug, will be the Bug for fixing up the macOS layer presentation. I'll also use it to turn back on the test.

Ended up filing a new issue for this, Bug 1855573.

Regressions: 1855721
Regressions: 1856656

(In reply to Pulsebot from comment #46)

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/243096f6ead7
Backed out changeset d3828ca7337d for causing android bustages on
underlay.yaml.
https://hg.mozilla.org/integration/autoland/rev/93cb4f911941
Add support for masks on opaque compositor surfaces. r=gfx-reviewers,lsalzman

== Change summary for alert #39770 (as of Wed, 04 Oct 2023 08:53:33 GMT) ==

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
3% startup_about_home_paint_cached windows10-64-shippable-qr e10s fission stylo webrender-sw 571.96 -> 587.92

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=39770

Keywords: perf-alert
See Also: → 1850754
No longer blocks: 1850118

(In reply to Pulsebot from comment #46)

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/243096f6ead7
Backed out changeset d3828ca7337d for causing android bustages on
underlay.yaml.
https://hg.mozilla.org/integration/autoland/rev/93cb4f911941
Add support for masks on opaque compositor surfaces. r=gfx-reviewers,lsalzman

== Change summary for alert #39744 (as of Mon, 02 Oct 2023 10:48:20 GMT) ==

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
9% perf_reftest_singletons style-attr-1.html linux1804-64-shippable-qr e10s fission stylo webrender 6.19 -> 6.77
5% glterrain linux1804-64-shippable-qr e10s fission stylo webrender-sw 2.58 -> 2.72
4% glterrain linux1804-64-shippable-qr e10s fission stylo webrender-sw 2.61 -> 2.72

Improvements:

Ratio Test Platform Options Absolute values (old vs new)
27% perf_reftest style-attr-1.html linux1804-64-shippable-qr e10s fission stylo webrender 4.92 -> 3.62
23% perf_reftest style-attr-1.html linux1804-64-shippable-qr e10s fission stylo webrender-sw 4.90 -> 3.78
20% perf_reftest_singletons style-attr-1.html linux1804-64-shippable-qr e10s fission stylo webrender 6.40 -> 5.14
9% perf_reftest_singletons tiny-traversal-singleton.html linux1804-64-shippable-qr e10s fission stylo webrender 1,076.03 -> 978.84
5% perf_reftest_singletons abspos-reflow-1.html linux1804-64-shippable-qr e10s fission stylo webrender 66.44 -> 62.94
3% tabswitch windows10-64-shippable-qr e10s fission stylo webrender-sw 8.47 -> 8.19

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=39744

Duplicate of this bug: 1850754
See Also: 1850754

(In reply to Pulsebot from comment #45)

Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d3828ca7337d
Add support for masks on opaque compositor surfaces r=gfx-reviewers,lsalzman

== Change summary for alert #39744 (as of Mon, 02 Oct 2023 10:48:20 GMT) ==

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
9% perf_reftest_singletons style-attr-1.html linux1804-64-shippable-qr e10s fission stylo webrender 6.19 -> 6.77
5% glterrain linux1804-64-shippable-qr e10s fission stylo webrender-sw 2.58 -> 2.72

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=39744

Comment on attachment 9353039 [details]
Bug 1849680 - Add support for masks on opaque compositor surfaces

Beta/Release Uplift Approval Request

  • User impact if declined: Significant power regression on youtube default player mode, which has enabled rounded corners on videos.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: Bug 1855721, Bug 1856656
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): We had a couple of regressions in nightly, fixed by the attached bugs, nothing else since then. Relatively low risk if it merges cleanly.
  • String changes made/needed:
  • Is Android affected?: Yes
Attachment #9353039 - Flags: approval-mozilla-beta?
Attachment #9350241 - Flags: approval-mozilla-beta?

Comment on attachment 9353039 [details]
Bug 1849680 - Add support for masks on opaque compositor surfaces

Switching uplift request to release. Fx119 is now in release candidate.
This patch will pend for consideration in a Fx119 dot release, after some time in Fx120 beta.

Attachment #9353039 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Attachment #9350241 - Flags: approval-mozilla-beta? → approval-mozilla-release?

Comment on attachment 9350241 [details] [diff] [review]
patch - add log

Rejecting release uplift request, looks safer to ride the train with Fx120

  • S3 bug
  • Not new in Fx119
  • Introduced regressions

Please reach out if there are any concerns/objections

Attachment #9350241 - Flags: approval-mozilla-release? → approval-mozilla-release-
Attachment #9353039 - Flags: approval-mozilla-release? → approval-mozilla-release-

(In reply to Donal Meehan [:dmeehan] from comment #56)

Comment on attachment 9350241 [details] [diff] [review]
patch - add log

Rejecting release uplift request, looks safer to ride the train with Fx120

  • S3 bug
  • Not new in Fx119
  • Introduced regressions

Please reach out if there are any concerns/objections

The duplicate Bug 1850118 has visible user impact for macOS YouTube HDR in windowed mode. If this doesn't make it to release, potentially a lot of users will experience that defect.

Flags: needinfo?(dmeehan)

Thanks :bradwerth for Comment 57.
Keeping my NI open, will assess closer to the planned dot release.
Will try aim to include this, setting the requests back to ?

Flags: needinfo?(dmeehan)
Attachment #9350241 - Flags: approval-mozilla-release- → approval-mozilla-release?
Attachment #9353039 - Flags: approval-mozilla-release- → approval-mozilla-release?
Regressions: 1863688

Comment on attachment 9350241 [details] [diff] [review]
patch - add log

Rejecting release uplift request.
Next week is RC week for Fx120, we don't have another Fx119 dot release planned before then.

Attachment #9350241 - Flags: approval-mozilla-release? → approval-mozilla-release-
Attachment #9353039 - Flags: approval-mozilla-release? → approval-mozilla-release-
Regressions: 1868026
Regressions: 1869994
See Also: → 1872892
Regressions: 1874700
See Also: → 1881569
You need to log in before you can comment on or make changes to this bug.