Patch for KRfb to work with RemoteAccess KWin Wayland -> XDP-KDE -> Pipewire stream
Works:
- Screen in read-only mode
davidedmundson | |
graesslin |
Plasma | |
KDE Applications |
Patch for KRfb to work with RemoteAccess KWin Wayland -> XDP-KDE -> Pipewire stream
Works:
Pay attention to logs, in case of difficulties use pipewire-monitor, qdbusviewer and launch desktop portals with -v options
No Linters Available |
No Unit Test Coverage |
There was a D5211, that added xcb framebuffer plugin; and after that x11 framebuffer was removed from source tree ( https://phabricator.kde.org/R437:f131f7ddba584779144e7f737a98b24de430824c ), so this patch of needs to be slightly updated to match the new krfb source.
I'm not competent in all this Wayland/OpenGL/GBM things (yet) but at least I hope that i can test this, when kwin part is ready and merged (which one it is? D1230? D1231?) so this work will not be forgotten in the annals of history :)
CMakeLists.txt | ||
---|---|---|
58 | "ouput" => "output"? | |
events/fakeinput/krfb_events_fakeinput.desktop | ||
5 ↗ | (On Diff #15147) | xx at the beginning? |
framebuffers/gbm/gbmframebuffer.cpp | ||
296 ↗ | (On Diff #15147) | Is the format *always* the same? True color 32bpp? |
framebuffers/gbm/logging.h | ||
25 ↗ | (On Diff #15147) | maybe whole this logging thing can be ported to ecm_qt_declare_logging_category ( https://api.kde.org/ecm/module/ECMQtDeclareLoggingCategory.html ) ? |
framebuffers/x11/x11framebufferplugin.cpp | ||
23 ↗ | (On Diff #15147) | no x11 plugin anymore :( |
krfb/krfb-events.json | ||
4 ↗ | (On Diff #15147) | "xxFrame Buffer plugins for KRfbxx" -> "xxEvents plugins for KRfbxx" ? :) |
@alexeymin , yeah, I've seen news about KRfb changes not so long ago. I'll fix this PR after support once remote access is upstreamed, it's quite troublesome to maintain so many changes around.
Thanks for comments!
P.S. The thing I'm stuck on is keyboard support for VNC... It almost explicitly requests rfbkeysyms (X11 ones), dunno how to work with these in Wayland.
While I'm here and still remember some things that I don't like here - naming:
There is a class EventsPlugin and files eventsplugin.{h,cpp}, class EventsManager and files eventsmanager.{h,cpp} and this is fine.
But there is also class EventHandler, but files are called events.{h,cpp}, it makes it a little bit harder to quickly read the code; maybe it makes sense to rename file also to eventhandler.h ?
Also class FakeInputEventHandler in file fakeinputevents.h; it may be also better to name the file fakeinputeventhandler.h...
And the most important suggestion I have: can this work be split in 2 parts:
Will be a bit easier to review and test, step by step.
What do you think?
I've digged through old diff and I must say, many of what's described here will be peeled off. For now we support read-only views, so I'll be focusing on it.
Too late did I realize that I'm making pipewire-based framebuffer, not xdp-kde based!
I guess rewiring stuff will take another 3-4 days, sorry :D
@jgrulich I started to hack together a very simple krfb-with-dbus-with-xdp-kde-with-pipewire... thingie.
Now I'm curious, you have handle and session_handle paths everywhere in Screencast interface inside XDP-KDE source. Why? I can understand why session_handle is there - to create and track sessions of course, but what is handle used for?
You are not supposed to be using xdg-desktop-portal-kde, but just xdg-desktop-portal (which will call xdg-desktop-portal-kde or xdg-desktop-portal-gtk based on DE). The first handle is object path of org.freedesktop.portal.Request object, this is an object created for every portal call which is supposed to be used e.g. to close the request. See how I use screencast portal here https://cgit.kde.org/xdg-portal-test-kde.git/tree/src/portaltest.cpp.
The Screencast portal DBus interface is here: https://github.com/flatpak/xdg-desktop-portal/blob/master/data/org.freedesktop.portal.ScreenCast.xml
Thanks for the xml, I was able to pass it to qdbus2cpp. Amusingly, if I just try auto reply = dbusXdpService->CreateSession(QVariantMap()); (with empty parameter map), the main desktop portal app just segfaults! I mean, not the kde impl one but the main XDG Desktop Portal!
I'm fixing my parameters thanks to your test app, but... should that really happen?
Hmm XDG Desktop Portal 0.11 just simply doesn't have ScreenCast interface on DBus... @jgrulich do I need some bleeding edge version or is it not yet in master branch?
To be honest, GLib + DBus is a mystery for me, I don't really know how this is implemented, it generates lots of code from XML files where you can easilly get lost so I have no idea whether they check parameters or not. It's better to ask someone on Flatpak channel if you really want to know what's going on, but I guess you shouldn't be able to call it with less parameters then you supposed to, right?
Nailed it, XDP just thinks KDE is inferior:
XDP: loading /usr/share/xdg-desktop-portal/portals/kde.portal XDP: portal implementation for KDE XDP: portal implementation supports org.freedesktop.impl.portal.Access XDP: portal implementation supports org.freedesktop.impl.portal.AppChooser XDP: portal implementation supports org.freedesktop.impl.portal.Email XDP: portal implementation supports org.freedesktop.impl.portal.FileChooser XDP: portal implementation supports org.freedesktop.impl.portal.Inhibit XDP: portal implementation supports org.freedesktop.impl.portal.Notification XDP: portal implementation supports org.freedesktop.impl.portal.Print
For GNOME it mentions ScreenCast though, interesting
Ah, that's because I use custom built XDP-KDE executable, installed to prefix instead of /usr
@jgrulich , thanks for the pointers
but I guess you shouldn't be able to call it with less parameters then you supposed to, right?
still, it shouldn't just crash the XDP right away... Just wanted to confirm this isn't normal
@jgrulich @alexeymin please review new framebuffer approach
This is the first draft of the implementation. I intend to add more features and fix review comments in the process.
Missing features:
How exactly I can test if something works? And what are the requirements? PipeWire? Something else?
Or is this only for code review for now?
Pay attention to logs, in case of difficulties use pipewire-monitor, qdbusviewer and launch desktop portals with -v options
@alexeymin This is a working version but I plan on adding more in the coming weekends. Just wanted to make sure there's someone motivated to review and merge this.
Thanks for looking at my stuff.
I have two things I observed when implementing same in WebRTC to have this working in Firefox and Chrome:
One more possible thing to improve, I don't know if there is interest in supporting Gnome, but Gnome creates streams in BGRx format, which means it wouldn't mach your stream, if you want to support it you need to change it to something like :
":", pwType->format_video.format, "Ieu", pwType->video_format.RGBx, SPA_POD_PROP_ENUM(2, pwType_->video_format.RGBx, pwType_->video_format.BGRx)
There is now new release of PipeWire (version 0.2.0), which breaks API a bit. I would suggest to bump PipeWire requirements to 0.2.0 and change your code. There is also now a version file included so in future we can use this to check PW version during build time and support more versions of the code. Unfortunately this header file was not included before and thus you cannot use it now, you would need to do the check in CMake in case you would want to support both PW 0.1.9 and PW 0.2.0. I'll be changing my code in xdg-desktop-portal-kde today, trying to support both in stable and only new PW 0.2.0 in master.
@jgrulich thanks, I'll definitely use that. Have my hands fullatm but will definitely return to this diff and refresh once have some free time
Now I realized what you mean, you mean that I didn't change libspa version as I did for pipewire, right? That hasn't change in released PW tarballs, but it's already changed now in master (Wim started versioning it) so I will have to fix this as well.
@jgrulich In my regularly-irregular wanderings I noticed you already picked up changes from this diff and that's totally okay. Should I close this?
I started looking into this because I work now on the remote desktop support in xdg-desktop-portal-kde and the best way how to implement it is to have support in some application. I forked your branch and added support for PipeWire 0.2.x and started rewriting it to use remote desktop portal. I hope you don't mind that. Feel free to close this one and we can later open a new review with full remote desktop support.
I hope you don't mind that.
On the contrary, I'm very glad it was picked up and even got to some mass media :)
Sure, just ping me for review, I have a vacation in October.