Playing youtube videos cause 60fps paint rate

RESOLVED FIXED in Firefox 46

Status

()

Core
Audio/Video: Playback
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jrmuizel, Assigned: mattwoodrow)

Tracking

(Blocks: 1 bug)

unspecified
mozilla46
Points:
---

Firefox Tracking Flags

(firefox43 ?, firefox44 affected, firefox45 affected, firefox46 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
I see this on OS X Nightly with e10s
(Reporter)

Updated

2 years ago
Keywords: regressionwindow-wanted
Component: Audio/Video → Audio/Video: Playback

Comment 1

2 years ago
I can see this on Windows7.
https://hg.mozilla.org/mozilla-central/rev/40b58759c962bf1a8dbf8b56a7227778dfdcfa50
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0 ID:20151209030228


Steps to reproduce:
1. Set layers.acceleration.draw-fps = true
2. Open http://www.youtube.com/watch?v=BH0BNbwqNK8&webm=1&html5=1

Actual Results:
Counter displays around 60

Expected Results:
Counter displays around 45

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8eb31a08c5b66fb68e949cb47857a124261fc742&tochange=fbe8ec51aa386356cff2198d92a6e9bd5e8d6906

Triggered by: fbe8ec51aa38	David Anderson — Compute the compositor's damage region before composites, rather than layers updates. (bug 1217560, r=mattwoodrow)

Comment 2

2 years ago
Sorry spam, I am not sure the behavior of comment#1 is regression or not.
Please ignore comment#1, if I have misunderstood this bug.
Please provide better STR (or confirm Alice's findings from comment 1) if you want help tracking this down.
Flags: needinfo?(jmuizelaar)
(Assignee)

Updated

2 years ago
Blocks: 1195790
(Assignee)

Updated

2 years ago
Assignee: nobody → matt.woodrow
Flags: needinfo?(jmuizelaar)
(Assignee)

Comment 4

2 years ago
Created attachment 8699787 [details] [diff] [review]
Don't take the snapshot until after compositing

Compositing ImageHosts updates the mLastFrameID variable, which we cache in LayerTreeInvalidation.

Taking the snapshot before we composite means that we store the value of the previous frame, and will then invalidate even if the image hasn't changed.
Attachment #8699787 - Flags: review?(dvander)
(Assignee)

Comment 5

2 years ago
Created attachment 8699789 [details] [diff] [review]
Don't trigger composites for the fps counter

mInvalidRegion is only read on the following composite, so we always ended up with an invalid region and forced 60fps compositing with video playing.
Attachment #8699789 - Flags: review?(bgirard)
(Assignee)

Comment 6

2 years ago
This makes quite a big difference to our YouTube power consumption.

Updated

2 years ago
Attachment #8699789 - Flags: review?(bgirard) → review+
Keywords: regressionwindow-wanted
Comment on attachment 8699787 [details] [diff] [review]
Don't take the snapshot until after compositing

Review of attachment 8699787 [details] [diff] [review]:
-----------------------------------------------------------------

r=me could you extend the comment with the same explanation?
Attachment #8699787 - Flags: review?(dvander) → review+
Is this potentially related to the excessive power consumption?
Matt, is this bug a regression from dvander's bug 1195790, after all? That change is in FF 44. Are your patches safe enough to uplift to 44 (Beta)?
status-firefox43: --- → ?
status-firefox44: --- → affected
status-firefox45: --- → affected
status-firefox46: --- → affected
Flags: needinfo?(matt.woodrow)
Priority: -- → P1
(Assignee)

Comment 10

2 years ago
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #8)
> Is this potentially related to the excessive power consumption?

Yes, it is definitely related.

(In reply to Chris Peterson [:cpeterson] from comment #9)
> Matt, is this bug a regression from dvander's bug 1195790, after all? That
> change is in FF 44. Are your patches safe enough to uplift to 44 (Beta)?

Yeah, it should be fairly low risk. I'd like to let it sit on Nightly for a bit, and then we should aim to uplift where we can.
Flags: needinfo?(matt.woodrow)

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f9637decbe5d
https://hg.mozilla.org/mozilla-central/rev/bccb7856b4c0
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.