Improve screen sharing with PipeWire on Linux Wayland session [chromium/src : master]
Jan Grulich (Gerrit)
Attention is currently required from: Avi Drissman, Wez, Guido Urdaneta, mark a. foltz.
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Jan Grulich (Gerrit)
Attention is currently required from: Avi Drissman, Wez, Guido Urdaneta, mark a. foltz.
1 comment:
Patchset:
Here are screenshots to help you understand the situation:
Current behavior with two xdg-desktop-portal dialogs where both need to be confirmed:
https://jgrulich.fedorapeople.org/chromium-two-dialogs.pngThis is when using this patch:
https://jgrulich.fedorapeople.org/chromium-one-dialog.png
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Wez (Gerrit)
Attention is currently required from: Jan Grulich, Avi Drissman, Guido Urdaneta, mark a. foltz.
3 comments:
Commit Message:
Patch Set #3, Line 26:
chromium:682122This bug seems to be a generic tracking bug for enabling screen-capture on Wayland?
Do we have a more specific bug (briefly) describing the UX improvement this change is making, that we could link here instead?
File chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc:
// Do not request windows to be shared in case we will use PipeWire
// capturer:
// 1) Windows will be already requested to be shared as part
// of screen sharing request.
// 2) We would throw an additional useless dialog to our
// users.
// 3) We won't get list of windows through the PipeWire capturer
// and the picker dialog doesn't support showing previews for
// windows.nit: If you have a more specific bug to link to then you could reduce this to:
// Avoid offering window-capture as a separate source, since PipeWire's
// content-picker will offer both screen and window sources.
// See crbug.com/<number>.if (!content::desktop_capture::CanUsePipeWire()) {
media_types.push_back(content::DesktopMediaID::TYPE_WINDOW);
}This presumably only makes sense if PipeWire is selected _and_ the TYPE_SCREEN type is included in |sources|, otherwise the PipeWire picker isn't going to be shown?
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Guido Urdaneta (Gerrit)
Attention is currently required from: Jan Grulich, Avi Drissman, mark a. foltz.
2 comments:
File chrome/browser/media/webrtc/display_media_access_handler.cc:
Patch Set #3, Line 204:
// Do not request windows to be shared in case we will use PipeWire capturer:Simplify this comment following the advice from wez@ for desktop_capture_base.cc
Patch Set #3, Line 211:
auto it = std::remove(media_types.begin(), media_types.end(),nit: Use base::Erase() from base/stl_util.h to simplify the code.
base::Erase(media_types, content::DesktopMediaID::TYPE_WINDOW)
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Jan Grulich (Gerrit)
Attention is currently required from: Jan Grulich, Avi Drissman, mark a. foltz.
Jan Grulich uploaded patch set #4 to this change.
Improve screen sharing with PipeWire on Linux Wayland session
Current scenario when PipeWire desktop capturer is used:
Chromium picker dialog is created with screen and window capturer. Each
capturer makes an xdg-desktop-portal call, showing another picker
dialog. Once user confirms both pickers on xdg-desktop-portal side, yet
another picker is shown as a new capturer is created for the web page
itself.
With this change:
Chromium picker dialog is created, but only screen capturer will be
created as with xdg-desktop-portal the picker will handle both screens
and windows. Also in the chromium picker, the "window" tab creating
window capturer expects a list of windows and doesn't show previews,
but we need actually to behave exactly like the "screen" tab and show
preview of selected window. Then again, yet another picker from
xdg-desktop-portal is shown as a new capturer is created for the web
page itself.
Bug: chromium:1157006
Change-Id: I39eafc72eb46da7868d1114b5c106030c22787a4
---
M AUTHORS
M chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
M chrome/browser/media/webrtc/display_media_access_handler.cc
M content/public/browser/desktop_capture.cc
M content/public/browser/desktop_capture.h
5 files changed, 41 insertions(+), 1 deletion(-)
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Jan Grulich (Gerrit)
Attention is currently required from: Avi Drissman, Wez, Guido Urdaneta, mark a. foltz.
5 comments:
Commit Message:
Patch Set #3, Line 26:
chromium:682122
This bug seems to be a generic tracking bug for enabling screen-capture on Wayland? […]
Done
File chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc:
// Do not request windows to be shared in case we will use PipeWire
// capturer:
// 1) Windows will be already requested to be shared as part
// of screen sharing request.
// 2) We would throw an additional useless dialog to our
// users.
// 3) We won't get list of windows through the PipeWire capturer
// and the picker dialog doesn't support showing previews for
// windows.
nit: If you have a more specific bug to link to then you could reduce this to: […]
Done
if (!content::desktop_capture::CanUsePipeWire()) {
media_types.push_back(content::DesktopMediaID::TYPE_WINDOW);
}
This presumably only makes sense if PipeWire is selected _and_ the TYPE_SCREEN type is included in | […]
Done
When content::desktop_capture::CanUsePipeWire() returns true, it means not only the PipeWire can be used, but actually that it will be used. In WebRTC it follows same conditions so if we detect Wayland, we will use this capturer. There is also no other possibility besides PipeWire capturer, because the X11 based one will not work on Wayland at all.
File chrome/browser/media/webrtc/display_media_access_handler.cc:
Patch Set #3, Line 204:
// Do not request windows to be shared in case we will use PipeWire capturer:
Simplify this comment following the advice from wez@ for desktop_capture_base. […]
Done
Patch Set #3, Line 211:
auto it = std::remove(media_types.begin(), media_types.end(),
nit: Use base::Erase() from base/stl_util.h to simplify the code. […]
Done
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Wez (Gerrit)
Attention is currently required from: Jan Grulich, Avi Drissman, Guido Urdaneta, mark a. foltz.
Patch set 5:Code-Review +1
1 comment:
Patchset:
chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc LGTM!
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Guido Urdaneta (Gerrit)
Attention is currently required from: Jan Grulich, Avi Drissman, mark a. foltz.
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Avi Drissman (Gerrit)
Attention is currently required from: Jan Grulich, mark a. foltz.
Patch set 5:Code-Review +1
1 comment:
File AUTHORS:
Patch Set #5, Line 443:
<gru...@gmail.comEnd with a >
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Jan Grulich (Gerrit)
Attention is currently required from: mark a. foltz.
1 comment:
File AUTHORS:
Patch Set #5, Line 443:
<gru...@gmail.comEnd with a >
Done
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Jan Grulich (Gerrit)
Patchset:
Hi, may I ask you to submit this to commit queue and possibly merge if it passes?
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.
Chromium LUCI CQ (Gerrit)
Attention is currently required from: mark a. foltz.
CQ is trying the patch.
Note: The patchset #6 "Fix typo in AUTHORS file" sent to CQ was uploaded after this CL was CR+1-ed.
Reviewer, please verify there is nothing unexpected https://chromium-review.googlesource.com/c/2578840/6
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/2578840/6
Bot data: {"action": "start", "triggered_at": "2020-12-11T07:24:17.0Z", "revision": "b5fdc7f7b455655fa0d621252fe5c51cd3cae318"}
Chromium LUCI CQ (Gerrit)
Attention is currently required from: mark a. foltz.
Only full committers OR the CL owner if they have tryjob access are allowed to trigger CQ runs.
Committers in this project are members of:
* https://chrome-infra-auth.appspot.com/auth/groups/project-chromium-committers
Tryjob access is additionally granted to members of:
* https://chrome-infra-auth.appspot.com/auth/groups/project-chromium-tryjob-access
Jan Grulich (Gerrit)
Attention is currently required from: mark a. foltz.
Patch set 6:Commit-Queue +2
mark a. foltz (Gerrit)
Attention is currently required from: Jan Grulich.
Patch set 6:Code-Review +1
mark a. foltz (Gerrit)
Attention is currently required from: Jan Grulich.
Patch set 6:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
Improve screen sharing with PipeWire on Linux Wayland session
Current scenario when PipeWire desktop capturer is used:
Chromium picker dialog is created with screen and window capturer. Each
capturer makes an xdg-desktop-portal call, showing another picker
dialog. Once user confirms both pickers on xdg-desktop-portal side, yet
another picker is shown as a new capturer is created for the web page
itself.
With this change:
Chromium picker dialog is created, but only screen capturer will be
created as with xdg-desktop-portal the picker will handle both screens
and windows. Also in the chromium picker, the "window" tab creating
window capturer expects a list of windows and doesn't show previews,
but we need actually to behave exactly like the "screen" tab and show
preview of selected window. Then again, yet another picker from
xdg-desktop-portal is shown as a new capturer is created for the web
page itself.
Bug: chromium:1157006
Change-Id: I39eafc72eb46da7868d1114b5c106030c22787a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2578840
Commit-Queue: mark a. foltz <mfo...@chromium.org>
Reviewed-by: mark a. foltz <mfo...@chromium.org>
Reviewed-by: Wez <w...@chromium.org>
Reviewed-by: Guido Urdaneta <gui...@chromium.org>
Reviewed-by: Avi Drissman <a...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836814}
---
M AUTHORS
M chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
M chrome/browser/media/webrtc/display_media_access_handler.cc
M content/public/browser/desktop_capture.cc
M content/public/browser/desktop_capture.h
5 files changed, 32 insertions(+), 0 deletions(-)
Jan Grulich (Gerrit)
1 comment:
Patchset:
Thank you.
To view, visit change 2578840. To unsubscribe, or for help writing mail filters, visit settings.