1511235 - invisible video decoding causes high CPU usage

Status: RESOLVED FIXED

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

Description

[Alastor Wu [:alwu]]

The issue is reported from [1], it seems that the suspend-video-decoding-on-background doesn't work, so we spend LOTS of CPU on video decoding for those invisible video.

[1] https://github.com/webcompat/web-bugs/issues/19389#issuecomment-442307004

Comment 1

[Alastor Wu [:alwu]]

In this website, there are 24 videos are playing at the same time. However, there is only one is visible.

After some digging, I found that only the one which user can see has been set the visibility state (APPROXIMATELY_VISIBLE), and other videos are all UNTRACKED.

The implementation of this site is to autoplay all 24 muted video at same time, and put them in one really wide panel which is wider than the viewport [1]. The panel itself is a div element "slideshow-inner". Every time user clicks "<" or >" on the panel, the site would move the panel to the left/right in order to show the next video.

The visibility check which is executed by presShell seems lazily. It won't update frame's visibility until frame is going to be showed soon. Every frame outside the visible range (which is larger than viewport) would be UNTRACKED.

Therefore, we should suspend video decoding for those video whose visibility state is UNTRACKED in order to reduce CPU usage.

[1] If you force to panel show all video, you can see there are some video which are outside of viewport
https://drive.google.com/file/d/1-KQWkV7h10ThxG7iRZXjH7mKLX-WiIoY/view?usp=sharing

Comment 2

[Alastor Wu [:alwu]]

Attachment: Bug 1511235 - part1 : suspend video decoding for video whose visibility state is UNTRACK.

If video has not been within the potential visible range (which is larger than viewport) yet, its visibility state won't
be updated and would stay in 'UNTRACK'. As those kinds of video are still invisible to users, we don't need to decode
any video frames, we can suspend their video decoding until they're going to be visible.

Comment 3

[Alastor Wu [:alwu]]

Attachment: Bug 1511235 - part2 : add test.

Add new webidl method for testing only and a test.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3432e8bee7f1
part1 : suspend video decoding for video whose visibility state is UNTRACK. r=jya
https://hg.mozilla.org/integration/autoland/rev/4c9f874d6868
part2 : add test. r=jya,baku

Comment 5

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/3432e8bee7f1
https://hg.mozilla.org/mozilla-central/rev/4c9f874d6868

Comment 6

[Cosmin Sabou [:CosminS]]

Backed out for causing bug 1513454 and 1513456.
Backout link: https://hg.mozilla.org/mozilla-central/rev/ef4f3c28e91ccdc6e563d5911a933692ffddffe4

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&group_state=expanded&revision=4c9f874d6868fda817959135626abfb864571aa5&selectedJob=216706456

Comment 7

[Alastor Wu [:alwu]]

In Linux x64 QuantumRender opt, I found that the sometome media element's visibility didn't be updated on time, it would be updated after media element has started playing. 

As we expect media should be visible before calling `video.setVisible()`, if not, it would causes the test fail because video decoding has been suspended so that we can't receive 'mozentervideosuspend' event.

Comment 8

[Alastor Wu [:alwu]]

Attachment: Bug 1511235 - part3 : ensure video is visible before starting test

Add testing function to know whether video is visible or not.

Comment 9

[Alastor Wu [:alwu]]

Try-result looks good
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c8349cadccec2a61dce295d62567d135f1d56a5

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5756686b400
part1 : suspend video decoding for video whose visibility state is UNTRACK. r=jya
https://hg.mozilla.org/integration/autoland/rev/93ad0a17cde4
part2 : add test. r=jya,baku
https://hg.mozilla.org/integration/autoland/rev/b8a2ab65c479
part3 : ensure video is visible before starting test r=jya,baku

Comment 11

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/a5756686b400
https://hg.mozilla.org/mozilla-central/rev/93ad0a17cde4
https://hg.mozilla.org/mozilla-central/rev/b8a2ab65c479

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Looks like the actual patch here is pretty small and the issue sounds pretty severe. Is this something we should consider uplifting to Beta?

Comment 13

[Alastor Wu [:alwu]]

Comment on attachment 9029604 [details]
Bug 1511235 - part1 : suspend video decoding for video whose visibility state is UNTRACK.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1346498

User impact if declined: User would have higher CPU usage when browsing sites which has invisible videos

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: none

List of other uplifts needed: none

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This patch is used to suspend video decoding when video is invisible, the change is trivial which only affects invisible videos, and we also have an automation test which makes a risk low.

String changes made/needed: none

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9029604 [details]
Bug 1511235 - part1 : suspend video decoding for video whose visibility state is UNTRACK.

[Triage Comment]
Fixes high CPU utilization on sites with invisible videos by making sure that decoding is suspended in those situations. Thanks for including a new test for this. Approved for 65.0b8.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2616da988292
https://hg.mozilla.org/releases/mozilla-beta/rev/9d3238bbfcc6
https://hg.mozilla.org/releases/mozilla-beta/rev/49db147e8a5c