Fix xdg portal verison check for persist_mode #6742

linuxrider · 2023-12-23T09:35:21Z

fixes #6741 on Fedora 39.

version check mixed up ScreenCast and RemoteDesktop which added persist_mode and restore_token on versions 4 and 2, respectively.
injection of persist_mode and restore_token happened in a too late step of setting up the session

rustdesk · 2023-12-23T09:42:44Z

linuxrider · 2023-12-23T10:17:19Z

Relevant API description: https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.RemoteDesktop.html#org-freedesktop-portal-remotedesktop-selectdevices

rustdesk · 2023-12-24T08:44:38Z

@linuxrider have you tested this fixes your persist_mode issue? We tested, it does not fix the issue.

linuxrider · 2023-12-24T09:19:09Z

I have tested it for Fedora 39 Gnome 45 on wayland. With this the persistent checkbox is present in the source selection portal dialog. Furthermore after accepting with persistence option, closing rustdesk and reopening no further dialog is shown upon reconnecting and remote control works.

This is the only procedure that works for me. I have tried several other procedures. E.g. adding the restore_token in on_create_session_response, on_select_devices_response and on_select_sources_response or only the first two. But then it crashed and I got the message on the rustdesk client X11 expected.

Documentation of the portals is not that clear for me. I think this is how it is supposed to work:
The chain of action is RemoteDesktop.CreateSession -> RemoteDesktop.SelectDevices ("is called" in on_create_session_response, here restore_token needs to be provided ) -> ScreenCast.SelectSources (persistence managed by RemoteDesktop ) -> RemoteDesktop.Start (returns new token).

The RemoteDesktop portal added persist_mode on version 2 and it is stated that persistence is managed by RemoteDesktop rather than ScreenCast portal https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.ScreenCast.html#org-freedesktop-portal-screencast-selectsources

rustdesk · 2023-12-24T09:21:14Z

Thanks @linuxrider

@fufesou

linuxrider · 2023-12-24T09:31:06Z

It works also with the nightly flatpak.
I am writing this in a unattended remote session.
Really glad that it works now. Thank you.

fufesou · 2023-12-24T10:13:51Z

~~There're two problems of this PR.~~

~~It does not fix the "restore_token" issue.~~.
persist_mode is supported until version 4 for org.freedesktop.portal.ScreenCast. https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.ScreenCast.html#:~:text=of%20this%20interface.-,persist_mode,-(u)

#6742 (comment)

1.2.3

1.2.3.mp4

master commit and this branch

persist_mode.mp4

linuxrider · 2023-12-24T11:02:51Z

So the version checking needs to be more fine grained and in more places to cover Fedora 39 and Ubuntu 22.04.
Your point 2: Does this mean that persistence handling is done in ScreenCast.SelectSources for portal versions < 4?

fufesou · 2023-12-24T11:16:07Z

I don't know how portal handle it under version 4. I'm also testing.

Another thing is the sharing icon persists after the connection end.

In 1.2.3, the sharing icon disappears after the connection.

fufesou · 2023-12-24T11:17:29Z

So the version checking needs to be more fine grained and in more places to cover Fedora 39 and Ubuntu 22.04.

Does this PR work fine in Fedora 39?

I've tested Fedora 38, the same behavior to Ubuntu 22.04.

linuxrider · 2023-12-24T11:18:36Z

@fufesou I have adapted the version checks and it still works for Fedora 39.
Does this work in your tests, too?
https://github.com/linuxrider/rustdesk/tree/persist_mode

linuxrider · 2023-12-24T11:21:34Z

I don't know how portal handle it under version 4. I'm also testing.

Another thing is the sharing icon persists after the connection end.

In 1.2.3, the sharing icon disappears after the connection.

The icon staying in the system tray after connection ends is also present in Fedora 39.

fufesou · 2023-12-24T11:24:51Z

The icon staying in the system tray after connection ends is also present in Fedora 39.

So your test may be invalid.

You need to run sudo systemctl restart rustdesk after the connection.

Or you can stop rustdesk service, do the following steps:

sudo systemctl stop rustdesk
pkill rustdesk
rustdesk
connect
stop rustdesk
rustdesk
connect

