[PATCH 0/2] Fix gnome-shell screenrecorder

Hello Guix,

Last week a user in the irc chat remarked that the built in Gnome screenrecorder is not available which rekado and me then investigated afterwards.

This functionality is provided by the org.gnome.Shell.Screencast dbus service which currently fails to start (if for example you try to investigate it with d-spy you get the not really helpful error: GDBus.Error:org.freedesktop.DBus.Error.Spawn.ChildExited: Process org.gnome.Shell.Screencast exited with status 1)

We later figured that this error happens because the required typelib files cannot be found.

As a fix I imported a patch from NixOS [1] from their gnome-shell 42/44 package and wrapped the relevant files with GI_TYPELIB_PATH. This seems to be strictly necessary only for the screencast service however I wrapped all of the dbus services listed in the js/dbusServices/meson.build file, same as what is done in NixOS.

In order for Screencast to work you'll also need pipewire available (I think for the gstreamer plug-in) which is why I added it to gnome-essential-extras in the second patch.

You'll also need pipewire running (i.e the home-service) which makes this a bit unintuitive for users.

I don't think it is currently possible to enable user level shepherd services from the system configuration, but correct me if I am wrong. Without pipewire running the screen recording stops immediately.

As usual i am not sure if the formating/wording of my commit messages and the name I gave the patch are fine.

Also thanks to rekado for helping to make sense of this.

[1] https://github.com/NixOS/nixpkgs/blob/nixos-23.05/pkgs/desktops/gnome/core/gnome-shell/wrap-services.patch

Dariqq (2):

gnu: gnome-shell: Wrap d-bus services.

gnu: gnome-essential-extras: Propagate pipewire.

gnu/local.mk | 1 +

gnu/packages/gnome.scm | 14 ++++-

.../gnome-shell-wrappable-dbus-services.patch | 59 +++++++++++++++++++

3 files changed, 72 insertions(+), 2 deletions(-)

create mode 100644 gnu/packages/patches/gnome-shell-wrappable-dbus-services.patch

base-commit: cd45294d576975a3bff2f755764a3f46f09ea6f9

--

2.41.0

Some of the dbus services can't find typelibs.

Change-Id: I2c31bf1bd92e281b86c57b06988c6a3793a58d40

---

gnu/local.mk | 1 +

gnu/packages/gnome.scm | 13 +++-

.../gnome-shell-wrappable-dbus-services.patch | 59 +++++++++++++++++++

3 files changed, 71 insertions(+), 2 deletions(-)

create mode 100644 gnu/packages/patches/gnome-shell-wrappable-dbus-services.patch

The built-in gnome-shell screencast requries pipewire.

Change-Id: I6ca5d309f1e6a5f3396b35cfda6d2d470d5b62be

---

gnu/packages/gnome.scm | 1 +

1 file changed, 1 insertion(+)

Am Montag, dem 08.04.2024 um 15:53 +0000 schrieb Dariqq:

Should pipewire not go into gnome-shell or something else? Propagating

it here seems weird.

Am Montag, dem 08.04.2024 um 15:53 +0000 schrieb Dariqq:

