Bug 189930 – [MediaStream] Update constraints supported by getDisplayMedia

• Component: WebRTC
• Status: RESOLVED FIXED

[Eric Carlson]

The spec has changed the constraints that getDisplayMedia supports.

[Eric Carlson]

Created attachment 350686 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/44740305>

[youenn fablet]

Comment on attachment 350686 [details]
Patch

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

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:163
> +    // 2. For each member present in constraints whose value, value, is a dictionary, run the following steps:

s/, value,//

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:175
> +

Remove this line?

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:213
> +        case MediaConstraintType::EchoCancellation:

From your quote of the spec, we should also reject if any of these value has a min/exact value.
Maybe we should add some tests for these values.

> Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp:245
> +            if (request.type != MediaStreamRequest::Type::UserMedia || !request.audioConstraints.isValid)

To improve readability, we could try to split more getDisplayDevice vs. getUserMedia code path.
For instance, have one top level request.type check in this function and introduce getCaptureDevices and getScreenDevices to iterate over relevant devices according request.type.

[Eric Carlson]

Comment on attachment 350686 [details]
Patch

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

>> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:163
>> +    // 2. For each member present in constraints whose value, value, is a dictionary, run the following steps:
> 
> s/, value,//

This is verbatim from the spec.

>> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:175
>> +
> 
> Remove this line?

OK

>> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:213
>> +        case MediaConstraintType::EchoCancellation:
> 
> From your quote of the spec, we should also reject if any of these value has a min/exact value.
> Maybe we should add some tests for these values.

These are audio-only constraints, and we don't support audio with getDisplayMedia. Good idea about the test, I will add an invalid audio constraint to the test.

>> Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp:245
>> +            if (request.type != MediaStreamRequest::Type::UserMedia || !request.audioConstraints.isValid)
> 
> To improve readability, we could try to split more getDisplayDevice vs. getUserMedia code path.
> For instance, have one top level request.type check in this function and introduce getCaptureDevices and getScreenDevices to iterate over relevant devices according request.type.

Good idea, I will do that.

[Eric Carlson]

Created attachment 350756 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 350756 [details]
Patch for landing

Clearing flags on attachment: 350756

Committed r236465: <https://trac.webkit.org/changeset/236465>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Carlson]

Reopening to attach new patch.

[Eric Carlson]

Created attachment 350787 [details]
Fix broken API tests

[WebKit Commit Bot]

Comment on attachment 350787 [details]
Fix broken API tests

Clearing flags on attachment: 350787

Committed r236476: <https://trac.webkit.org/changeset/236476>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.