Bug 231286

Summary: getDisplayMedia MediaStreamTrack.applyConstraints behavior regressed in Safari 15
Product: WebKit Reporter: Arjun Vade <avade>
Component: WebRTCAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, ews-watchlist, glenn, hta, jer.noble, philipj, pnormand, sergio, tommyw, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: Safari 15   
Hardware: Mac (Intel)   
OS: macOS 11   
Attachments:
Description Flags
issue_illustrator
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Arjun Vade 2021-10-06 05:01:17 PDT
Created attachment 440353 [details]
issue_illustrator

A valid applyConstraints on a MediaStreamTrack is now throwing OverconstrainedError. This used to work in earlier versions of safari and has regressed only with Safari 15. Attaching a sample html page that illustrates the issue.

Steps to reproduce:
1. Open the html page attached in safari
2. Click on the start button
3. Notice that video 1 works, while video 2 doesn't. Open the dev tools console and notice the OverconstrainedError.
Comment 1 Radar WebKit Bug Importer 2021-10-06 06:20:26 PDT
<rdar://problem/83930865>
Comment 2 youenn fablet 2021-10-06 06:41:31 PDT
Created attachment 440360 [details]
Patch
Comment 3 Eric Carlson 2021-10-06 09:24:32 PDT
Comment on attachment 440360 [details]
Patch

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

It would be good to include tests the specify only width, only height, and neither.

> Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.cpp:261
> +        if (!requestedWidth &&  requestedHeight && size().isEmpty())
> +            return { };

Should this be `!requestedHeight`?

> Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.cpp:265
> +        if (!requestedWidth) {
> +            captureSize.setWidth(*requestedWidth);

This will crash, shouldn't it be `if (requestedWidth)`?

> Source/WebCore/platform/mediastream/RealtimeVideoCaptureSource.cpp:268
> +            captureSize.setHeight(*requestedHeight);

Can't requestedHeight be null here?
Comment 4 youenn fablet 2021-10-08 09:10:30 PDT
Created attachment 440629 [details]
Patch
Comment 5 Eric Carlson 2021-10-08 09:58:43 PDT
Comment on attachment 440629 [details]
Patch

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

> Source/WebKit/WebProcess/cocoa/RemoteRealtimeAudioSource.cpp:49
> +    fprintf(stderr, "RemoteRealtimeAudioSource::create %d\n", (int)source->identifier().toUInt64());

Oops!

> Source/WebKit/WebProcess/cocoa/RemoteRealtimeDisplaySource.cpp:44
> +    fprintf(stderr, "RemoteRealtimeDisplaySource::create %d\n", (int)source->identifier().toUInt64());

Ditto

> Source/WebKit/WebProcess/cocoa/RemoteRealtimeVideoSource.cpp:51
> +    fprintf(stderr, "RemoteRealtimeVideoSource::create %d\n", (int)source->identifier().toUInt64());

Ditto
Comment 6 youenn fablet 2021-10-11 00:39:50 PDT
Created attachment 440761 [details]
Patch
Comment 7 youenn fablet 2021-10-18 07:33:27 PDT
Created attachment 441600 [details]
Patch
Comment 8 youenn fablet 2021-10-19 00:21:16 PDT
Created attachment 441695 [details]
Patch
Comment 9 EWS 2021-10-19 04:52:29 PDT
Committed r284444 (243206@main): <https://commits.webkit.org/243206@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 441695 [details].
Comment 10 Philippe Normand 2021-12-05 03:31:18 PST
Comment on attachment 441695 [details]
Patch

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

> LayoutTests/fast/mediastream/getDisplayMedia-size-expected.txt:6
> +FAIL original source should stay at a small size assert_equals: expected 640 but got 320
> +FAIL original source should stay at a big size assert_equals: expected 640 but got 1280

This is a bit confusing. Should these tests be reworked to have PASS expectations?