linuxrider · 2023-12-24T14:10:09Z

I did the test without installation of the compiled executable directly and with the flatpak from github. Closing the flatpak removes rustdesk from memory. I also did a reboot and the session still persisted.
Here it the flatpak to flatpak Fedora 39 test.
Bildschirmaufzeichnung vom 2023-12-24, 14-58-42.webm

I could not get the rpm nor the deb to work properly in the vm.
Will try once more.

fufesou · 2023-12-24T14:24:44Z

@linuxrider I've also tested with Fedora 39. This PR works fine.

It was my fault.

The versions of org.freedesktop.portal.RemoteDesktop and org.freedesktop.portal.ScreenCast are different.

Your PR is good. 👍

Version	org.freedesktop.portal.RemoteDesktop	org.freedesktop.portal.ScreenCast
Ubuntu 22.04	1	4
Fedora39	2	5

fufesou · 2023-12-24T14:39:02Z

@sahilyeole FYI.

There will be compatibility issues here. persist_mode is only supported when org.freedesktop.portal.RemoteDesktop version >= 2. The latest Fedora39 is the minimum version that can meet this requirement.

And there's one left strange thing. Even after the connection ends, the shared state persists.
It is introduced in #6675 . Maybe we can improve it.

sahilyeole · 2023-12-24T14:57:16Z

Sure, I will work on it soon.

sahilyeole · 2023-12-24T15:10:41Z

@linuxrider @fufesou
Should we use persist_mode for select_sources as well?
Is it needed there too?

we can add a check there like screencast_portal::version(&portal) >= 4

linuxrider · 2023-12-24T22:13:41Z

@sahilyeole For org.freedesktop.portal.RemoteDesktop.version >= 2 it is enough to set persist_mode fororg.freedesktop.portal.RemoteDesktop.SelectDevices. It does not work if it is also set in SelectSources. Persistence is managed in org.freedesktop.portal.RemoteDesktop (https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.ScreenCast.html).
For org.freedesktop.portal.RemoteDesktop.version < 2 it is a different story and I don't understand how it works exactly.

fufesou · 2023-12-25T01:24:52Z

we can add a check there like screencast_portal::version(&portal) >= 4

@sahilyeole I'm not sure if the restore token can be used for both RemoteDesktop and ScreenCast.

Maybe we can use "RemoteDesktop" and "uinput" together.

For org.freedesktop.portal.RemoteDesktop.version < 2 and org.freedesktop.portal.ScreenCast.version >= 4 and non-flatpak, use uinput as the input method.

Otherwise, use "RemoteDesktop" as the input method.

It's a little complicated.

What do you think?

sahilyeole · 2023-12-25T10:57:56Z

Currently, we are only using rdp input when uinput is not available, like on flatpak.
On normal wayland we are still using uinput.
One possible solution I can think of to maintain persistence same as before my PR for normal wayland:
Skip org.freedesktop.portal.RemoteDesktop.SelectDevices in the callback chain when uinput is already available. Use persist_mode for select_sources.
This way the function request_remote_desktop will work the same as it was before my PR only when uinput is already available (on normal wayland), also same persistence as before.

But the need for such implementation comes down to how much of a big problem the unavailability of persist_mode is because during my testing I didn't see any problem so I'm not sure.
Maybe persistence for select_devices is not needed because we are already not using rdp input for normal wayland.
Let me know if we should proceed with it.

fufesou · 2023-12-25T12:36:32Z

One possible solution I can think of to maintain persistence same as before my PR for normal wayland:
Skip org.freedesktop.portal.RemoteDesktop.SelectDevices in the callback chain when uinput is already available. Use persist_mode for select_sources.

Agree. I've also confirmed with @rustdesk

sahilyeole · 2023-12-27T00:54:14Z

After this PR wayland arch linux (with version 2) as remote is not working. It asks for input permission but the connection is not made.
It might not be stable to use yet, not sure.
So should I comment the part added in this PR for flatpak? Because for normal wayland I'm turning back pipewire.rs code to how it was before #6675 anyway (with persist_mode in select_sources).
Also, version 2 is currently only available in latest releases that too fail on arch, not tested fedora.

