Bug 221747 – Make RemoteRealtimeVideoSource a RealtimeVideoCaptureSource

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

Make RemoteRealtimeVideoSource a RealtimeVideoCaptureSource

[youenn fablet]

Created attachment 419969 [details]
WIP

[youenn fablet]

Created attachment 419976 [details]
Patch

[youenn fablet]

Created attachment 419978 [details]
Patch

[Eric Carlson]

Comment on attachment 419978 [details]
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::GeneratePresets(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?

[youenn fablet]

(In reply to Eric Carlson from comment #4)
> Comment on attachment 419978 [details]
> 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::GeneratePresets(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?

Right, let's add it to CreateMediaSourceForCaptureDeviceWithConstraints.

[youenn fablet]

Created attachment 420103 [details]
Patch for landing

[youenn fablet]

> > 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?

Actually, I need to implement it for clone-then-increase-resolution-or-frameRate cases.
I removed on the other hand the applyConstraintsFailed/Succeeded since they are no longer needed for video.

[EWS]

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

[youenn fablet]

Created attachment 420107 [details]
Patch

[EWS]

Committed r272778: <https://commits.webkit.org/r272778>

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

[Radar WebKit Bug Importer]

<rdar://problem/74270196>