-
-
Notifications
You must be signed in to change notification settings - Fork 959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support XDG portal and Pipewire #2507
base: master
Are you sure you want to change the base?
Conversation
This is not present in the glad version imported in Sunshine, plug the holes so that these functions may be used.
Add a new portal "grab" implementation that will request the necessary permissions over XDG portals and use Pipewire for video data streaming. This supports DMA-Buf buffer exchange for hardware accelerated encoding (VAAPI, nvenc), software encoding will only work with memory buffers.
|
How does one use the new portal? Compiled and launched the PR on Arch Linux with KDE Plasma 6 and just getting this in the logs. (I already removed the KMS CAP_SYS_ADMIN as it was trying to interact with the DBus session as root) I am able to do the Pipewire screenshare in OBS which opens the dialog for selecting a monitor/window/creating a new output.
|
if (next_frame > now) { | ||
std::this_thread::sleep_for((next_frame - now) / 3 * 2); | ||
} | ||
while (next_frame > now) { | ||
std::this_thread::sleep_for(1ns); | ||
now = std::chrono::steady_clock::now(); | ||
} | ||
next_frame = now + delay; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency you might want to incorporate the changes from #2333.
The changes would be fairly benign: kick the two-step sleep; base the next_frame time on previous theoretical instead of after capture; add sleep overshoot tracking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cheers! Will look to add this.
@parkerlreed as it seems, you are already using the XDG portal, and it is not playing ball:
This FWIW, I have been testing "the real thing" (i.e. a locally built flatpak), on a recent GNOME Desktop. |
Other than removing the cap sys admin, I don't think anything else is done special for tweaking. Used the existing sunshine-git AUR tooling and dropped in your branch. https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=sunshine-git I'm happy to test the Flatpak directly if you have a builld I can try. Considering OBS is requesting successfully, I think the actual portal session on KDE is working as expected. Thanks. |
Tried it out, this seems to fail when no remotedesktop protocol is found instead of falling back, so this was not usable on cosmic. and I assume sway with the default https://github.com/emersion/xdg-desktop-portal-wlr/ but https://github.com/waycrate/xdg-desktop-portal-luminous may work |
I built and ran this locally instead of packaging it up. It gets to here and then slowly eats all available memory and then freezes the system. I did not get a prompt from the Portal for selecting a monitor/window.
|
@garnacho Thank you for the PR! Would you mind signing the CLA? |
@garnacho you don't need to check with your employer, unless you're submitting the PR on behalf of them. |
@garnacho I'll convert this to a draft for now. You can mark it as ready once you've signed the CLA. Thank you! |
This comment was marked as off-topic.
This comment was marked as off-topic.
@garnacho, any chance you can move forward with the CLA? Having proper XDG portal + Pipewire support would be a massive upgrade in user experience in any modern Linux distro! |
@garnacho Bump? |
I'll be closing this due to no response. If you'd like it re-opened please let me know. |
The CLA is no longer required on this project. If you're still interested in implementing this please let me know. |
Reopening now that this can be accepted under the terms of GPLv3 alone. I will shepherd this PR through if the author is not available. |
I was avoiding looking at the code incase the PR was going to be dropped, but now that the code is acceptable would it be better to split portals and pipewire into two seperate categories? Pipewire can be used independently by dropping the file descriptor and by passing a path directly to If this is a feature that sunshine would like in the future I wonder if it would be worth it to split this into a it would also be nice if this didn't have a hard reliance on the remote desktop protocol. |
Has anyone tried capturing virtual display using this pipewire capture method? I built and ran this but even though i get the portal dialog to select the outputs, i get |
Sorry, I've been off the grid for personal reasons since late Aug... It is great news to me that a CLA is no longer required, since the legal department of my employer commanded me not to sign it (and took their sweet time in doing so... Also sometime during Aug). Now that this barrier is lifted, I'll look to update the PR, resolve conflicts, etc. Pipewire+portal support is still beneficial to the Linux ecosystem, and would be nice to have. |
This fixed capturing virtual monitors problem on wayland for me. Since kms cannot capture virtual monitors on gnome wayland (at least for me) this pr makes it possible to capture virtual devices. Thank you for your pr @garnacho |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking forward to this PR! From my understanding this can replace our KMS/Wayland capture, and no setcap will be needed?
I have a few comments/suggestions below and can review further once this is rebased.
Let me know if you have any questions with the packaging stuff. Thanks!
AND NOT (${LIBDRM_FOUND} AND ${LIBCAP_FOUND}) | ||
AND NOT ${LIBVA_FOUND}) | ||
message(FATAL_ERROR "Couldn't find either cuda, wayland, x11, (libdrm and libcap), or libva") | ||
message(FATAL_ERROR "Couldn't find either cuda, wayland, x11, pipewire, (libdrm and libcap), or libva") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
message(FATAL_ERROR "Couldn't find either cuda, wayland, x11, pipewire, (libdrm and libcap), or libva") | |
message(FATAL_ERROR "Couldn't find either cuda, libva, pipewire, wayland, x11, or (libdrm and libcap)") |
Let's alphabetize these.
AND NOT ${WAYLAND_FOUND} | ||
AND NOT ${X11_FOUND} | ||
AND NOT ${PIPEWIRE_FOUND} | ||
AND NOT (${LIBDRM_FOUND} AND ${LIBCAP_FOUND}) | ||
AND NOT ${LIBVA_FOUND}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AND NOT ${WAYLAND_FOUND} | |
AND NOT ${X11_FOUND} | |
AND NOT ${PIPEWIRE_FOUND} | |
AND NOT (${LIBDRM_FOUND} AND ${LIBCAP_FOUND}) | |
AND NOT ${LIBVA_FOUND}) | |
AND NOT ${LIBVA_FOUND} | |
AND NOT ${PIPEWIRE_FOUND} | |
AND NOT ${WAYLAND_FOUND} | |
AND NOT ${X11_FOUND} | |
AND NOT (${LIBDRM_FOUND} AND ${LIBCAP_FOUND}) | |
) |
pkg_check_modules(GIO gio-2.0 gio-unix-2.0) | ||
pkg_check_modules(PIPEWIRE libpipewire-0.3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Runtime deps will need to be added here: https://github.com/LizardByte/Sunshine/blob/master/cmake/packaging/linux.cmake#L43-L73
And for packaging need to be added in these different places:
Sunshine/scripts/linux_build.sh
Line 88 in 7dd836d
function add_debain_based_deps() { Sunshine/scripts/linux_build.sh
Line 151 in 7dd836d
function add_fedora_deps() { Sunshine/packaging/sunshine.rb
Line 40 in 7dd836d
on_linux do Sunshine/packaging/linux/Arch/PKGBUILD
Line 15 in 7dd836d
depends=( # BuildRequires: boost-devel >= 1.86.0 - https://github.com/LizardByte/Sunshine/tree/master/packaging/linux/flatpak/modules (probably) and
modules:
@@ -0,0 +1,777 @@ | |||
/** | |||
* @file src/platform/linux/portalgrab.cpp | |||
* @brief todo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every file has this completed now (probably wasn't the case when this PR was first submitted), so plenty of examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you regenerate these egl.h
and egl.c
files or just add to the existing ones? One of things on my todo list is try to find a way to generate them during build time, so if you regenerated them, that would be nice to know the parameters you used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC I wrote these manually... I seemed to see in the git history other instances of this happening, and was expecting to get some directions during review. The good news is that these additions should be part of a regenerated egl.[ch] set of files.
Thanks for the preliminary review @ReenigneArcher!
In principle, yes to both. The Wayland implementation available relies on a protocol fairly specific to wlroots-based compositors, there does exist a proxy between XDG portals and this protocol that AFAIU should work with this implementation, plus a portal implementation will also work on other major desktops like KDE and GNOME. And the greater support matrix should require less people to resort to KMS capturing and setcap. One benefit I found is being able to run via Flatpak only requiring uinput workarounds on the host for gamepads and input to work (e.g. installing
Thank you! I will. |
A thing I forgot to assert/reply, this indeed does not require setcap, and allows Sunshine to run with plain user permissions within the environment of a desktop session capable of portals. The portal implementation may/will show a dialog asking for user confirmation once during Sunshine startup. |
It's worth noting that portals capture can often perform worse then using the wayland/kms capture depending on the compositor/hardware. I don't see this being a total replacement for them but rather just a really good alternative for most people. Also since it was brought up, Wayland now has a standardized protocol for screen capture https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/124 which will likely be nice to swap to in the future. |
Since nvidia driver 560 there has been developments on nvfbc quoting the 560 beta driver. |
@cgutman is there something missing on this pr for it's implementation? |
Description
This PR adds a new "grab" implementation to use XDG desktop portals and Pipewire.
Screenshot
Issues Fixed or Closed
Type of Change
.github/...
)Checklist
Branch Updates
LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.