rustdesk · 2023-12-27T03:26:20Z

After this PR wayland arch linux (with version 2) as remote is not working

I will revert this PR first.

This reverts commit 57acadd.

rustdesk · 2023-12-27T03:29:47Z

Reverted

fix xdg portal verison check for persist_mode

54fa915

rustdesk merged commit 57acadd into rustdesk:master Dec 24, 2023

rustdesk added a commit that referenced this pull request Dec 27, 2023

Revert "fix xdg portal verison check for persist_mode (#6742)"

be9cc47

This reverts commit 57acadd.

rustdesk mentioned this pull request Dec 27, 2023

Revert "Fix xdg portal verison check for persist_mode" #6755

Merged

rustdesk added a commit that referenced this pull request Dec 27, 2023

Revert "fix xdg portal verison check for persist_mode (#6742)" (#6755)

4b581c3

This reverts commit 57acadd.

sahilyeole mentioned this pull request Dec 27, 2023

Wayland: Skip remote desktop portal calls when uinput is available #6758

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix xdg portal verison check for persist_mode #6742

Fix xdg portal verison check for persist_mode #6742

linuxrider commented Dec 23, 2023

rustdesk commented Dec 23, 2023

linuxrider commented Dec 23, 2023

rustdesk commented Dec 24, 2023 •

edited

linuxrider commented Dec 24, 2023

rustdesk commented Dec 24, 2023

linuxrider commented Dec 24, 2023 •

edited

fufesou commented Dec 24, 2023 •

edited

linuxrider commented Dec 24, 2023

fufesou commented Dec 24, 2023

fufesou commented Dec 24, 2023

linuxrider commented Dec 24, 2023

linuxrider commented Dec 24, 2023

fufesou commented Dec 24, 2023

linuxrider commented Dec 24, 2023

fufesou commented Dec 24, 2023 •

edited

fufesou commented Dec 24, 2023 •

edited

sahilyeole commented Dec 24, 2023

sahilyeole commented Dec 24, 2023

linuxrider commented Dec 24, 2023

fufesou commented Dec 25, 2023 •

edited

sahilyeole commented Dec 25, 2023 •

edited

fufesou commented Dec 25, 2023

sahilyeole commented Dec 27, 2023 •

edited

rustdesk commented Dec 27, 2023 •

edited

rustdesk commented Dec 27, 2023

Fix xdg portal verison check for persist_mode #6742

Fix xdg portal verison check for persist_mode #6742

Conversation

linuxrider commented Dec 23, 2023

rustdesk commented Dec 23, 2023

linuxrider commented Dec 23, 2023

rustdesk commented Dec 24, 2023 • edited

linuxrider commented Dec 24, 2023

rustdesk commented Dec 24, 2023

linuxrider commented Dec 24, 2023 • edited

fufesou commented Dec 24, 2023 • edited

1.2.3

master commit and this branch

linuxrider commented Dec 24, 2023

fufesou commented Dec 24, 2023

fufesou commented Dec 24, 2023

linuxrider commented Dec 24, 2023

linuxrider commented Dec 24, 2023

fufesou commented Dec 24, 2023

linuxrider commented Dec 24, 2023

fufesou commented Dec 24, 2023 • edited

fufesou commented Dec 24, 2023 • edited

sahilyeole commented Dec 24, 2023

sahilyeole commented Dec 24, 2023

linuxrider commented Dec 24, 2023

fufesou commented Dec 25, 2023 • edited

sahilyeole commented Dec 25, 2023 • edited

fufesou commented Dec 25, 2023

sahilyeole commented Dec 27, 2023 • edited

rustdesk commented Dec 27, 2023 • edited

rustdesk commented Dec 27, 2023

rustdesk commented Dec 24, 2023 •

edited

linuxrider commented Dec 24, 2023 •

edited

fufesou commented Dec 24, 2023 •

edited

fufesou commented Dec 24, 2023 •

edited

fufesou commented Dec 24, 2023 •

edited

fufesou commented Dec 25, 2023 •

edited

sahilyeole commented Dec 25, 2023 •

edited

sahilyeole commented Dec 27, 2023 •

edited

rustdesk commented Dec 27, 2023 •

edited