fix: use generic capturer to list both screens and windows when possible #39111
Conversation
There was a problem hiding this comment.
The code looks good to me, but I think we need to resolve a patch - would you mind rebasing with main and resolving that patch conflict, and I'll review?
Also, if it's not too much trouble, you mention in the PR description backporting a fix from WebRTC. Would you mind linking to the CL of that upstream backport in the patch description or the PR description? That will just help us remove the patch when it eventually lands in main, without accidentally removing some needed code 🙂 Thanks again for this fix!
aiddya
force-pushed
the
pipewire-double-popup
branch
from
46c8331
to
2bcd128
Compare
|
@VerteDinde I've rebased and resolved the patch conflict. One of the Chromium updates pushed after I created this PR has brought in the backported WebRTC patch. It was the cause of the conflict and I've removed it. On the other hand, this means that I'll have to backport the PR manually to 26.x.y 😞. I'll add the CL details in the backport PR. |
aiddya
force-pushed
the
pipewire-double-popup
branch
from
2bcd128
to
f57d71c
Compare
Screensharing with PipeWire via XDG Desktop Portal requires explicit user permission via permission dialogs. Chromium has separate tabs for screens and windows and thus its portal implementation requests permissions separately for each. However, the screencast portal has no such limitation and supports both screens and windows in a single request. WebRTC now supports this type of capture in a new method called called `CreateGenericCapturer`. The `desktopCapturer` implementation has been modified to use it. Additionally, Chromium has been patched to use same generic capturer to ensure that the source IDs remain valid for `getUserMedia`.
aiddya
force-pushed
the
pipewire-double-popup
branch
from
f57d71c
to
ffc1271
Compare
VerteDinde
added
semver/patch
|
Release Notes Persisted
|
|
I have automatically backported this PR to "26-x-y", please check out #39189 |
trop
bot
added
in-flight/26-x-y
and removed
target/26-x-y
|
@aiddya Our automation opened a backport automatically - if you want, happy to manually backport the CL to it myself if you wouldn't mind linking me the relevant CL. Otherwise, I'll close the automated backport and let you open a manual backport 🙇♀️ |
|
@VerteDinde That would be great, thanks! Here's the CL: https://webrtc-review.googlesource.com/c/src/+/311549 |
trop
bot
added
merged/26-x-y
|
@VerteDinde Is this backportable to stable branches? I've noticed that the double permission dialogs sometimes cause focus bugs, where one dialog is perfectly superimposed on the other, but the bottom one has focus. The top dialog is not clickable, which leaves the impression that the application has frozen. I want to backport this to 25-x-y and perhaps 24-x-y too. The double permission dialogs were introduced by #38833, so the backport can be seen as a fixup. |
trop
bot
added
merged/25-x-y
…ble (electron#39111) Screensharing with PipeWire via XDG Desktop Portal requires explicit user permission via permission dialogs. Chromium has separate tabs for screens and windows and thus its portal implementation requests permissions separately for each. However, the screencast portal has no such limitation and supports both screens and windows in a single request. WebRTC now supports this type of capture in a new method called called `CreateGenericCapturer`. The `desktopCapturer` implementation has been modified to use it. Additionally, Chromium has been patched to use same generic capturer to ensure that the source IDs remain valid for `getUserMedia`.
Description of Change
Screensharing with PipeWire via XDG Desktop Portal requires explicit user permission via permission dialogs. Chromium has separate tabs for screens and windows and thus its portal implementation requests permissions separately for each. However, the screencast portal has no such limitation and supports both screens and windows in a single request.
WebRTC now supports this type of capture in a new method called
CreateGenericCapturer. ThedesktopCapturerimplementation has been modified to use it. Additionally, Chromium has been patched to use same generic capturer to ensure that the source IDs remain valid forgetUserMedia.Fixes #39043
Checklist
npm testpassesRelease Notes
Notes: Fixed a redundant permission popup while fetching screens and windows using
desktopCapturer.getSources()on Wayland.