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

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Sotaro Ikeda [:sotaro]]

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

Comment 1

[Sotaro Ikeda [:sotaro]]

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

Comment 2

[Sotaro Ikeda [:sotaro]]

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

Comment 3

[Glenn Watson [:gw]]

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?

Comment 4

[Sotaro Ikeda [:sotaro]]

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

Comment 5

[Sotaro Ikeda [:sotaro]]

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

Comment 6

[Sotaro Ikeda [:sotaro]]

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

Comment 7

[Glenn Watson [:gw]]

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?

Comment 8

[Sotaro Ikeda [:sotaro]]

Attachment: patch - add log

Comment 9

[Sotaro Ikeda [:sotaro]]

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

Comment 10

[Sotaro Ikeda [:sotaro]]

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.

Comment 11

[Sotaro Ikeda [:sotaro]]

Attachment: screenshot when corners of video were rounded

Comment 12

[Sotaro Ikeda [:sotaro]]

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

Comment 13

[Mayank Bansal]

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.

Comment 14

[Glenn Watson [:gw]]

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.

Comment 15

[Glenn Watson [:gw]]

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

Comment 16

[Glenn Watson [:gw]]

DC has methods to support rounded rect clips:

https://learn.microsoft.com/en-us/windows/win32/api/dcomp/nn-dcomp-idcompositionrectangleclip

Comment 17

[Glenn Watson [:gw]]

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

Comment 18

[Sotaro Ikeda [:sotaro]]

(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

• https://bugs.chromium.org/p/chromium/issues/detail?id=1179421
• https://chromium-review.googlesource.com/c/chromium/src/+/2709385
• https://source.chromium.org/chromium/chromium/src/+/main:components/viz/service/display/overlay_strategy_underlay.h;l=17

Comment 20

[Animesh Bhattacharya [:AnimeshB]]

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.

Comment 21

[David Turner]

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>

Comment 22

[Jeff Muizelaar [:jrmuizel]]

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

Comment 23

[David Turner]

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

Comment 24

[Bob Hood [:bhood]]

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

Comment 25

[BugBot [:suhaib / :marco/ :calixte]]

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

For more information, please visit BugBot documentation.

Comment 26

[Glenn Watson [:gw]]

Attachment: Bug 1849680 - Add support for masks on opaque compositor surfaces

Comment 27

[Glenn Watson [:gw]]

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

Comment 28

[Pulsebot]

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

Comment 29

[Cosmin Sabou [:CosminS]]

Backed out for causing mda failures on test_video_low_power_telemetry.html.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=mda&revision=14877179e1a724e52ee7ec0488d45d1bff1d2e76&selectedTaskRun=Ps_wWe8wR9WzvAyO2LxZXA.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=429232027&repo=autoland

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

Comment 30

[Brad Werth [:bradwerth]]

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.

Comment 31

[Glenn Watson [:gw]]

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

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4333e71e3754
Add support for masks on opaque compositor surfaces r=gfx-reviewers,lsalzman

Comment 33

[Timothy Nikkel (:tnikkel)]

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

Comment 34

[Noemi Erli[:noemi_erli]]

Backed out changeset 4333e71e3754 (Bug 1849680) for causing wrench bustages CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=WVtg9CUvQrq-Qb-RwQFYPw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=4333e71e3754938bc5beee5baabbfd6657869bc6

Log: https://treeherder.mozilla.org/logviewer?job_id=429455736&repo=autoland&lineNumber=1033

Backout: https://hg.mozilla.org/integration/autoland/rev/12d4ba8ee11d86a3baa28d4a55ec7f346637077d

Comment 35

[Glenn Watson [:gw]]

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?

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c369e42eb572
Add support for masks on opaque compositor surfaces r=gfx-reviewers,lsalzman

Comment 37

[Noemi Erli[:noemi_erli]]

Relanded as requested in previous comment.

Comment 38

[Iulian Moraru]

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

Comment 39

[Brad Werth [:bradwerth]]

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

Comment 40

[Iulian Moraru]

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

• Push that was backed out

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

Comment 41

[Glenn Watson [:gw]]

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.

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0cca5d3b5a43
Add support for masks on opaque compositor surfaces r=gfx-reviewers,lsalzman

Comment 43

[Iulian Moraru]

Backed out for causing wrench build bustages.

• Push with failures

• Failure log for wrench builds on android

• Failure log for wrench build on windows

• Backout link

Comment 44

[Glenn Watson [:gw]]

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

Comment 45

[Pulsebot]

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

Comment 46

[Pulsebot]

Pushed by [e-mail]:
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

Comment 47

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/93cb4f911941

Comment 49

[Brad Werth [:bradwerth]]

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

Comment 50

[Alex Finder]

(In reply to Pulsebot from comment #46)

Pushed by [e-mail]:
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

Comment 51

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #46)

Pushed by [e-mail]:
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

Comment 53

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Pulsebot from comment #45)

Pushed by [e-mail]:
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 54

[Glenn Watson [:gw]]

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

Comment 55

[Donal Meehan [:dmeehan]]

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.

Comment 56

[Donal Meehan [:dmeehan]]

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

Comment 57

[Brad Werth [:bradwerth]]

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

Comment 58

[Donal Meehan [: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 ?

Comment 59

[Donal Meehan [:dmeehan]]

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.