Add screen cast portal #122

jadahl · 2017-09-19T13:00:41Z

This adds a screen cast portal, provided via org.freedesktop.portal.ScreenCast. A sample user illustrating how its used can be found here: https://gitlab.gnome.org/snippets/19 . A xdg-desktop-portal-gtk implementation (needing mutter 3.26 + https://bugzilla.gnome.org/show_bug.cgi?id=787715 ): https://github.com/jadahl/xdg-desktop-portal-gtk/commits/wip/screen-cast .

There are some things to note about it, and as is, it is not ready to be merged. More on that below.

This is, as far as I can tell, the first portal API that introduces long lived sessions. I need this so that I can later couple a screen cast session with a remote desktop session. To summarize, doing remote desktop might (most likely) involve screen casting, but we should not pop up two portal dialogs; thus we need a combined "Start" switch.

Sessions here are currently associated by passing a session ID that the application retrieves from the Result vardict from a "CreateSession" method. This is then passed to every method that depend on a given session (SelectSources, Start, Stop).

The "combined" sessions will be done by having the application first create a org.freedesktop.portal.RemoteDesktop session (in the same way as is done in the commit introducing the ScreenCast in this PR). The session ID is then passed as an option when creating the org.freedesktop.portal.ScreenCast session. This will mean that the screen cast session is controlled by the remote desktop session, which in practice means that the application Starts/Stops both sessions atomically using the remote desktop session. Vardict entries from the screen cast Start method will instead be passed via the Result object of the remote desktop Start method.

This requires PipeWire, which is not yet possible to be used from inside a flatpak sandbox. As the idea is to have the portal pass the PipeWire remote file descriptor to the client, this portal API naturally must be changed to also do this. The intention here is to first get feedback on the portal API itself, then make the needed adaptations for passing a file descriptor.

alexlarsson · 2017-09-21T08:57:20Z

The handling of the sessions is not threadsafe. You can have parallel callers of screencast methods, closing of request handles or disappearance of dbus names trigger code, but nothing protects the sessions hash table. The data in that hashtable is never returned from check_session though, so I think all you need is to wrap a mutex around that and where things are added/removed from the hash.

Also, I think the API is a bit weird wrt session lifetimes, or at least not cleanly specified. The common scenario will be the client:

create session
select sources
start
stop (which terminatest the session)

But this leaves a couple of questions for other scenarios:

What happens if the client wishes to terminate the session but it was not started. For instance, maybe the start call fails, or the user closed the window before the screencasting was started.
Can i reuse one session for multiple screencasts
What happens if i call start multiple times, or stop before start. Or change the sources while a screencast is active.
What happens if the app creates a session, then dies before it started the recording, but the portal still calls force_stop()

I think you should make session close an explicit operation (and make owner disappearing from bus the same operation as this). You also need to track the state of the screencase (is it active or not, etc) and return errors to the client if its in the wrong state (double start, stop before start, etc). This will validate stuff in the common code so that the portal implementations don't have to care about weird cases. Additionally, it will make it possible for the session close/client exit to automatically do the right thing (stop only if active). Also, you need to decide if multiple start/stop pairs can be done on a single session (and if not return errors if this happens).

jadahl · 2017-12-01T04:02:13Z

The new branch contains the new reworked API, which means "sessions" are outsourced to a generic org.freedesktop.portal.Session API. Another change is that I made it a requirement to call "SelectSources()" before "Start()". The rest is more about clarifications on what may be called when etc.

matthiasclasen · 2017-12-12T03:36:54Z

data/org.freedesktop.impl.portal.ScreenCast.xml

