1237160 - Frame drops reported when video plays in the background

Status: RESOLVED FIXED

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

Description

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Steps to reproduce:

* Navigate to video e.g. https://www.youtube.com/watch?v=q4O_PovGTzo
* Right click and select Stats for Nerds
* Observe 0 frame drops
* Select another tab
* Select the original tab

Expected results:

Frame drops should show 0

Actual results:

Frame drops have increased while the other tab is shown

Comment 1

[Jean-Yves Avenard [:jya]]

Matt, the issue appears to be due to bug 1230338.

The VideoSink queries the VideoContainer to find out how many frames have been dropped.

However, this reports the frames that are removed due to being too old. This happens when there are more than 100 frames queued in the ImageContainer.

Incidentally, when the element is hidden, the images aren't composited, so they are "dropped" and are being reported to the VideoSink as such, regardless of why they were dropped in the first place (e.g. too slow to render them, or just not presenting it)..

Ideally, if we had the reason on why an image wasn't composited it would help distinguish the two cases. But this is likely a too intensive change

I think we should just revert partially bug 1230338, or simply ignore the value of how many frames have been dropped if it is found that the element is hidden.

The later wouldn't be 100% accurate, like if we did drop a frame because it's too slow, and suddenly the media element is hidden. The behaviour would be racy on when the next images are added to the container: is it prior the element being hidden or after.

Additionally, something that I find highly suspicious: frames are removed from the VideoContainer only when it has accumulated over 100 images.
That means that for a typical 1080p youtube video, when hidden, those images will constantly occupy over 310MB of memory.

Those frames should be dropped at the time they would have been rendered if the element was visible. But I'm not familiar enough with gfx/layers to know if that's a possibility.

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Do not count frames not composited as dropped.

This is a partial reversal of bug 1230338. We can't distinguish frames that are never composited because the media element is hidden from the frames genuinely dropped due to machine slowness. So until we can distinguish them, let's not report them as dropped.

Comment 3

[Matt Woodrow (:mattwoodrow)]

I'm not convinced that not reporting dropped frames (from the compositor) at all is an improvement.

When I tested this we were frequently dropping from on the compositor which weren't being reported, and we were worried about this interfering with Netflix/Youtube's fallback mechanisms.

I'd much prefer if we pushed forward on annotating the dropped frames correctly so that we can report correct numbers.

This isn't my module though, so if you and Anthony would prefer to ship this alternative then feel free to do so.

Comment 4

[Jean-Yves Avenard [:jya]]

I agree that reporting frames dropped by the compositor would be better. But as it is, reported them wrongly is worse than not reporting them at all. 

Should you tab-out or hide the window for any reason, every single frames ought to be displayed will be reported as drop. 

And that is bad. 

Now, if you implement bug 1245400, we will be able to report dropped frames properly. And I intend to act in this as soon as we can. Bug 1245400 is right in your alley :)

Comment 5

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8715173 [details] [diff] [review]
Do not count frames not composited as dropped.

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

(In reply to Jean-Yves Avenard [:jya] from comment #4)
> I agree that reporting frames dropped by the compositor would be better. But
> as it is, reported them wrongly is worse than not reporting them at all. 
> 
> Should you tab-out or hide the window for any reason, every single frames
> ought to be displayed will be reported as drop. 
> 
> And that is bad. 

I'm not convinced that this alternative is any better though. We're just swapping one bug for another.

Code-wise this patch is fine, but I'll defer to Chris to make the call as to which bug he'd prefer shipping.


> 
> Now, if you implement bug 1245400, we will be able to report dropped frames
> properly. And I intend to act in this as soon as we can. Bug 1245400 is
> right in your alley :)

Fixing this seems like a much better option, but I don't have time to work on it at the moment. I'm committed to fixing 3d transform regressions.

I'm happy to provide advice for it if you want to work on it though.

Comment 6

[Jean-Yves Avenard [:jya]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #5)

> > And that is bad. 
> 
> I'm not convinced that this alternative is any better though. We're just
> swapping one bug for another.
> 

I have trouble understanding how you could think that not accounting for a few frames that could potentially be dropped (knowing that the biggest chunk of dropped frames is always by far when we skip to the next key frame);

And a guarantee to get massive amount of frames being reported as dropped when they aren't...

Typical example:
I start watching a video, I get a notification that I got a new email, I switch to the tab with email and come back to the video..

Every frames have been marked as dropped..

> on it at the moment. I'm committed to fixing 3d transform regressions.
> 
> I'm happy to provide advice for it if you want to work on it though.

I will try, but it will take far longer to do it than it would anyone from the gfx team..

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8715173 [details] [diff] [review]
Do not count frames not composited as dropped.

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

I'd rather misreport a few frames than have the YouTube/Netflix quality drop back to low-res if you blur a tab for a while.

We should find a better solution that distinguishes between frames dropped due us being in the background vs dropped due rendering slow/late.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f3d064ce06d

Comment 9

[Jean-Yves Avenard [:jya]]

[Tracking Requested - why for this release]: Reporting an excessive number of dropped frames will cause youtube and netflix to drop to a lower quality video.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0f3d064ce06d

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Jean-Yves, are you going to fill the uplift request? Thanks

Comment 12

[Jean-Yves Avenard [:jya]]

Comment on attachment 8715173 [details] [diff] [review]
Do not count frames not composited as dropped.

Approval Request Comment
[Feature/regressing bug #]: 1237160
[User impact if declined]: Netflix or youtube and other web site relying on the quality attribute, could serve lower resolution than they should.
[Describe test coverage new/current, TreeHerder]: in central, manual test
[Risks and why]: Low, we go back to earlier code. We stop reporting frames genuinely dropped, but the impact should be low.
[String/UUID change made/needed]: None

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8715173 [details] [diff] [review]
Do not count frames not composited as dropped.

Taking it to improve the video situation.
Should be in 45 beta 6.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/99270715327e

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/30965b27b48e