[bug#63863] [PATCH] gnu: home: Add support for home-pipewire-service

["Ludovic Courtès" <ludo@gnu.org>]

Hi,

Brian Cully <bjc@spork.org> skribis:

> This adds a set of home shepherd services which will start the required
> services for a functional pipewire setup.
>
> * gnu/home/services/sound.scm (home-pipewire-shepherd-service),
> (home-pipewire-pulse-shepherd-service), (home-wireplumber-shepherd-service),
> (home-pipewire-shepherd-services), (generate-doc): new procedures.
> (home-pipewire-service-type): new service type.
> (home-pipewire-configuration): new struct.
> * doc/guix.texi (Sound Home Services): document it.

[...]

> +@cindex PipeWire, home service
> +
> +@uref{https://pipewire.org, PipeWire} provides a low-latency,
> +graph-based audio and video processing service. In addition to its
> +native protocol, it can also be used as a replacement for both JACK and
> +PulseAudio.

Could you explain why a Home service is necessary (I’d expect it to be
started on-demand via D-Bus or similar, like PulseAudio)?

Also, please leave two spaces after end-of-sentence periods (this eases
navigation in Emacs).

> +@table @asis
> +@item @code{pipewire} (default: @code{pipewire}) (type: file-like)
> +The PipeWire package to use.
> +
> +@item @code{wireplumber} (default: @code{wireplumber}) (type: file-like)
> +The WirePlumber package to use.

Could you add a few words saying what each of these packages does,
especially the second one.

> +@item @code{enable-pulseaudio?} (default: @code{#t}) (type: boolean)
> +Enable PulseAudio replacement.

Maybe add: “When true, PulseAudio applications will talk to PipeWire,
which will handle them as if they were ``native'' PipeWire
applications.” (I’m making it up, but you get the idea.)

> +;;; PipeWire support.
> +;;;
> +(define-configuration/no-serialization home-pipewire-configuration

Please leave an empty line after the comment.

> +(define (home-pipewire-shepherd-service config)
> +  (shepherd-service
> +   (documentation "PipeWire screen and audio sharing.")
> +   (provision '(pipewire))
> +   (requirement '(dbus))
> +   (start #~(make-forkexec-constructor
> +             (list #$(file-append
> +                      (home-pipewire-configuration-pipewire config)
> +                      "/bin/pipewire"))))))

Please add the ‘stop’ method or the process will never be stopped.  :-)

> +   (start #~(make-forkexec-constructor
> +             (list #$(file-append
> +                      (home-pipewire-configuration-pipewire config)
> +                      "/bin/pipewire-pulse"))))))

Same here…

> +   (start #~(make-forkexec-constructor
> +             (list #$(file-append
> +                      (home-pipewire-configuration-wireplumber config)
> +                      "/bin/wireplumber"))))))

… and here.


> +(define (home-pipewire-shepherd-services config)
> +  (define shepherd-services
> +    (filter
> +     identity
> +     (list home-pipewire-shepherd-service home-wireplumber-shepherd-service
> +           (and (home-pipewire-configuration-enable-pulseaudio? config)
> +                home-pipewire-pulseaudio-shepherd-service))))
> +  (map (cut <> config) shepherd-services))

Rather:

  (cons* (home-pipewire-shepherd-service config)
         (home-wireplumber-shepherd-service config)
         (if …
             (list (home-pipewire-pulseaudio-shepherd-service config))
             '()))

> +   (description
> +    "Start essential PipeWire services.")

Can you add a couple of sentences?  That’s useful when running ‘guix
home search’ or similar.

> +
> +\f
> +;;;
> +;;; Generate documentation.
> +;;;
> +(define (generate-doc)
> +  (configuration->documentation 'home-pipewire-configuration))

This is unused, please remove it.

Could you send a v2?

Thanks!

Ludo’.