Bug 237359 - Scope capture sources by page identifiers
Summary: Scope capture sources by page identifiers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks: 237316
  Show dependency treegraph
 
Reported: 2022-03-02 03:48 PST by youenn fablet
Modified: 2022-03-10 13:11 PST (History)
14 users (show)

See Also:


Attachments
Patch (128.82 KB, patch)
2022-03-02 03:51 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (132.10 KB, patch)
2022-03-02 04:05 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (134.13 KB, patch)
2022-03-02 04:16 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (134.94 KB, patch)
2022-03-02 05:25 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (135.79 KB, patch)
2022-03-02 05:34 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (136.48 KB, patch)
2022-03-02 06:09 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (139.39 KB, patch)
2022-03-04 03:05 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (139.54 KB, patch)
2022-03-04 03:47 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing - v2 (138.58 KB, patch)
2022-03-04 08:37 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing - v3 (152.47 KB, patch)
2022-03-07 00:29 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Rebasing (151.44 KB, patch)
2022-03-08 04:32 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Rebasing (151.36 KB, patch)
2022-03-08 04:34 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (152.52 KB, patch)
2022-03-08 05:36 PST, 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 youenn fablet 2022-03-02 03:48:24 PST
Scope capture sources by page identifiers
Comment 1 youenn fablet 2022-03-02 03:51:35 PST
Created attachment 453585 [details]
Patch
Comment 2 youenn fablet 2022-03-02 04:05:29 PST
Created attachment 453586 [details]
Patch
Comment 3 youenn fablet 2022-03-02 04:16:45 PST
Created attachment 453587 [details]
Patch
Comment 4 youenn fablet 2022-03-02 05:25:46 PST
Created attachment 453591 [details]
Patch
Comment 5 youenn fablet 2022-03-02 05:34:40 PST
Created attachment 453592 [details]
Patch
Comment 6 youenn fablet 2022-03-02 06:09:01 PST
Created attachment 453596 [details]
Patch
Comment 7 youenn fablet 2022-03-02 08:15:35 PST
Comment on attachment 453596 [details]
Patch

GTK failure is legit and due to difference of -expected.txt files, I'll fix that. Putting as r? while waiting for iOS results.
Comment 8 Eric Carlson 2022-03-02 08:45:04 PST
Comment on attachment 453596 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:328
> +            if (mediaConstraints) {
> +                auto error = source->applyConstraints(*mediaConstraints);
> +                if (error)
> +                    return WTFMove(error->message);
> +            }

It would be nice to improve this eventually so we apply a union of the constraints, so we always capture at the highest requested constraint values and decimate/downsample as necessary. Maybe add a FIXME?

> Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:343
> +#if PLATFORM(IOS_FAMILY)
> +    if (perPageSources.cameraSource) {
> +        // We cannot reuse the source so fail it.
> +        perPageSources.cameraSource->captureFailed();
> +        perPageSources.cameraSource->stop();
> +    }
> +#endif

I would be cleaner to have the compile flag in a method (`canCaptureFromMultipleCameras`?).
Comment 9 youenn fablet 2022-03-04 03:05:54 PST
Created attachment 453828 [details]
Patch for landing
Comment 10 youenn fablet 2022-03-04 03:20:49 PST
> > Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:328
> > +            if (mediaConstraints) {
> > +                auto error = source->applyConstraints(*mediaConstraints);
> > +                if (error)
> > +                    return WTFMove(error->message);
> > +            }
> 
> It would be nice to improve this eventually so we apply a union of the
> constraints, so we always capture at the highest requested constraint values
> and decimate/downsample as necessary. Maybe add a FIXME?

RealtimeVideoCaptureSource does this for width, height and frameRate.
This should ensure there is no functionality regression.

> > Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:343
> > +#if PLATFORM(IOS_FAMILY)
> > +    if (perPageSources.cameraSource) {
> > +        // We cannot reuse the source so fail it.
> > +        perPageSources.cameraSource->captureFailed();
> > +        perPageSources.cameraSource->stop();
> > +    }
> > +#endif
> 
> I would be cleaner to have the compile flag in a method
> (`canCaptureFromMultipleCameras`?).

Done
Comment 11 youenn fablet 2022-03-04 03:47:43 PST
Created attachment 453829 [details]
Patch for landing
Comment 12 youenn fablet 2022-03-04 08:35:11 PST
Comment on attachment 453829 [details]
Patch for landing

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

> Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:295
> +    if (failSilently == FailSilently::No)

Instead of introducing this boolean, I'll add a dedicated method instead.
Comment 13 youenn fablet 2022-03-04 08:37:21 PST
Created attachment 453846 [details]
Patch for landing - v2
Comment 14 youenn fablet 2022-03-07 00:29:53 PST
Created attachment 453946 [details]
Patch for landing - v3
Comment 15 youenn fablet 2022-03-08 04:32:40 PST
Created attachment 454101 [details]
Rebasing
Comment 16 youenn fablet 2022-03-08 04:34:37 PST
Created attachment 454102 [details]
Rebasing
Comment 17 youenn fablet 2022-03-08 05:36:51 PST
Created attachment 454111 [details]
Patch for landing
Comment 18 EWS 2022-03-09 00:05:55 PST
Committed r291034 (248208@main): <https://commits.webkit.org/248208@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454111 [details].
Comment 19 Radar WebKit Bug Importer 2022-03-09 00:06:30 PST
<rdar://problem/90016088>
Comment 20 Jon Lee 2022-03-10 13:11:02 PST
This fixed platform/ios/mediastream/audio-muted-in-background-tab-gpu-process.html, originally marked as failing in bug 211509. Removed test expectation in r291125.