Add "labwc over VNC" backend (Wayland)#3587
Conversation
|
Wow, this is quite a monumental work! The refactoring aspects look good as well. I really appreciate the work you put into this project and the way you interact with the user community of xrdp. I will give this a build and a test run as soon as I have time. |
Can confirm that it works fine on my Rocky 10 build, no apparent issues thus far, other than the mentioned screen resizing one. The only thing I had to do besides turning off drdyvnc was to create an .xsession file in my user's homedir to run Again, awesome job! |
|
Thanks for that @lcniel I've not gotten to the bottom of the resize yet. The resize state machine triggers, but then labwc resizes back to a smaller size. I've added some (untested) code to enable logging for labwc and wayvnc which might help to debug this. |
It's weird. I had some occasional wonkiness with interactive resizing with TurboVNC when connected to WayVNC (like the configuration resetting once connected, I had to change it away from server-side resizing every time, but that might just be some RFB implementation thing), but most of the time it worked without issue. |
matt335672
force-pushed
the
wayland_prototyping
branch
2 times, most recently
from
0cd609c to
6edd861
Compare
|
I'm fairly confident the resize issue is an xrdp problem. Resizing is a complicated dance involving the client and server. wayvnc is using an additional message in its ExtendedDesktopSize response which essentially informs xrdp the resize request has been forwarded. We don't cope well with that message. I suspect what's happening is the resize to the client screen size is happening, and then it is getting overridden when the server reports its original screen size late. I'm not completely sure about this, but I'm sure it's a fixable problem. I'm a little busy IRL for the next week or so, but I'll get to it when I can. Debug logging for both labwc and wayvnc can now be enabled in sesman.ini. |
I had a look at the neatvnc source code: https://github.com/any1/neatvnc/blob/4962e0af5d550b2c4fd34c79dbdca543f233d87c/src/server.c#L1632 and that looks like exactly what happens, yeah (and it makes perfect sense architecture-wise). It looks like there's no code in xrdp for handling this at all and xrdp only ever checks that the response code is 0 (if I am parsing its logic correctly) and otherwise calls failure. So I think another case needs to be added here to capture request forwarded? |
|
I think you're right, from the debugging I've done. I also need a timer event so can pick up if the resize doesn't happen. I'm adding that in at the moment, but it's going to be a bit slow for the rest of the week. |
matt335672
force-pushed
the
wayland_prototyping
branch
from
5451d0f to
971bb63
Compare
|
I've added support for the forwarded resize request to VNC. This seems to work pretty well, but occasionally I get a We're going to need the 'forwarded resize request' support in |
matt335672
force-pushed
the
wayland_prototyping
branch
from
971bb63 to
06fd9fd
Compare
|
The required VNC functionality is now merged into devel, and I've rebased this PR. |
Hi, I tried it out, it works as advertised! Nice work! I can confirm though that wayvnc will segfault from time to time. I seem to be able to consistently trigger it by resizing the window below c. 860x720 (extrapolating from my monitor resolution). However, if I directly connect at this resolution, it will connect without crashing. In general this is not consistent with how my client (Remmina) normally behaves when connecting to xrdp. |
|
I think the next step will be for me to move to Trixie, build |
I had a bit of a look but I can't seem to get trace-level debug for wayvnc via the sesman.ini settings. I did get this, by resizing back and forth: From journald: From wayvnc debug (snipped) I also get some tearing from time to time. It looks to me like screen width is off by some (variable?) amount at some points. I think there might be both that and a race condition going on? I can't easily match up logs since the wayvnc ones don't have timestamps. EDIT: Another try. xrdp: wayvnc: Sizes seem to match here and I didn't see the same kind of tearing. But I'm not sure wayvnc's messages should look like that? Should it be sending multiple extended desktop sizes that are then ignored? My vacation starts next week so depending on how things are I might also invest some time into this. At least I can test stuff on RHEL-likes. I don't know if it would be feasible to run valgrind or something to try and catch the segafault. I'm going to guess there's an incorrectly sized buffer or similar at play here. |
|
I'll try with the latest |
The monolithic session module is split into: - A control modile (session.[hc]) - A session base class module (session_base.[hc]) - An X11 specific class, derived from the base class. The session_parameters object gets its own include file, so that the class files do not need to include session.h. This picks up layering violations. The hope is it will be simple to add another labwc-specific class with minimal changes to other modules.
xrdp makes a lot of assumptions that a display will have an integer associated with it. This commit removes that assumption, and allows a display (or wayland display) to be either something like ":10" (for X11 display ten), or "wayland-1" (for wayland display 1).
Make it clear that the display number passed to auth_start_session() is for X11 only.
Add a way to create debug logs for labwc and wayvnc
matt335672
force-pushed
the
wayland_prototyping
branch
from
06fd9fd to
5818fce
Compare
|
Force-push to incorporate #3596 and #3597 I've put this onto Trixie now. Running with the stock wayvnc was identical to Xubuntu 25-04 in terms of results. I've now rebuilt wayvnc from source:- Using the devel version seems a lot more solid and I've not crashed it yet with resizing. I'll try to work out if a recent change to one of the three products above has fixed the resize issue. |
|
I've tried rolling back to : The change is a few fixes to the https://github.com/any1/neatvnc/releases/tag/v0.9.2 That seems pretty solid too. I've rebased on wayvnc 0.9.1 / neatvnc 0.9.5, and I'll continue testing with that for now. |
I tested with wayvnc 0.9.1, neatvnc 0.9.5, and a built-from-source aml 0.3.0 (all very simple on Rocky 10, it's enough to just download source RPM:s and replace the version numbers before Very promising, all of this! |
|
Quick status update; I'm still plodding away with wayvnc and 'resize on connect'. It works a lot of the time, but occasionally:-
|
I've been sick and on vacation so I haven't tested much, but I guess your wayvnc PR was the fix to 1. Does it also fix 2 or is that an open issue? I haven't been able to trigger it, but I can test some more. |
|
Hi @lcniel The PR fixes both issues. My first effort didn't! HTH |
|
I'm going to pick out the bits of this for X11/Wayland independence and merge them into devel. Then I'll re-work and rebase this and see where we are. It's likely to take a while. |
Just give me a shoutout if you want anything manually tested. As it happens, I am working on a draft PR for an unrelated matter so I have my environments ready-to-go. |
|
Thanks. There might be quite a bit of informal testing to do on the display independence change, even though it won't add any functionality. |
|
Not sure how far along you are on this, or if this is even useful at all, but I am working on putting something based on this branch into trial prod, and in order to fix some segfaults from sesexec I had to add some NULL guards. Maybe this will help someone else who is looking at this, at any rate. Fixes segfault on trying to start Xorg under at least some circumstances (initialization race): --- a/sesman/sesexec/session.c
+++ b/sesman/sesexec/session.c
@@ +158,6 -158,9 @@ session_process_sigchld_event(struct session_data *self)
unsigned int
session_active(const struct session_data *self)
{
+ if (self == NULL) {
+ return 0;
+ }
return self->vtable->active_processes(self);
}Fixes segfault on closing labwc session which presumably would also occur with x11 session (I did this one first, so it's semi-redundant with the other one): --- a/sesman/sesexec/sesexec.c
+++ b/sesman/sesexec/sesexec.c
@@ -317,11 +317,13 @@ sesexec_main_loop_cleanup(void)
/* Don't allow sesexec to terminate with an active
session, as we can't connect to such a session */
- if (session_active(g_session_data))
- {
- LOG(LOG_LEVEL_INFO,
- "Stopping session on xrdp-sesexec exit");
- session_send_term(g_session_data, 1);
+ if (g_session_data != NULL) {
+ if (session_active(g_session_data))
+ {
+ LOG(LOG_LEVEL_INFO,
+ "Stopping session on xrdp-sesexec exit");
+ session_send_term(g_session_data, 1);
+ }
}
session_data_free(g_session_data);
}I tested this with Also, for multi-session to work correctly with respect to wayvnc, a distinct control socket is needed: /* Get the VNC socket name */
g_snprintf(sockname, sizeof(sockname), XRDP_X11RDP_STR,
login_info->uid, baseobj->display);
g_snprintf(ctlname, sizeof(ctlname), "%s-%s", sockname, "ctl");
if (g_file_exist(sockname))
{
(void)g_file_delete(sockname);
}
if (g_file_exist(ctlname))
{
(void)g_file_delete(ctlname);
}
struct list *params = list_create();
if (params == NULL ||
(!(params->auto_free = 1)) || // Just set 'auto_free' inline
!list_add_strdup_multi(params,
wayvnc,
"-u",
sockname,
"-S",
ctlname,
NULL)) |
|
Thanks @lcniel Can you add a bit more detail around the description of the wayvnc control socket, and why it is needed? I don't fully understand that. |
Suppose a single user starts multiple distinct sessions on the same server (yes, once again...), then wayvnc wants a distinct control socket for each instance (which, well, makes sense). I found out the hard way that it doesn't automatically just create a new control socket, you need to explicitly tell it to do that. At least in my environment. |
|
I have a comment and a question based on pretty extensive stress-testing in "unstable production". All tests here are done using RFX over the GFX pipeline. First, the comment: There's a non-trivial performance toll for xrdp itself compared to when running an xorgxrdp session, which I assume is down to the VNC encoding handling. When I do a stress test, like a video stream using labwc-vnc at a high enough resolution (like 1980x1080) xrdp will hit 100% CPU usage, at which point it will become unresponsive. It seems like input is just dropped in this state. If I do the same thing over xorgxrdp, then xrdp CPU usage peaks around 85% and the input remains responsive (potentially at degraded quality, but I can still interact with the scene). Similarly, if I do the same thing with wayvnc, it may start to aggressively drop frames depending on my internet connection, but it remains responsive. I would generally expect worse performance with the wayvnc shim approach than with xorgxrdp, but the dropping of input is a bit annoying as a failure mode. I'm not sure if there's a way to fix that, though, as it might be related to how xrdp interfaces with the input handling of wayvnc, I haven't tested if X11 VNC behaves the same if using that as a backend. The question I have is if display quality levels for RFX over GFX is meant to be supported with a VNC shim, or if it's meant to be lossless stream-only. I have trouble getting anything except what looks like perfectly lossless display quality, I thought xrdp might do some kind of buffering and compression of VNC frames but maybe it doesn't? I don't really have much experience with VNC as an xrdp backend and I'm not sure if this is how it usually works. Before I turn on development logging and see if there is some specific issue with my build it would be good to know if this is expected to work or not. I tried to figure out how the display buffer is dealt with internally but I couldn't quite figure it out. |
|
Thanks @lcniel Because we're using VNC, we should just drop frames if we can't keep up. We shouldn't be dropping input though. As regards the quality of this setup, it's will be lossless. To get lossy compression, you still need to go via |
2 participants
See #2637 (pinned issue) for the background on this.
This is more a show-and-tell than a PR at the moment. I though I'd put it out there to get some comments though. If we get somewhere useful with this I'll refactor a different PR for a merge.
First a couple of screenshots. This is a remmina client talking to an Xunutu25-04 backend. I've tested nothing else yet:-
Session is XFCE 4.20 running under the labwc compositor.
Working:-
Not working:-
There are some code problems too:-
The biggest issue to my mind is with X11 display numbers which are used extensively in xrdp. For X11 these are globals, and (effectively) always integers. Wayland displays are always strings. So the socket dir for a running session looks like this:-
I've tried to keep compatibility with number for X11, but this is distorting the code in places. I think it might be better to use strings like
x11-10(e.g. for X11 display 10) rather than a straight10. This will need a change to the audio drivers, but we can stick with the plain number for xorgxrdp support.I'm quite busy IRL for a week or so. After that, I'll start work on fixing deficiencies and trying other platforms, in particular Trixie which is now out.
Comments/questions are very welcome!