+            <term>streams a(ua{sv})</term>
+            <listitem><para>
+              An array of PipeWire streams. Each stream consists of a PipeWire
+              node ID (the first element in the tuble, and a Vardict of


s/tuble/tuple/

matthiasclasen · 2017-12-12T03:42:08Z

data/org.freedesktop.portal.ScreenCast.xml

+            <term>streams a(ua{sv})</term>
+            <listitem><para>
+              An array of PipeWire streams. Each stream consists of a PipeWire
+              node ID (the first element in the tuble, and a Vardict of


s/tuble/tuple/

matthiasclasen · 2017-12-12T03:42:50Z

data/org.freedesktop.portal.ScreenCast.xml

+            <listitem><para>
+              An array of PipeWire streams. Each stream consists of a PipeWire
+              node ID (the first element in the tuble, and a Vardict of
+              properties.


Is it the case that the array only has one member unless 'multiple' is true ? might be worth pointing out in the docs.

Also, does the node id give you some kind of usable for-display identifier for the multiple streams if you were to show the multiple streams in the UI, or should that be in the dbus API?

I think it'd make sense to add things to the vardict that comes with each stream. The idea is that we can add things as we need them. I guess a 'human readable name' ala Built in display or Dell 24" might be useful.

matthiasclasen · 2017-12-12T04:01:45Z

Any update on when the final pipewire api will be available ?

alexlarsson · 2017-12-19T08:43:00Z

src/session.c

+
+  g_dbus_interface_skeleton_set_flags (G_DBUS_INTERFACE_SKELETON (session),
+                                       G_DBUS_INTERFACE_SKELETON_FLAGS_HANDLE_METHOD_INVOCATIONS_IN_THREAD);
+


I think you should add a g-authorize-method callback here that only allows callers where the sender matches the sender part of the object path

jadahl · 2017-12-19T09:06:45Z

Any update on when the final pipewire api will be available ?

Wim added some kind of permission system in PipeWire/pipewire@d5643e8 a few days ago but I have yet to see whether the portal can make use of it.

alexlarsson · 2017-12-19T09:30:00Z

I had a look over this, and it seems ok to me. One thing I don't quite understand though is how the permissions are going to work under wayland? I mean, the protocol that the portal implementation uses to get the data needs some kind of authentication to not just break all the privacy between wayland clients (sandboxed or not).

jadahl · 2017-12-19T09:49:45Z

I had a look over this, and it seems ok to me. One thing I don't quite understand though is how the permissions are going to work under wayland? I mean, the protocol that the portal implementation uses to get the data needs some kind of authentication to not just break all the privacy between wayland clients (sandboxed or not).

The idea is that the flow should be something like this:

A sandboxed app requests a screen cast
The portal shows the UI and the user approves
The portal tells mutter to open a PipeWire stream which it does and hands over a stream node id
The portal opens a PipeWire remote, "hides" all nodes except the ones just opened by mutter
The portal "seals" (does something so that no further permission changes can be done) the PipeWire remote
The portal will hand over a file descriptor with the open sealed pw_remote to the app
The app opens its pw_remote using the file descriptor, and should now with this handle only be able to see the streams that was opened exactly for the screen cast session it received the pw_remote fd from.

jadahl · 2017-12-19T09:59:46Z

Or what do you mean with data needed? The portal implementation is outside of the sandbox; the apps should only see the communication between the sandox and the non-impl side of just the channel between the portal and the app, right? I mean, two separate apps will not see the portal communication done between the other app and the portal, right? Where do you see any privacy being breached?

alexlarsson · 2018-01-09T14:08:58Z

Ah, the thing I was missing was the "sealing". I.e. what makes the returned fd different from just a connection to the entire pipewire graph.

matthiasclasen · 2018-01-09T14:30:36Z

We are still waiting for pipewire permission api to be integrated here before this can be merged, correct ?

jadahl · 2018-01-09T14:32:23Z

We are still waiting for pipewire permission api to be integrated here before this can be merged, correct ?

Right. I talked to Wim earlier and can move forward with the API currently on the PipeWire master branch.

jadahl · 2018-01-23T10:44:20Z

This new branch needs PipeWire checked out from master from yesterday (22nd Jan), preferably at least commit f2aafffb2c68a023fd20b08451de20d1e2808fb3 to make the test python app able to work. It also needs an updated mutter; I'll provide a link to the corresponding branch after I cleaned it up a bit.

alexlarsson · 2018-01-23T12:52:22Z

data/org.freedesktop.impl.portal.Session.xml

+      The portal can abort the interaction by calling
+      org.freedesktop.impl.portal.Session.Close() on the Session object.
+  -->
+  <interface name="org.freedesktop.impl.portal.Session">


Can you add a version property here:

<property name="version" type="u" access="read"/>

That way we can extend this api later and do runtime checks on it.

alexlarsson · 2018-01-23T12:52:55Z

data/org.freedesktop.portal.Session.xml

+      closing all active sessions made by said client.
+  -->
+  <interface name="org.freedesktop.portal.Session">
+


version here too

alexlarsson · 2018-01-23T13:07:05Z

data/org.freedesktop.portal.ScreenCast.xml

+      @short_description: Screen cast portal
+  -->
+  <interface name="org.freedesktop.portal.ScreenCast">
+    <!--


Want a version property here too, and on the impl.

Oh, i see you have one at the end.

alexlarsson · 2018-01-23T13:08:19Z

data/org.freedesktop.portal.ScreenCast.xml

+        This function can only be called once, if there was one or more stream
+        shared in the response to starting the session.
+    -->
+    <method name="TakePipeWireRemote">


I think you need:

<annotation name="org.gtk.GDBus.C.UnixFD" value="true"/>

here, so that you get the right codegen for the fds.

Oh, i see you did that in the makefile. We're changing those to inline annotations instead.

alexlarsson · 2018-01-23T13:20:17Z

src/screen-cast.c

+
+  fd_list = g_unix_fd_list_new ();
+  fd_id = g_unix_fd_list_append (fd_list,
+                                 pw_remote_steal_fd (remote->remote),


I believe this leaks the fd. fd_list_append() dups the passed in fd and keeps that.

alexlarsson · 2018-01-23T13:21:29Z

Overall this looks good to me, commented on some things.

Add two new D-Bus interfaces: org.freedesktop.portal.Session and org.freedesktop.impl.portal.Session. These two interfaces may be used by portal APIs that want long lived sessions. Each contain a Close method and a Closed signal, but ultimately its the interface that makes use of the Session concept that specifies exactly how the method and signal may be used. In general, a session may always be closed by the portal, for example if the session owner vanished, so an API must always take this into consideration.

A 'Call' object is like a Request but without a reply, thus without any D-Bus object that sends signals or have methods. It is meant for simple method calls without any Request object carrying a reply.

jadahl · 2018-01-24T08:34:01Z

data/org.freedesktop.portal.ScreenCast.xml

+        This function can only be called once, if there was one or more stream
+        shared in the response to starting the session.
+    -->
+    <method name="TakePipeWireRemote">


I will change this to "OpenPipeWireRemote", and allow it to be called multiple times, having the portal open the PipeWire remote on demand. The reason is that this simplifies app-side by allowing to pass one pw_remote file descriptor per gstreamer object. The alternative would be to add ref-counting to pw_remote and teach the gstreamer things to share.

Introduce a portal enabling applications to create PipeWire based screen cast streams. The application facing API is defined by org.freedesktop.portal.ScreenCast and uses org.freedesktop.portal.Session for keeping track of screen cast sessions. The portal depends on having PipeWire present on the host system, as xdg-desktop-portal will link against it, open a PipeWire remote on behalf of the client, set up permission, then hand over the remote as a file descriptor to the application interacting with the portal.

Add print preview

jadahl force-pushed the wip/screen-cast branch from e59227d to 9f768ed Compare November 29, 2017 11:06

matthiasclasen requested changes Dec 12, 2017

View reviewed changes

alexlarsson reviewed Dec 19, 2017

View reviewed changes

jadahl force-pushed the wip/screen-cast branch from 9f768ed to b4a1026 Compare January 23, 2018 10:40

jadahl mentioned this pull request Jan 23, 2018

Add remote desktop portal #145

Merged

alexlarsson reviewed Jan 23, 2018

View reviewed changes

jadahl added 2 commits January 24, 2018 14:55

Add 'Call' object similar to Request

085f3f5

A 'Call' object is like a Request but without a reply, thus without any D-Bus object that sends signals or have methods. It is meant for simple method calls without any Request object carrying a reply.

jadahl force-pushed the wip/screen-cast branch 2 times, most recently from 8da45c5 to da8ba44 Compare January 24, 2018 07:14

jadahl changed the title ~~[RFC] Add screen cast portal~~ Add screen cast portal Jan 24, 2018

jadahl commented Jan 24, 2018

View reviewed changes

jadahl force-pushed the wip/screen-cast branch from da8ba44 to a69be55 Compare January 24, 2018 11:04

matthiasclasen approved these changes Jan 24, 2018

View reviewed changes

matthiasclasen merged commit 10a7816 into flatpak:master Jan 24, 2018

muelli mentioned this pull request Jan 24, 2018

Add a webcam portal #38

Closed

phw mentioned this pull request Jan 25, 2018

Investigate the use of pipewire for screen recording phw/peek#231

Open

TheFrozenFire mentioned this pull request Jul 25, 2019

Wayland platform not available flathub/us.zoom.Zoom#24

Open

agx pushed a commit to agx/xdg-desktop-portal that referenced this pull request Dec 20, 2023

Merge pull request flatpak#122 from matthiasclasen/print-preview

c960ad1

Add print preview

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add screen cast portal #122

Add screen cast portal #122

jadahl commented Sep 19, 2017 •

edited

alexlarsson commented Sep 21, 2017

jadahl commented Dec 1, 2017

matthiasclasen Dec 12, 2017

matthiasclasen Dec 12, 2017

matthiasclasen Dec 12, 2017

alexlarsson Dec 19, 2017

jadahl Dec 19, 2017

matthiasclasen commented Dec 12, 2017

alexlarsson Dec 19, 2017

jadahl commented Dec 19, 2017

alexlarsson commented Dec 19, 2017

jadahl commented Dec 19, 2017

jadahl commented Dec 19, 2017

alexlarsson commented Jan 9, 2018

matthiasclasen commented Jan 9, 2018

jadahl commented Jan 9, 2018

jadahl commented Jan 23, 2018

alexlarsson Jan 23, 2018

alexlarsson Jan 23, 2018

alexlarsson Jan 23, 2018

alexlarsson Jan 23, 2018

alexlarsson Jan 23, 2018

alexlarsson Jan 23, 2018

alexlarsson Jan 23, 2018

alexlarsson commented Jan 23, 2018

jadahl Jan 24, 2018


		g_dbus_interface_skeleton_set_flags (G_DBUS_INTERFACE_SKELETON (session),
		G_DBUS_INTERFACE_SKELETON_FLAGS_HANDLE_METHOD_INVOCATIONS_IN_THREAD);

Add screen cast portal #122

Add screen cast portal #122

Conversation

jadahl commented Sep 19, 2017 • edited

alexlarsson commented Sep 21, 2017

jadahl commented Dec 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthiasclasen commented Dec 12, 2017

Choose a reason for hiding this comment

jadahl commented Dec 19, 2017

alexlarsson commented Dec 19, 2017

jadahl commented Dec 19, 2017

jadahl commented Dec 19, 2017

alexlarsson commented Jan 9, 2018

matthiasclasen commented Jan 9, 2018

jadahl commented Jan 9, 2018

jadahl commented Jan 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexlarsson commented Jan 23, 2018

Choose a reason for hiding this comment

jadahl commented Sep 19, 2017 •

edited