You can skip [arguments] and just write [#:phases].

LGTM

Is there no other way to specify the entry point?

Cheers

On 08.04.24 18:49, Liliana Marie Prikler wrote:

I wasn't sure if only the gstreamer plugin of pipewire are required or

something else from the pipewire package too and that was the easy solution.

I just tested adding pipewire to gnome-shell and adding

GST_PLUGIN_SYSTEM_PATH to the wrapper seems to be enough.

While working on this I also found that also gst-plugins-good are also

required for the Screencast service which will get found using the

system profile as it is propagated by gnome-essential-extras. Should

that also be added to gnome-shell?

Hi,

On 08.04.24 18:54, Liliana Marie Prikler wrote:

+---

If it is just about the name in argv[0] what about creating a *-wrapper

instead and leaving the original file untouched rather than the wrapper

replacing the original file?

One quick way would be to rename the wrapper script and original file

after wrap-program is called, fix the path in the wrapper script and

adjust the dbus service file to call the *-wrapper instead.

Have a nice day.

Am Dienstag, dem 09.04.2024 um 15:35 +0000 schrieb Dariqq:

You can possibly use wrap-script instead of wrap-program, so that you

only need a single file. I've also written Javascript code to expand

GI_TYPELIB_PATH in other packages already, perhaps you want to copy

that?

Cheers

On 09.04.24 19:04, Liliana Marie Prikler wrote:

Thanks for the hint with the javascript code. This makes the fix even

easier as the extra patch is no longer required.

But then what to do with pipewire/GST_PLUGIN_SYSTEM_PATH? We would be

back at propagating it somewhere.

Also do you have an idea how to communicate to users to enable the

pipewire home service if they want the screenrecorder to work?

Have a nice day.

Am Mittwoch, dem 10.04.2024 um 08:07 +0000 schrieb Dariqq:

For pipewire I'm not sure what the problem is, really. For

GST_PLUGIN_SYSTEM_PATH you can add a wrapper that amends the

environment variable or possibly call some Gst function – either in

Guile with wrap-script or possibly in Javascript.

Cheers

Hi,

On 10.04.24 20:20, Liliana Marie Prikler wrote:

Unfortunately wrap-script does not work as the file

#$gnome-shell/share/gnome-shell/org.gnome.Shell.Screencast has no

shebang line set and even if it would ( enabled by the extra patch) gjs

is not a valid interpreter for wrap_script and trying to change that

rebuilds everything.

I am currently trying to find out if it is possible to do a similiar

thing as the GI_TYPELIB_PATH with javascript. Or change the environment

variable before through another way. My limited javascript knowledge is

not helping here either.

Have a nice day

The screencast service can now find typelibs and gstreamer plugins.

[inputs]: Add gst-plugins-good and pipewire.

[#:phases]<'wrap-programs>: Create a wrapper for screencast service and use it.

Change-Id: I2c31bf1bd92e281b86c57b06988c6a3793a58d40

---

Hi,

Here is v2 of the patch. The wrapper is now created manually as I was unable to find a way to manipulate the gstreamer plugin load path from within javascript. Importantly this leaves the original javascript file untouched and therefore circumvents the argv[0] problem in v1.

All the required gstreamer plugins (pipewire and gst-plugins-good) are now inputs and are in the wrapper via GST_PLUGIN_SYSTEM_PATH.

I've also left a comment for the required home-pipewire-service.

In gnome-shell 45+ the invocation has changed to 'gjs -m service'. Should this also be a comment somewhere?

gnu/packages/gnome.scm | 37 ++++++++++++++++++++++++++++++++++++-

1 file changed, 36 insertions(+), 1 deletion(-)

Am Freitag, dem 12.04.2024 um 17:26 +0000 schrieb Dariqq:

I'm not sure this fix is sounds. It looks like it misses manual

invocation from the command line, does it not? (Well, it's gnome-

shell, so if you have to invoke it from the command line, things are

broken, but still…) What keeps you from wrapping the environment

variable in Javascript?

Gnome-shell 45+ is currently irrelevant, but we might have to adapt

things once we get there on gnome-team. We do need a better gjs-

patching strategy.

Cheers

On 19.04.24 20:51, Liliana Marie Prikler wrote:

If only the typelib path is set the dbus service now longer greets you

with an error but still fails to start without the gstreamer plugins.

That's why in v1 I was propagating pipewire through gnome-essential-extras.

I am not sure what you mean by manual invocation. The screencast service

should get started via dbus. Nothing manual should be required (other

than enabling the pipewire home service).

What keeps you from wrapping the environment

I was unable to find a way to manipulate the GST_PLUGIN_PATH or specify

the gst search path from within javascript. There is

Gst.Registry().scan_path but this creates a new registry and I don't

think is what we want here. (but I don't know anything about gstreamer

and javascript so I could be wrong here).

I guess the most simple patch for the typelibs would be something like

Am Samstag, dem 20.04.2024 um 10:11 +0000 schrieb Dariqq:

Just what it says on the tin, manually running the "binary" through the

command line. It's not super useful with GNOME, but I think we should

make sure our services work whichever way they could be invoked.

Instead of creating a new Gst.Registry – as per the () here – you could

use gst_registry_get() or whatever the according JS method is to access

the Registry singleton.

The Registry constructor ought to be protected, but idk how GJS handles

that.

Hmm, could you add --gst-plugin-path on the command line?

You can redo the needed logic locally and add a "XXX: Use wrap-script

once it can handle gjs" for example. That's no hard blocker, but it's

not the nicest option either, I admit. Let's try our other options

first.

Cheers

On 20.04.24 12:46, Liliana Marie Prikler wrote:

I've tried adding Gst.Registry.get().scan_path() after the GI_TYPELIB

expansion but I am not sure why it does not work. Stracing the

invocation shows that the pipewire gstreamer plugin is accessed at the

beginning but it is still not available somehow. Probably because of my

lack of knowledge about gstreamer.

I've just found GLib.setenv() to set environment variables for the

process directly from within gjs. The GLib typelib comes from the

gobject-introspection package so this should be available.

From my limited testing it seems to work (at least for the screencast

service). What do you think?

Have a nice day.

Am Sonntag, dem 21.04.2024 um 12:37 +0000 schrieb Dariqq:

Yes, GLib.setenv is the function I had in mind to twiddle the

environment. If you find the right location to do so at the start of

the program, please send a patch that does that.

Cheers

Adjust screencast such that GI_TYPELIB_PATH and GST_PLUGIN_SYSTEM_PATH are set

before starting.

Add all required gstreamer plugins to inputs.

To be able to use it a running pipewire service is needed.

[inputs]: Add gst-plugins-good and pipewire.

[#:phases]<'wrap-programs>: Wrap org.gnome.Shell.Screencast.

Change-Id: I2c31bf1bd92e281b86c57b06988c6a3793a58d40

---

Here is v3 which only uses js to set the environment variables.

Slight difference to before is that GST_PLUGIN_SYSTEM_PATH gets set to specific paths rather than prepending.

I've also tested if GI_TYPELIB_PATH can be set with GLib.setenv but that didn't seem to work,

so I 've used something similiar as in other places.

gnu/packages/gnome.scm | 15 +++++++++++++++

1 file changed, 15 insertions(+)

Hi Liliana,

Were you able to take a look at v3 yet? I've sent it a couple of weeks

ago utilizing Glib.setenv to set the gst plugin path.

Have a nice day.

On 21.04.24 14:43, Liliana Marie Prikler wrote:

Am Sonntag, dem 21.04.2024 um 17:02 +0000 schrieb Dariqq:

Sorry for the late reply. You ought to use GLib.getenv to get the

current value, so that you can add the needed paths as a prefix (or

suffix? I'd personally prefer letting the user prefix them).

Cheers

Adjust screencast such that GI_TYPELIB_PATH and GST_PLUGIN_SYSTEM_PATH are set

before starting.

Add all required gstreamer plugins to inputs.

To be able to use it a running pipewire service is needed.

[inputs]: Add gst-plugins-good and pipewire.

[#:phases]<'wrap-programs>: Wrap org.gnome.Shell.Screencast.

Change-Id: I2c31bf1bd92e281b86c57b06988c6a3793a58d40

---

Here is v4 which appends the gstreamer plugins to GST_PLUGIN_SYSTEM_PATH, handling the case when getenv returns null. I think the difference between appendending or prepending in this case is not too important as the user GST_PLUGIN_PATH should take precedent. The more important thing is that the plugins are in any plugin path at all.

gnu/packages/gnome.scm | 16 ++++++++++++++++

1 file changed, 16 insertions(+)

Hi Dariqq,

Dariqq <dariqq@posteo.net> writes:

Thanks for your efforts improving our GNOME desktop experience!

Perhaps a simple patch would convey the change better and be easier to

maintain in the future / be readily available for other distributions to

use.

Otherwise, LGTM!

--

Thanks,

Maxim

Hi Maxim,

On 08.05.24 21:51, Maxim Cournoyer wrote:

The slight annoyance with this is that if pipewire is not running the

option to enable the screen recorder is there but it will stop

immediately. But at least searching online for this problem has some

mentions of pipewire and i added a comment in the pacakge aswell. I

think it is definitly a better solution than not knowing that the option

is even there as the button does not get displayed at all if the service

fails to start.

The simple patch that would do this is basically the patch from nixos in

v1 of this which adds a shebang line for gjs to the service invocation

files (rather than the dbus service invoking $gjs $service). The problem

then is that wrap-program changes the filename to * .real which makes

gjs unhappy.

The people from nix circumvent this by using some js at the beginning to

reset the entrypoint to the correct value.

One nice way around avoiding the problem would be using wrap-script

instead though that does not support gjs as interpreter (yet?) and

adding that forces a rebuild of all packages due to (guix build utils)

changing.

Maybe another comment, similiar to the one Liliana suggested earlier in

this thread, could be added at the beginning to inform about changing to

wrap script + patch instead once that is a viable option?

Have a nice day

Hi Dariqq,

Am Mittwoch, dem 08.05.2024 um 21:18 +0000 schrieb Dariqq:

The pattern we typically use is to add an autotools-style "variable",

e.g. @GNOME_SHELL_GST_PLUGIN_SYSTEM_PATH@ through a patch, then use

substitute* to fill it in. I don't think it's a requirement, but since

Maxim suggested, it'd definitely be nice to have.

Cheers

Hi Liliana,

On 09.05.24 00:11, Liliana Marie Prikler wrote:

How would this work in this case for gnomeshell? Put the js that gets

concatenated here into the actual file with a patch adding placeholder

variables for GST Plugin path and gi typelib path and replace later?

As these js files get generated from a common template for each service

this would require substituting all of them and not only the screencast

service.

Hi Dariqq,

Dariqq <dariqq@posteo.net> writes:

[...]

It'd be nice to prep such support on core-updates. You can test it on

master by having a (guix build utils-next) module that you explicitly

use.

That could be a good reminder to have, yes.

--

Thanks,

Maxim

Hi Liliana and Maxim,

On 09.05.24 00:11, Liliana Marie Prikler wrote:

Tried this today and as the js service files are created from a common

template using mesons 'configure_file' method this sets all

autotools-style variables unknown to meson to the empty string.

Afterwardes the susbtitute* at the wrapping phase is unable to replace

anything ofc.

So I think I would need to either change the naming-scheme of the

placeholders or substitute them into the template file before the files

get configured by meson.

Do you have a preference for any option (or maybe another idea)?

Have a nice day

Am Freitag, dem 10.05.2024 um 14:59 +0000 schrieb Dariqq:

Or you add an option to meson_options.txt to fill it in, so that you

can provide the right value via #:configure-flags

Cheers

On 10.05.24 18:04, Liliana Marie Prikler wrote:

I'd want to avoid putting the environement variables together myself and

rather use getenv to retrieve the value at build time though. Don't

think that would work with #:configure-flags

Have a nice day

Am Donnerstag, dem 09.05.2024 um 11:30 -0400 schrieb Maxim Cournoyer:

I've pushed the current version as-is, without reminder, so that we can

at least mark the issue done. If someone has a good text to add,

please don't hesitate to do so in post. Also don't hesitate to submit

new stuff to gnome-team or core-updates after the pending merges are

through.

Cheers and thanks for all the hard work :)