Bug 228816

Summary: MediaPlayerPrivateMediaStreamAVFObjC should skip enqueuing frames when not visible
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebRTCAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, cdumez, changseok, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, hta, jer.noble, kondapallykalyan, pdr, philipj, sergio, tommyw, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description youenn fablet 2021-08-05 03:09:27 PDT 
MediaPlayerPrivateMediaStreamAVFObjC should skip enqueuing frames when not visible
Comment 1 youenn fablet 2021-08-05 03:10:59 PDT 
<rdar://81077972>
Comment 2 youenn fablet 2021-08-05 03:24:45 PDT 
Created attachment 434974 [details]
Patch
Comment 3 Eric Carlson 2021-08-05 09:38:47 PDT 
Comment on attachment 434974 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=434974&action=review

> Source/WebCore/ChangeLog:11
> +        We do this by stopping to call ensureLayers when getting a new track. Instead we react upon player renderingCanBeAccelerated value.

s/by stopping to call/by not calling/

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:611
> -void MediaPlayerPrivateMediaStreamAVFObjC::setVisible(bool visible)
> +void MediaPlayerPrivateMediaStreamAVFObjC::setVisible(bool isVisible)

Not new to this patch, but we might want to rename this to `setPageIsVisible`, or maybe `setViewportIsVisible`, to match the names of the other "visible" methods.
Comment 4 youenn fablet 2021-08-05 10:49:04 PDT 
(In reply to Eric Carlson from comment #3)
> Comment on attachment 434974 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=434974&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        We do this by stopping to call ensureLayers when getting a new track. Instead we react upon player renderingCanBeAccelerated value.
> 
> s/by stopping to call/by not calling/
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:611
> > -void MediaPlayerPrivateMediaStreamAVFObjC::setVisible(bool visible)
> > +void MediaPlayerPrivateMediaStreamAVFObjC::setVisible(bool isVisible)
> 
> Not new to this patch, but we might want to rename this to
> `setPageIsVisible`, or maybe `setViewportIsVisible`, to match the names of
> the other "visible" methods.

Thanks for the review, I'll do that renaming in a follow-up: https://bugs.webkit.org/show_bug.cgi?id=228837
Comment 5 youenn fablet 2021-08-05 10:53:07 PDT 
Created attachment 435006 [details]
Patch for landing
Comment 6 EWS 2021-08-06 02:15:58 PDT 
Committed r280720 (240311@main): <https://commits.webkit.org/240311@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 435006 [details].