Bug 231286 - getDisplayMedia MediaStreamTrack.applyConstraints behavior regressed in Safari 15
Summary: getDisplayMedia MediaStreamTrack.applyConstraints behavior regressed in Safar...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: Safari 15
Hardware: Mac (Intel) macOS 11
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-06 05:01 PDT by Arjun Vade
Modified: 2021-12-05 03:31 PST (History)
11 users (show)

See Also:


Attachments
issue_illustrator (1.29 KB, text/html)
2021-10-06 05:01 PDT, Arjun Vade
no flags Details
Patch (11.30 KB, patch)
2021-10-06 06:41 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (35.76 KB, patch)
2021-10-08 09:10 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (36.12 KB, patch)
2021-10-11 00:39 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (35.91 KB, patch)
2021-10-18 07:33 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (36.69 KB, patch)
2021-10-19 00:21 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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?