[webkit-reviews] review granted: [Bug 221747] Make RemoteRealtimeVideoSource a RealtimeVideoCaptureSource : [Attachment 419978] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 11 09:04:42 PST 2021


Eric Carlson <eric.carlson at apple.com> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 221747: Make RemoteRealtimeVideoSource a RealtimeVideoCaptureSource
https://bugs.webkit.org/show_bug.cgi?id=221747

Attachment 419978: Patch

https://bugs.webkit.org/attachment.cgi?id=419978&action=review




--- Comment #4 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 419978
  --> https://bugs.webkit.org/attachment.cgi?id=419978
Patch

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

> Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:312
> +void
UserMediaCaptureManagerProxy::generatePresets(RealtimeMediaSourceIdentifier id,
CompletionHandler<void(Vector<VideoPresetData>&&, IntSize)>&&
completionHandler)

This name isn't entirely accurate because it generates presets and fetches the
current size. I don't love either of these names, but they would be more
correct: `getSizeAndGeneratePresets`, or `generatePresetsAndGetSize`?

> Source/WebKit/WebProcess/cocoa/RemoteRealtimeVideoSource.cpp:83
> +	  
connection()->sendWithAsyncReply(Messages::UserMediaCaptureManagerProxy::Genera
tePresets(identifier()), [this, protectedThis = WTFMove(protectedThis)](auto&&
presets, auto size) {

As we discussed, can this be moved into
CreateMediaSourceForCaptureDeviceWithConstraints so we don't need to make two
calls to the UI process to become ready?

> Source/WebKit/WebProcess/cocoa/RemoteRealtimeVideoSource.cpp:248
> +void RemoteRealtimeVideoSource::setFrameRateWithPreset(double,
RefPtr<VideoPreset>)
>  {
> -    m_hasRequestedToEnd = true;
> -    connection()->send(Messages::UserMediaCaptureManagerProxy::RequestToEnd
{ m_identifier }, 0);
>  }

Can this be removed completely?


More information about the webkit-reviews mailing list