Subject: Pulseaudio woes

From: mirai <[email-masked]>
Date: Tue, 3 Jan 2023 14:43:23 +0000

I've found out that this service (even before this patch-set)
is biased towards being used as a home service rather than a system service.

The reason is that it sets the XDG_RUNTIME_DIR environment variable through shepherd
to:

--8<---------------cut here---------------start------------->8---
(list (string-append
        "XDG_RUNTIME_DIR=/run/user/"
        (number->string (passwd:uid (getpwnam #$user)))))
--8<---------------cut here---------------end--------------->8---

This directory does not exist if this is launched as a system-wide service.
I presume that this went unnoticed because most of the times you'd want to
use this service system-wide you also configure this either as a MPD
'satellite instance' or the audio-outputs are always network-streaming ones.

This falls apart if you configure a system-wide mpd-service with a pulseaudio
output as it will try to access XDG_RUNTIME_DIR which is not created for the
system? #t 'mpd' user.

Now, pulseaudio is usually launched as a 'per-user' daemon although it can
be used in a system-pulse configuration though this is strongly discouraged.
Under most conditions (pulseaudio config mostly unchanged),
a system-wide mpd-service-type is also able to launch its own pulse instance
but due to the inherent assumptions present in the original service we have that:
  - The XDG_RUNTIME_DIR env var is set when it should only be set for 'system? #f' users.
  - PULSE_CLIENTCONFIG and PULSE_CONFIG are not set by shepherd.
	These usually correspond to:
	  PULSE_CONFIG =/etc/pulse/client.conf
	  PULSE_CONFIG=/etc/pulse/daemon.conf
        But if you use mpd-service-type as a home service, you could also have
      these set to a user-specific pulse config, which will be overridden by shepherd
      if these are set in mpd-service-type.
        But again, if you don't set these env vars and use it as a systemwide service
      with a pulseaudio output and you workaround the XDG_RUNTIME_DIR by unsetting it,
      your pulse daemon won't read its config and there will be no output.

This is not just a theoretical issue, consider this snippet:

--8<---------------cut here---------------start------------->8---
;; Set a systemwide mpd service that streams with pulseaudio RTP output.

(service pulseaudio-service-type (pulseaudio-configuration
                                    (extra-script-files
                                       (list
                                         (plain-file
                                           "rtp.pa"
                                           (string-join (list "load-module module-null-sink sink_name=rtp"
                                                              "load-module module-rtp-send source=rtp.monitor"
                                                              "set-default-sink rtp") "\n" 'suffix))))))
(service mpd-service-type
                   (mpd-configuration
                    (outputs (list (mpd-output (name "Pulseaudio over RTP")
                                     (type "pulse")
                                     (extra-options '((sink . "rtp")))))))
--8<---------------cut here---------------end--------------->8---

The problem here is how to conditionally select which environment variables should be set based
on whether this is being launched as a system-wide or home-instance service. I'm all ears for
suggestions.

Regarding the current patch-set, I'm thinking that some of the non-public procedures could
be defined with 'define-deprecated' instead which should (?) be cleaner than the current
comments that are demarcating deprecated code.

Regards,
Bruno