| Summary: | Scope capture sources by page identifiers | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||||||||||||||||||
| Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||
| Severity: | Normal | CC: | annulen, eric.carlson, ews-watchlist, glenn, gyuyoung.kim, hta, jer.noble, jonlee, philipj, ryuan.choi, sergio, tommyw, webkit-bug-importer, youennf | ||||||||||||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=211509 | ||||||||||||||||||||||||||||||
| Bug Depends on: | |||||||||||||||||||||||||||||||
| Bug Blocks: | 237316 | ||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||
|
Description
youenn fablet
2022-03-02 03:48:24 PST
Created attachment 453585 [details]
Patch
Created attachment 453586 [details]
Patch
Created attachment 453587 [details]
Patch
Created attachment 453591 [details]
Patch
Created attachment 453592 [details]
Patch
Created attachment 453596 [details]
Patch
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 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`?). Created attachment 453828 [details]
Patch for landing
> > 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 Created attachment 453829 [details]
Patch for landing
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. Created attachment 453846 [details]
Patch for landing - v2
Created attachment 453946 [details]
Patch for landing - v3
Created attachment 454101 [details]
Rebasing
Created attachment 454102 [details]
Rebasing
Created attachment 454111 [details]
Patch for landing
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]. 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. |