1732230 - Heavy freezing while playing videos on YouTube fullscreen

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Asif Youssuff]

Noticed today that fullscreen videos on YouTube were freezing and stuttering heavily.

Example video: https://www.youtube.com/watch?v=Q6SQ2_iShIs

I played this back on a 13" Early 2015 MacBook Air with a 1.6GHz dual core i5 and macOS Catalina.

Issue occurs on both 720p and 1080p (have not tried other resolutions).

Profile: https://share.firefox.dev/3kypx43

Profile is a recording of starting playback, fullscreen, playing back for a few minutes and exiting fullscreen.

mozregression:

Was this inbound build good, bad, or broken? (type 'good', 'bad', 'skip', 'retry', 'back' or 'exit' and press Enter): good
84:16.64 INFO: Narrowed inbound regression window from [ddc9c3bd, af7e7b1d] (3 builds) to [c9a191f9, af7e7b1d] (2 builds) (~1 steps left)
84:16.77 INFO: No more inbound revisions, bisection finished.
84:16.77 INFO: Last good revision: c9a191f99fa323e42bc664320d160b4ff4f3f4f1
84:16.77 INFO: First bad revision: af7e7b1d891ee07ec15c3702cbce8f509679b82a
84:16.77 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c9a191f99fa323e42bc664320d160b4ff4f3f4f1&tochange=af7e7b1d891ee07ec15c3702cbce8f509679b82a

Comment 1

[Asif Youssuff]

Attachment: about:support

Comment 2

[Brad Werth [:bradwerth]]

I'll sort it out.

Comment 3

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

Set release status flags based on info from the regressing bug 1731815

Comment 4

[Markus Stange [:mstange]]

I also see a huge memory increase of the VTDecoderXPCService during fullscreen youtube playback. Maybe we're only enqueuing buffers and never dequeuing them? Or we're missing some other reference counting release?

Comment 5

[Markus Stange [:mstange]]

[Tracking Requested - why for this release]: Video playback regression

Comment 6

[Markus Stange [:mstange]]

Yeah, NativeLayerCA::Representation::EnqueueSurface never releases sampleBuffer. I think it needs to.

Comment 7

[Markus Stange [:mstange]]

Same with pixelBuffer. The layer will have a ref to the sampleBuffer, which has a ref to the pixelBuffer. All other references need to be released.

Sorry for not catching this in review. This should be using CFTypeRefPtr to do the release automatically.

Comment 8

[Brad Werth [:bradwerth]]

Attachment: Bug 1732230: Ensure deallocation in NativeLayerCA::Representation::EnqueueSurface.

Using CFTypeRefPtr ensures that the created Core Foundation / Media types are
deallocated under all code paths. The duplicated syntax of -"Deallocator"
variables is necessary because CFTypeRefPtr is designed to work with
functions that return the required type. All of the creation methods in use
here take a type reference as a parameter, making it impossible to pass in a
CFTypeRefPtr directly. The "Deallocator" variables serve the purpose of
ensuring deallocation even though the variables are not otherwise referenced.

This change also cleans up the casting for CFMutableDictionaryRef to use a
simpler Objective-C idiom.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22bebc9d9463
Ensure deallocation in NativeLayerCA::Representation::EnqueueSurface. r=mstange

Comment 13

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/22bebc9d9463

Comment 15

[Timea Cernea [:tbabos][inactive]]

Verified-fixed on the latest Firefox Nightly 94.0a1 (2021-09-27) (64-bit) on macOS 10.15 and macOS Beta 12.