[bug#39053] [PATCH] Add pulseaudio configuration and fix volume bugs

[bug#39053] [PATCH] Add pulseaudio configuration and fix volume bugs

[Leo Prikler]

This series of patches adds a configuration type for pulseaudio and also fixes
a bug, where various applications would inadvertently max out the system volume
(see e.g. #38172).

The second patch of this series differs from the one proposed there, in that
it uses environment variables to achieve its goal rather than rebuilding
pulseaudio.  As a result, pulseaudio-service will very likely ignore user
configuration in $XDG_CONFIG_DIR/pulse unless those variables are unset.
Doing so in ~/.profile or an equivalent to it should suffice.

Regards,
Leo

[bug#39052] [PATCH 1/3] services: Add pulseaudio-configuration.

[Leo Prikler]

* gnu/services/sound (<pulseaudio-configuration>): New record.
(pulseaudio-etc): New procedure.
(pulseaudio-service-type): Update accordingly.
---
 gnu/services/sound.scm | 47 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
index aaca733729..f01d958ce7 100644
--- a/gnu/services/sound.scm
+++ b/gnu/services/sound.scm
@@ -34,6 +34,7 @@
   #:export (alsa-configuration
             alsa-service-type
 
+            pulseaudio-configuration
             pulseaudio-service-type))
 
 ;;; Commentary:
@@ -106,19 +107,61 @@ ctl.!default {
 ;;; PulseAudio
 ;;;
 
+(define-record-type* <pulseaudio-configuration>
+  pulseaudio-configuration make-pulseaudio-configuration
+  pulseaudio-configuration?
+  (package pulseaudio-package (default pulseaudio))
+  (client-conf pulseaudio-client-conf (default '()))
+  (daemon-conf pulseaudio-daemon-conf (default '((flat-volumes no))))
+  (default-script pulseaudio-default-script (default #f))
+  (system-script pulseaudio-system-script (default #f)))
+
 (define (pulseaudio-environment config)
   ;; Define this variable in the global environment such that
   ;; pulseaudio swh-plugins works.
   `(("LADSPA_PATH"
      . ,(file-append swh-plugins "/lib/ladspa"))))
 
+(define (pulseaudio-conf-entry arg)
+  (match arg
+    ((key value)
+     (format #f "~a = ~s~%" key value))
+    ((? string? _)
+     (string-append arg "\n"))))
+
+(define pulseaudio-etc
+  (match-lambda
+    (($ <pulseaudio-configuration> package client-conf daemon-conf
+                                   default-script system-script)
+     (let ((default.pa (if default-script
+                           (apply mixed-text-file "default.pa"
+                                  default-script)
+                           (file-append package "/etc/pulse/default.pa"))))
+       `(("pulse"
+          ,(file-union
+            "pulse"
+            `(("client.conf"
+               ,(apply mixed-text-file "client.conf"
+                       (map pulseaudio-conf-entry client-conf)))
+              ("daemon.conf"
+               ,(apply mixed-text-file "daemon.conf"
+                       "default-script-file = " default.pa "\n"
+                       (map pulseaudio-conf-entry daemon-conf)))
+              ("default.pa" ,default.pa)
+              ("system.pa"
+               ,(if system-script
+                    (apply mixed-text-file "system.pa"
+                           system-script)
+                    (file-append package "/etc/pulse/system.pa")))))))))))
+
 (define pulseaudio-service-type
   (service-type
    (name 'pulseaudio)
    (extensions
     (list (service-extension session-environment-service-type
-                             pulseaudio-environment)))
-   (default-value #f)
+                             pulseaudio-environment)
+          (service-extension etc-service-type pulseaudio-etc)))
+   (default-value (pulseaudio-configuration))
    (description "Configure PulseAudio sound support.")))
 
 ;;; sound.scm ends here
-- 
2.24.1

[bug#39054] [PATCH 2/3] services: pulseaudio-service-type: Honor /etc.

[Leo Prikler]

* gnu/services/sound (pulseaudio-environment): Add PULSE_CONFIG and
PULSE_CLIENTCONFIG.
---
 gnu/services/sound.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
index f01d958ce7..859aa9c16d 100644
--- a/gnu/services/sound.scm
+++ b/gnu/services/sound.scm
@@ -119,8 +119,10 @@ ctl.!default {
 (define (pulseaudio-environment config)
   ;; Define this variable in the global environment such that
   ;; pulseaudio swh-plugins works.
-  `(("LADSPA_PATH"
-     . ,(file-append swh-plugins "/lib/ladspa"))))
+  `(("LADSPA_PATH" . ,(file-append swh-plugins "/lib/ladspa"))
+    ;; Define these variables, so that pulseaudio honors /etc.
+    ("PULSE_CONFIG" . "/etc/pulse/daemon.conf")
+    ("PULSE_CLIENTCONFIG" . "/etc/pulse/client.conf")))
 
 (define (pulseaudio-conf-entry arg)
   (match arg
-- 
2.24.1

[bug#39055] [PATCH 3/3] services: Add pulseaudio to %desktop-services.

[Leo Prikler]

* gnu/services/desktop.scm (%desktop-services): Add pulseaudio service.
---
 gnu/services/desktop.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index b40622a637..1be05fda4e 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -1183,6 +1183,7 @@ or setting its password with passwd.")))
 
          x11-socket-directory-service
 
+         (service pulseaudio-service-type)
          (service alsa-service-type)
 
          %base-services))
-- 
2.24.1

[bug#39053] [PATCH] Add pulseaudio configuration and fix volume bugs

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> This series of patches adds a configuration type for pulseaudio and also fixes
> a bug, where various applications would inadvertently max out the system volume
> (see e.g. #38172).

Derp, I did not notice you had re-rolled the patches already :-)

Please ignore the /etc/pulse related questions, this looks much better.

> The second patch of this series differs from the one proposed there, in that
> it uses environment variables to achieve its goal rather than rebuilding
> pulseaudio.  As a result, pulseaudio-service will very likely ignore user
> configuration in $XDG_CONFIG_DIR/pulse unless those variables are unset.
> Doing so in ~/.profile or an equivalent to it should suffice.

Sounds reasonable.  Users who wish to control pulseaudio outside of the
configuration system can just exclude the service, or unset PULSE_CONFIG
and PULSE_CLIENT_CONFIG in their profile.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39062] [PATCH 1/6] services: Add pulseaudio-configuration.

[Leo Prikler]

* gnu/services/sound (<pulseaudio-configuration>): New record.
(pulseaudio-etc): New procedure.
(pulseaudio-service-type): Update accordingly.
---
 gnu/services/sound.scm | 43 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
index aaca733729..307e62fd1b 100644
--- a/gnu/services/sound.scm
+++ b/gnu/services/sound.scm
@@ -34,6 +34,7 @@
   #:export (alsa-configuration
             alsa-service-type
 
+            pulseaudio-configuration
             pulseaudio-service-type))
 
 ;;; Commentary:
@@ -106,19 +107,57 @@ ctl.!default {
 ;;; PulseAudio
 ;;;
 
+(define-record-type* <pulseaudio-configuration>
+  pulseaudio-configuration make-pulseaudio-configuration
+  pulseaudio-configuration?
+  (client-conf pulseaudio-client-conf
+               (default '()))
+  (daemon-conf pulseaudio-daemon-conf
+               (default '()))
+  (script-file pulseaudio-script-file
+               (default (file-append pulseaudio "/etc/pulse/default.pa")))
+  (system-script-file pulseaudio-system-script-file
+                      (default
+                        (file-append pulseaudio "/etc/pulse/system.pa"))))
+
 (define (pulseaudio-environment config)
   ;; Define this variable in the global environment such that
   ;; pulseaudio swh-plugins works.
   `(("LADSPA_PATH"
      . ,(file-append swh-plugins "/lib/ladspa"))))
 
+(define (pulseaudio-conf-entry arg)
+  (match arg
+    ((key . value)
+     (format #f "~a = ~s~%" key value))
+    ((? string? _)
+     (string-append arg "\n"))))
+
+(define pulseaudio-etc
+  (match-lambda
+    (($ <pulseaudio-configuration> client-conf daemon-conf
+                                   default-script-file system-script-file)
+     `(("pulse"
+        ,(file-union
+          "pulse"
+          `(("client.conf"
+             ,(apply mixed-text-file "client.conf"
+                     (map pulseaudio-conf-entry client-conf)))
+            ("daemon.conf"
+             ,(apply mixed-text-file "daemon.conf"
+                     "default-script-file = " default-script-file "\n"
+                     (map pulseaudio-conf-entry daemon-conf)))
+            ("default.pa" ,default-script-file)
+            ("system.pa" ,system-script-file))))))))
+
 (define pulseaudio-service-type
   (service-type
    (name 'pulseaudio)
    (extensions
     (list (service-extension session-environment-service-type
-                             pulseaudio-environment)))
-   (default-value #f)
+                             pulseaudio-environment)
+          (service-extension etc-service-type pulseaudio-etc)))
+   (default-value (pulseaudio-configuration))
    (description "Configure PulseAudio sound support.")))
 
 ;;; sound.scm ends here
-- 
2.24.1

[bug#39061] [PATCH 2/6] services: pulseaudio-service-type: Honor /etc.

[Leo Prikler]

* gnu/services/sound (pulseaudio-environment): Add PULSE_CONFIG and
PULSE_CLIENTCONFIG.
---
 gnu/services/sound.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
index 307e62fd1b..6b2b345a44 100644
--- a/gnu/services/sound.scm
+++ b/gnu/services/sound.scm
@@ -123,8 +123,10 @@ ctl.!default {
 (define (pulseaudio-environment config)
   ;; Define this variable in the global environment such that
   ;; pulseaudio swh-plugins works.
-  `(("LADSPA_PATH"
-     . ,(file-append swh-plugins "/lib/ladspa"))))
+  `(("LADSPA_PATH" . ,(file-append swh-plugins "/lib/ladspa"))
+    ;; Define these variables, so that pulseaudio honors /etc.
+    ("PULSE_CONFIG" . "/etc/pulse/daemon.conf")
+    ("PULSE_CLIENTCONFIG" . "/etc/pulse/client.conf")))
 
 (define (pulseaudio-conf-entry arg)
   (match arg
-- 
2.24.1

[bug#39053] [PATCH 3/6] services: pulseaudio-configuration: Set flat-volumes to no.

[Leo Prikler]

* gnu/services/sound (pulseaudio-configuration) [daemon-conf]:
Disable flat-volumes.  Enabling flat-volumes, as is the upstream default,
causes unpleasant experiences to users when applications inadvertently max
out the system volume (e.g. as in #38172).
---
 gnu/services/sound.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
index 6b2b345a44..5d72b3ada2 100644
--- a/gnu/services/sound.scm
+++ b/gnu/services/sound.scm
@@ -113,7 +113,7 @@ ctl.!default {
   (client-conf pulseaudio-client-conf
                (default '()))
   (daemon-conf pulseaudio-daemon-conf
-               (default '()))
+               (default '((flat-volumes . no))))
   (script-file pulseaudio-script-file
                (default (file-append pulseaudio "/etc/pulse/default.pa")))
   (system-script-file pulseaudio-system-script-file
-- 
2.24.1

[bug#39065] [PATCH 4/6] services: Split ladspa-service-type from pulseaudio-service-type.

[Leo Prikler]

* gnu/services/sound.scm (ladspa-configuration): New record.
(ladspa-environment): New procedure.
(ladspa-service-type): New service type.
(pulseaudio-environment): Remove LADSPA_PATH.
* doc/guix.texi: Adjust documentation.
---
 doc/guix.texi          | 20 +++++++++++---------
 gnu/services/sound.scm | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 60491af4d4..431895aa7d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15951,19 +15951,21 @@ pcm.!default @{
 See @uref{https://www.alsa-project.org/main/index.php/Asoundrc} for the
 details.
 
-@deffn {Scheme Variable} pulseaudio-service-type
-This is the type for the @uref{http://www.pulseaudio.org/, PulseAudio
-sound server}, which sets the @var{LADSPA_PATH} environment variable to
-allow PulseAudio load modules from @code{swh-plugins} package.
+@deffn {Scheme Variable} ladspa-service-type
+This service sets the @var{LADSPA_PATH} variable, so that programs, which
+respect it, e.g. PulseAudio, can load LADSPA plugins.
 
-See @uref{http://plugin.org.uk/ladspa-swh/docs/ladspa-swh.html} for the
-details.
-
-The following example will setup the service:
+The following example will setup the service to enable modules from the
+@code{swh-plugins} package:
 
 @lisp
-(service pulseaudio-service-type)
+(service ladspa-service-type
+         (ladspa-configuration (plugins (list swh-plugins))))
 @end lisp
+
+See @uref{http://plugin.org.uk/ladspa-swh/docs/ladspa-swh.html} for the
+details.
+
 @end deffn
 
 @node Database Services
diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
index 5d72b3ada2..64b45f585f 100644
--- a/gnu/services/sound.scm
+++ b/gnu/services/sound.scm
@@ -35,7 +35,10 @@
             alsa-service-type
 
             pulseaudio-configuration
-            pulseaudio-service-type))
+            pulseaudio-service-type
+
+            ladspa-configuration
+            ladspa-service-type))
 
 ;;; Commentary:
 ;;;
@@ -162,4 +165,32 @@ ctl.!default {
    (default-value (pulseaudio-configuration))
    (description "Configure PulseAudio sound support.")))
 
+\f
+;;;
+;;; LADSPA
+;;;
+
+(define-record-type* <ladspa-configuration>
+  ladspa-configuration make-ladspa-configuration
+  ladspa-configuration?
+  (plugins ladspa-plugins (default '())))
+
+(define (ladspa-environment config)
+  ;; Define this variable in the global environment such that
+  ;; pulseaudio swh-plugins (and similar LADSPA plugins) work.
+  `(("LADSPA_PATH" .
+     (string-join
+      ',(map (lambda (package) (file-append package "/lib/ladspa"))
+             (ladspa-plugins config))
+      ":"))))
+
+(define ladspa-service-type
+  (service-type
+   (name 'ladspa)
+   (extensions
+    (list (service-extension session-environment-service-type
+                             ladspa-environment)))
+   (default-value (ladspa-configuration))
+   (description "Configure LADSPA plugins.")))
+
 ;;; sound.scm ends here
-- 
2.24.1

[bug#39053] [PATCH 5/6] doc: Add pulseaudio documentation.

[Leo Prikler]

* doc/guile.texi: Add documentation for pulseaudio-service-type and
pulseaudio-configuration.
---
 doc/guix.texi | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index 431895aa7d..05d826f6d8 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15951,6 +15951,48 @@ pcm.!default @{
 See @uref{https://www.alsa-project.org/main/index.php/Asoundrc} for the
 details.
 
+@deffn {Scheme Variable} pulseaudio-service-type
+This is the type for the  @uref{http://www.pulseaudio.org/, PulseAudio}
+sound server.  It exists to allow system overrides of the default settings
+via @code{pulseaudio-configuration}, see below.
+
+@quotation Warning
+This service on its own does not ensure, that the @code{pulseaudio} package
+exists on your machine.  It merely adds configuration files for it, as
+detailed below.  In the (admittedly unlikely) case, that you find yourself
+without a @code{pulseaudio} package, consider enabling it through the
+@code{alsa-service-type} above.
+@end quotation
+@end deffn
+
+@deftp {Data Type} pulseaudio-configuration
+Data type representing the configuration for @code{pulseaudio-service}.
+
+@table @asis
+@item @code{client-conf} (default: @var{()})
+List of settings to set in @file{client.conf}.
+Accepts a list of strings or a symbol-value pairs.  A string will be
+inserted as-is with a newline added.  A pair will be formatted as
+``key = value'', again with a newline added.
+
+@item @code{daemon-conf} (default: @var{((flat-volumes . no))})
+List of settings to set in @file{daemon.conf}, formatted just like
+@code{client-conf}.
+
+In addition to the above, @code{default-script-file} will be set to the
+value of @code{script-file}.  By default, @var{flat-volumes} is set to
+``no'', so as to avoid bugs related to this feature.
+
+@item @code{script-file}
+Script file to use as as @file{default.pa}.  Defaults to the one included by
+the @code{pulseaudio} package.
+
+@item @code{system-script-file}
+Script file to use as as @file{system.pa}.  Defaults to the one included by
+the @code{pulseaudio} package.
+@end table
+@end deftp
+
 @deffn {Scheme Variable} ladspa-service-type
 This service sets the @var{LADSPA_PATH} variable, so that programs, which
 respect it, e.g. PulseAudio, can load LADSPA plugins.
-- 
2.24.1

[bug#39066] [PATCH 6/6] services: Add pulseaudio to %desktop-services.

[Leo Prikler]

* gnu/services/desktop.scm (%desktop-services): Add pulseaudio service.
---
 gnu/services/desktop.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index b40622a637..1be05fda4e 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -1183,6 +1183,7 @@ or setting its password with passwd.")))
 
          x11-socket-directory-service
 
+         (service pulseaudio-service-type)
          (service alsa-service-type)
 
          %base-services))
-- 
2.24.1

[bug#39061] [PATCH 2/6] services: pulseaudio-service-type: Honor /etc.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> * gnu/services/sound (pulseaudio-environment): Add PULSE_CONFIG and
> PULSE_CLIENTCONFIG.
> ---
>  gnu/services/sound.scm | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
> index 307e62fd1b..6b2b345a44 100644
> --- a/gnu/services/sound.scm
> +++ b/gnu/services/sound.scm
> @@ -123,8 +123,10 @@ ctl.!default {
>  (define (pulseaudio-environment config)
>    ;; Define this variable in the global environment such that
>    ;; pulseaudio swh-plugins works.
> -  `(("LADSPA_PATH"
> -     . ,(file-append swh-plugins "/lib/ladspa"))))
> +  `(("LADSPA_PATH" . ,(file-append swh-plugins "/lib/ladspa"))
> +    ;; Define these variables, so that pulseaudio honors /etc.
> +    ("PULSE_CONFIG" . "/etc/pulse/daemon.conf")
> +    ("PULSE_CLIENTCONFIG" . "/etc/pulse/client.conf")))

I squashed this with the first patch.

By the way, for future merge request, please open a single bug report
and send all patches to NNNNN@debbugs.gnu.org.  I've merged the bugs
opened by this series.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39053] [bug#39063] [PATCH 3/6] services: pulseaudio-configuration: Set flat-volumes to no.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> * gnu/services/sound (pulseaudio-configuration) [daemon-conf]:
> Disable flat-volumes.  Enabling flat-volumes, as is the upstream default,
> causes unpleasant experiences to users when applications inadvertently max
> out the system volume (e.g. as in #38172).

I moved this comment inside the service definition code so that we don't
have to reach for 'git blame' when we forget why it was added.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39053] [bug#39065] [PATCH 4/6] services: Split ladspa-service-type from pulseaudio-service-type.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> * gnu/services/sound.scm (ladspa-configuration): New record.
> (ladspa-environment): New procedure.
> (ladspa-service-type): New service type.
> (pulseaudio-environment): Remove LADSPA_PATH.
> * doc/guix.texi: Adjust documentation.

This forgot to actually remove LADSPA_PATH from PULSEAUDIO-ENVIRONMENT;
fixed!

The previous pulseaudio-service-type is so recent that I don't think a
deprecation notice is necessary.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39064] [PATCH 5/6] doc: Add pulseaudio documentation.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1776 bytes --]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> * doc/guile.texi: Add documentation for pulseaudio-service-type and
> pulseaudio-configuration.

Ideally this would have been squashed with the first commit, but then
we'd have to move ladspa-service-type first, so I kept it as a
standalone commit.  I also added a copyright notice for you.

[...]
  

> +@deftp {Data Type} pulseaudio-configuration
> +Data type representing the configuration for @code{pulseaudio-service}.
> +
> +@table @asis
> +@item @code{client-conf} (default: @var{()})
> +List of settings to set in @file{client.conf}.
> +Accepts a list of strings or a symbol-value pairs.  A string will be
> +inserted as-is with a newline added.  A pair will be formatted as
> +``key = value'', again with a newline added.
> +
> +@item @code{daemon-conf} (default: @var{((flat-volumes . no))})
> +List of settings to set in @file{daemon.conf}, formatted just like
> +@code{client-conf}.
> +
> +In addition to the above, @code{default-script-file} will be set to the
> +value of @code{script-file}.  By default, @var{flat-volumes} is set to
> +``no'', so as to avoid bugs related to this feature.

The first sentence of this paragraph is obsolete, no?  The second is
rather vague, so I opted to remove the whole thing.  Let me know if you
think something should be added!

> +@item @code{script-file}
> +Script file to use as as @file{default.pa}.  Defaults to the one included by
> +the @code{pulseaudio} package.
> +
> +@item @code{system-script-file}
> +Script file to use as as @file{system.pa}.  Defaults to the one included by
> +the @code{pulseaudio} package.
> +@end table
> +@end deftp

I added a (default: ...) on these two and removed the related sentences.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39053] [PATCH 6/6] services: Add pulseaudio to %desktop-services.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 229 bytes --]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> * gnu/services/desktop.scm (%desktop-services): Add pulseaudio service.

I added a "Fixes: " line to the commit message pointing to the bug
report that inspired this series.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#38172: [bug#39053] [PATCH] Add pulseaudio configuration and fix volume bugs

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> This series of patches adds a configuration type for pulseaudio and also fixes
> a bug, where various applications would inadvertently max out the system volume
> (see e.g. #38172).

Thanks!  I've pushed the patches with mentioned tweaks in
2c7511fb6..71e33e32f.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39053] [bug#39064] [PATCH 5/6] doc: Add pulseaudio documentation.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

Marius Bakke <mbakke@fastmail.com> writes:

>> +@item @code{script-file}
>> +Script file to use as as @file{default.pa}.  Defaults to the one included by
>> +the @code{pulseaudio} package.
>> +
>> +@item @code{system-script-file}
>> +Script file to use as as @file{system.pa}.  Defaults to the one included by
>> +the @code{pulseaudio} package.
>> +@end table
>> +@end deftp
>
> I added a (default: ...) on these two and removed the related sentences.

I also changed to @var{...} instead of @code{...} for the options; and
used @code{...} instead of @var{...} on the defaults after reading the
generated HTML.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#38172: [bug#39053] [PATCH] Add pulseaudio configuration and fix volume bugs

[Leo Prikler]

Am Samstag, den 11.01.2020, 18:23 +0100 schrieb Marius Bakke:
> Leo Prikler <leo.prikler@student.tugraz.at> writes:
> 
> > This series of patches adds a configuration type for pulseaudio and
> > also fixes
> > a bug, where various applications would inadvertently max out the
> > system volume
> > (see e.g. #38172).
> 
> Thanks!  I've pushed the patches with mentioned tweaks in
> 2c7511fb6..71e33e32f.
Thanks!  Also, I'm sorry about accidentally opening like 10 bugs due to
my misconfiguration there.  I only noticed after the fact, that merely
CC'ing the original bug does nothing, when the mail is still sent to
guix-patches as well.  I've learned my lesson and will be more careful
in the future.

I do still have some open questions, though.
> > +In addition to the above, @code{default-script-file} will be set to
> > the
> > +value of @code{script-file}.  By default, @var{flat-volumes} is
> > set
> > to
> > +``no'', so as to avoid bugs related to this feature.
> > 
> The first sentence of this paragraph is obsolete, no?  The second is
> rather vague, so I opted to remove the whole thing.  Let me know if
> you
> think something should be added!
I'm not quite sure about the first sentence.  While everyone can read
the code and the output files produced from it, I think we should
document, that we actually always insert this line into
@file{daemon.conf}.
For instance, if you don't supply your own @file{default.pa}, the first
line of @file{daemon.conf} will be
--8<---------------cut here---------------start------------->8---
default-script-file = /gnu/store/<hash>-pulseaudio-
<version>/etc/pulse/default.pa
--8<---------------cut here---------------end--------------->8---
What are your thoughts on this?

> I added a (default: ...) on these two and removed the related
> sentences.
I was hesitant to do that due to the line limits.  Do those not count
for documentation or are such exceptions allowed?

Thanks again for your help and also thanks for your feedback.

Regards,
Leo

[bug#39053] [PATCH] Add pulseaudio configuration and fix volume bugs

[Leo Prikler]

Am Samstag, den 11.01.2020, 18:23 +0100 schrieb Marius Bakke:
> Leo Prikler <leo.prikler@student.tugraz.at> writes:
> 
> > This series of patches adds a configuration type for pulseaudio and
> > also fixes
> > a bug, where various applications would inadvertently max out the
> > system volume
> > (see e.g. #38172).
> 
> Thanks!  I've pushed the patches with mentioned tweaks in
> 2c7511fb6..71e33e32f.
Thanks!  Also, I'm sorry about accidentally opening like 10 bugs due to
my misconfiguration there.  I only noticed after the fact, that merely
CC'ing the original bug does nothing, when the mail is still sent to
guix-patches as well.  I've learned my lesson and will be more careful
in the future.

I do still have some open questions, though.
> > +In addition to the above, @code{default-script-file} will be set to
> > the
> > +value of @code{script-file}.  By default, @var{flat-volumes} is
> > set
> > to
> > +``no'', so as to avoid bugs related to this feature.
> > 
> The first sentence of this paragraph is obsolete, no?  The second is
> rather vague, so I opted to remove the whole thing.  Let me know if
> you
> think something should be added!
I'm not quite sure about the first sentence.  While everyone can read
the code and the output files produced from it, I think we should
document, that we actually always insert this line into
@file{daemon.conf}.
For instance, if you don't supply your own @file{default.pa}, the first
line of @file{daemon.conf} will be
--8<---------------cut here---------------start------------->8---
default-script-file = /gnu/store/<hash>-pulseaudio-
<version>/etc/pulse/default.pa
--8<---------------cut here---------------end--------------->8---
What are your thoughts on this?

> I added a (default: ...) on these two and removed the related
> sentences.
I was hesitant to do that due to the line limits.  Do those not count
for documentation or are such exceptions allowed?

Thanks again for your help and also thanks for your feedback.

Regards,
Leo

[bug#39053] [PATCH] Add pulseaudio configuration and fix volume bugs

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 2676 bytes --]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> Am Samstag, den 11.01.2020, 18:23 +0100 schrieb Marius Bakke:
>> Leo Prikler <leo.prikler@student.tugraz.at> writes:
>> 
>> > This series of patches adds a configuration type for pulseaudio and
>> > also fixes
>> > a bug, where various applications would inadvertently max out the
>> > system volume
>> > (see e.g. #38172).
>> 
>> Thanks!  I've pushed the patches with mentioned tweaks in
>> 2c7511fb6..71e33e32f.
> Thanks!  Also, I'm sorry about accidentally opening like 10 bugs due to
> my misconfiguration there.  I only noticed after the fact, that merely
> CC'ing the original bug does nothing, when the mail is still sent to
> guix-patches as well.  I've learned my lesson and will be more careful
> in the future.

Right, no worries!

> I do still have some open questions, though.
>> > +In addition to the above, @code{default-script-file} will be set to
>> > the
>> > +value of @code{script-file}.  By default, @var{flat-volumes} is
>> > set
>> > to
>> > +``no'', so as to avoid bugs related to this feature.
>> > 
>> The first sentence of this paragraph is obsolete, no?  The second is
>> rather vague, so I opted to remove the whole thing.  Let me know if
>> you
>> think something should be added!
> I'm not quite sure about the first sentence.  While everyone can read
> the code and the output files produced from it, I think we should
> document, that we actually always insert this line into
> @file{daemon.conf}.
> For instance, if you don't supply your own @file{default.pa}, the first
> line of @file{daemon.conf} will be
> --8<---------------cut here---------------start------------->8---
> default-script-file = /gnu/store/<hash>-pulseaudio-
> <version>/etc/pulse/default.pa
> --8<---------------cut here---------------end--------------->8---
> What are your thoughts on this?

Oh, right.  I suppose that can be surprising.  Would you like to
resubmit a documentation update?

I wonder if we could use the flunking new 'this-record', and refer to
the SCRIPT-FILE directly in the default parameter list.  But that's
probably overkill for this instance.  :-)

>> I added a (default: ...) on these two and removed the related
>> sentences.
> I was hesitant to do that due to the line limits.  Do those not count
> for documentation or are such exceptions allowed?

Not sure what/if there are line length limits on guix.texi, but 95
characters should be okay for one-off instances like these.  I suppose
we could escape a line break if it breaks someones workflow.

> Thanks again for your help and also thanks for your feedback.

Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39053] [PATCH] Add pulseaudio configuration and fix volume bugs

[Leo Prikler]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

Am Sonntag, den 12.01.2020, 00:49 +0100 schrieb Marius Bakke:
> Oh, right.  I suppose that can be surprising.  Would you like to
> resubmit a documentation update?
Sure, see the attachment.  (Not sure if that's overkill.)

> I wonder if we could use the flunking new 'this-record', and refer to
> the SCRIPT-FILE directly in the default parameter list.  But that's
> probably overkill for this instance.  :-)
Well, actually, that might not be /that/ bad, were it not for the fact
that defaults have to be documented.  Using 'this-record' would allow
people to use the system-script-file there as well, although I don't
know if the value will be read when PA starts in system mode.
That said, we would also force people to copypasta this into their own
configuration.  Is that still acceptable?

> Not sure what/if there are line length limits on guix.texi, but 95
> characters should be okay for one-off instances like these.  I
> suppose
> we could escape a line break if it breaks someones workflow.
I personally would not notice a line limit either way safe for perhaps
some very short lines, due to having olivetti enabled in all text
modes.

Regards,
Leo

[-- Attachment #2: 0001-doc-Fully-document-pulseaudio-daemon-conf.patch --]
[-- Type: text/x-patch, Size: 1447 bytes --]

From c1f3757d53423cc51f1f9017edd4427c20e7025c Mon Sep 17 00:00:00 2001
From: Leo Prikler <leo.prikler@student.tugraz.at>
Date: Sun, 12 Jan 2020 01:20:27 +0100
Subject: [PATCH] doc: Fully document pulseaudio daemon-conf.

* doc/guix.texi: Add note about the default-script-file line and the intention
behind it.
---
 doc/guix.texi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index d2038d18e1..0b8569b54a 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15979,6 +15979,17 @@ inserted as-is with a newline added.  A pair will be formatted as
 @item @var{daemon-conf} (default: @code{'((flat-volumes . no))})
 List of settings to set in @file{daemon.conf}, formatted just like
 @var{client-conf}.
+@quotation Note
+In addition to the above settings being applied, the first line of the
+generated @file{daemon.conf} will always read
+@code{default-script-file = FILE} with @var{FILE} being the path to the
+supplied @var{script-file}.
+This makes pulseaudio read the correct @file{default.pa} when started in
+user-mode (the default behaviour).
+Such a line does not exist for @file{system.pa}.  If you need to load this
+file for some reason, you'll have to use environment variables as detailed
+in the PulseAudio documentation.
+@end quotation
 
 @item @var{script-file} (default: @code{(file-append pulseaudio "/etc/pulse/default.pa")})
 Script file to use as as @file{default.pa}.
-- 
2.24.1

[bug#39053] [PATCH] Add pulseaudio configuration and fix volume bugs

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> Am Sonntag, den 12.01.2020, 00:49 +0100 schrieb Marius Bakke:
>> Oh, right.  I suppose that can be surprising.  Would you like to
>> resubmit a documentation update?
> Sure, see the attachment.  (Not sure if that's overkill.)

Looks good.

[...]


> diff --git a/doc/guix.texi b/doc/guix.texi
> index d2038d18e1..0b8569b54a 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -15979,6 +15979,17 @@ inserted as-is with a newline added.  A pair will be formatted as
>  @item @var{daemon-conf} (default: @code{'((flat-volumes . no))})
>  List of settings to set in @file{daemon.conf}, formatted just like
>  @var{client-conf}.
> +@quotation Note
> +In addition to the above settings being applied, the first line of the
> +generated @file{daemon.conf} will always read
> +@code{default-script-file = FILE} with @var{FILE} being the path to the
> +supplied @var{script-file}.
> +This makes pulseaudio read the correct @file{default.pa} when started in
> +user-mode (the default behaviour).
> +Such a line does not exist for @file{system.pa}.  If you need to load this
> +file for some reason, you'll have to use environment variables as detailed
> +in the PulseAudio documentation.
> +@end quotation

I tried finding documentation for system.pa to no avail.  Do you have a
link at hand?  Perhaps it's better to remove the system.pa sentences
from here, and add a note below "system-script-file" how to make it
effective, preferably with an @url{...} pointing to PA documentation.

WDYT?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39053] [PATCH] Add pulseaudio configuration and fix volume bugs

[Leo Prikler]

Am Sonntag, den 12.01.2020, 20:55 +0100 schrieb Marius Bakke:
> Leo Prikler <leo.prikler@student.tugraz.at> writes:
> 
> > Am Sonntag, den 12.01.2020, 00:49 +0100 schrieb Marius Bakke:
> > > Oh, right.  I suppose that can be surprising.  Would you like to
> > > resubmit a documentation update?
> > Sure, see the attachment.  (Not sure if that's overkill.)
> 
> Looks good.
> 
> [...]
> 
> 
> > diff --git a/doc/guix.texi b/doc/guix.texi
> > index d2038d18e1..0b8569b54a 100644
> > --- a/doc/guix.texi
> > +++ b/doc/guix.texi
> > @@ -15979,6 +15979,17 @@ inserted as-is with a newline added.  A
> > pair will be formatted as
> >  @item @var{daemon-conf} (default: @code{'((flat-volumes . no))})
> >  List of settings to set in @file{daemon.conf}, formatted just like
> >  @var{client-conf}.
> > +@quotation Note
> > +In addition to the above settings being applied, the first line of
> > the
> > +generated @file{daemon.conf} will always read
> > +@code{default-script-file = FILE} with @var{FILE} being the path
> > to the
> > +supplied @var{script-file}.
> > +This makes pulseaudio read the correct @file{default.pa} when
> > started in
> > +user-mode (the default behaviour).
> > +Such a line does not exist for @file{system.pa}.  If you need to
> > load this
> > +file for some reason, you'll have to use environment variables as
> > detailed
> > +in the PulseAudio documentation.
> > +@end quotation
> 
> I tried finding documentation for system.pa to no avail.  Do you have
> a
> link at hand?  Perhaps it's better to remove the system.pa sentences
> from here, and add a note below "system-script-file" how to make it
> effective, preferably with an @url{...} pointing to PA documentation.
That's not surprising, given that the format for default.pa and
system.pa is the same.  The convention in PulseAudio is, that the
former be loaded when it is started in "user mode", and the latter be
loaded when it is started in "system mode".  However, there exists but
one environment variable (PULSE_SCRIPT now that I recall its name),
that overrides BOTH settings.

Upon closer inspection, it seems however, that this environment
variable is not the only way to to supply a script to load.
Reading the output of `pulseaudio --help` also contains a few lines on
that.

--8<---------------cut here---------------start------------->8---
STARTUP SCRIPT:
  -L, --load="MODULE ARGUMENTS"         Load the specified plugin
module with
                                        the specified argument
  -F, --file=FILENAME                   Run the specified script
  -C                                    Open a command line on the
running TTY
                                        after startup

  -n                                    Don't load default script file
--8<---------------cut here---------------end--------------->8---

> WDYT?
It should be worded in a way, that does not cause such a
misunderstanding.  What I meant to convey (but failed at doing so) was
something along the lines of "consult the PulseAudio documentation on
how to load scripts", not "consult the PulseAudio documentation on
system.pa".  Not loading system.pa unless being told to do so is a
Guix-specific feature ;)

Can you patch this documentation in a way that people, who are not me,
also understand it?  I fully admit that I'm weak at explaining.

Regards,
Leo

[bug#39053] [bug#39066] [PATCH 6/6] services: Add pulseaudio to %desktop-services.

[Ludovic Courtès]

Leo Prikler <leo.prikler@student.tugraz.at> skribis:

> * gnu/services/desktop.scm (%desktop-services): Add pulseaudio service.

This was done in 71e33e32fcedbd0aaafda4fd548fb8443064253c.

Closing!

Ludo'.