Bug 235953

Summary: MediaRecorderPrivate should not need to create IOSurfaces
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebRTCAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 235951    
Bug Blocks:    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch for landing none

Description youenn fablet 2022-02-01 08:01:23 PST 
MediaRecorderPrivate should not need to create IOSurfaces
Comment 1 youenn fablet 2022-02-01 08:39:14 PST 
Created attachment 450532 [details]
Patch
Comment 2 youenn fablet 2022-02-02 06:04:12 PST 
Created attachment 450641 [details]
Patch
Comment 3 Eric Carlson 2022-02-02 08:56:11 PST 
Comment on attachment 450641 [details]
Patch

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

> Source/WebKit/ChangeLog:13
>          * WebProcess/cocoa/UserMediaCaptureManager.cpp:
>  
> +2022-02-01  Youenn Fablet  <youenn@apple.com>

This doesn't look right

> Source/WebKit/GPUProcess/webrtc/RemoteMediaRecorder.cpp:95
> +        if (!pixelBuffer)
> +            return;

Is it worth logging an error message?

> Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderPrivate.cpp:110
> +        if (!copySharedVideoFrame(remoteSample->imageBuffer()))
> +            return;

Ditto
Comment 4 youenn fablet 2022-02-03 01:39:40 PST 
(In reply to Eric Carlson from comment #3)
> Comment on attachment 450641 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=450641&action=review
> 
> > Source/WebKit/ChangeLog:13
> >          * WebProcess/cocoa/UserMediaCaptureManager.cpp:
> >  
> > +2022-02-01  Youenn Fablet  <youenn@apple.com>
> 
> This doesn't look right

Right, will fix.

> > Source/WebKit/GPUProcess/webrtc/RemoteMediaRecorder.cpp:95
> > +        if (!pixelBuffer)
> > +            return;
> 
> Is it worth logging an error message?
> 
> > Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderPrivate.cpp:110
> > +        if (!copySharedVideoFrame(remoteSample->imageBuffer()))
> > +            return;
> 
> Ditto

It is worth, the issue is that it might log this 30 fps per second which is a lot.
I'll file a bug to add reasonable logging at reader/writer level.
Comment 5 youenn fablet 2022-02-03 01:40:38 PST 
I filed https://bugs.webkit.org/show_bug.cgi?id=236066
Comment 6 youenn fablet 2022-02-03 01:41:26 PST 
Created attachment 450749 [details]
Patch for landing
Comment 7 EWS 2022-02-03 06:32:28 PST 
Committed r289050 (246757@main): <https://commits.webkit.org/246757@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 450749 [details].
Comment 8 Radar WebKit Bug Importer 2022-02-03 06:33:17 PST 
<rdar://problem/88432554>