invisible video decoding causes high CPU usage

RESOLVED FIXED in Firefox 65

Status

()

defect
P2
normal
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: alwu, Assigned: alwu)

Tracking

unspecified
mozilla66
Points:
---
Bug Flags:
webcompat ?
in-testsuite +

Firefox Tracking Flags

(firefox63 wontfix, firefox64 wontfix, firefox65 fixed, firefox66 fixed)

Details

(Whiteboard: [webcompat], URL)

Attachments

(3 attachments)

(Assignee)

Description

5 months ago
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
Flags: webcompat?
Whiteboard: [webcompat]
(Assignee)

Comment 1

5 months ago
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
Assignee: nobody → alwu
(Assignee)

Comment 2

5 months ago
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.
(Assignee)

Comment 3

5 months ago
Add new webidl method for testing only and a test.

Comment 4

4 months ago
Pushed by alwu@mozilla.com:
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

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3432e8bee7f1
https://hg.mozilla.org/mozilla-central/rev/4c9f874d6868
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
(Assignee)

Comment 7

4 months ago
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.
(Assignee)

Comment 8

4 months ago
Add testing function to know whether video is visible or not.
(Assignee)

Updated

4 months ago
Flags: needinfo?(alwu)

Comment 10

4 months ago
Pushed by alwu@mozilla.com:
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

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a5756686b400
https://hg.mozilla.org/mozilla-central/rev/93ad0a17cde4
https://hg.mozilla.org/mozilla-central/rev/b8a2ab65c479
Status: REOPENED → RESOLVED
Last Resolved: 4 months ago4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Looks like the actual patch here is pretty small and the issue sounds pretty severe. Is this something we should consider uplifting to Beta?
Flags: needinfo?(alwu)
Flags: in-testsuite+
(Assignee)

Comment 13

4 months ago
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
Flags: needinfo?(alwu)
Attachment #9029604 - Flags: approval-mozilla-beta?
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.
Attachment #9029604 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.