GNU bug report logs - #39053
[PATCH] Add pulseaudio configuration and fix volume bugs

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39053 in the body.
You can then email your comments to 39053 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: guix-patches <at> gnu.org
Subject: [PATCH] Add pulseaudio configuration and fix volume bugs
Date: Thu,  9 Jan 2020 14:57:43 +0100

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

Message #8 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, 39053 <at> debbugs.gnu.org
Subject: Re: [bug#39053] [PATCH] Add pulseaudio configuration and fix volume
 bugs
Date: Thu, 09 Jan 2020 22:50:49 +0100

[Message part 1 (text/plain, inline)]

Leo Prikler <leo.prikler <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: Marius Bakke <mbakke <at> fastmail.com>, raingloom <at> riseup.net
Cc: Oleg Pykhalov <go.wigust <at> gmail.com>, 39053 <at> debbugs.gnu.org,
 38172 <at> debbugs.gnu.org
Subject: Re: bug#38172: WebkitGTK-based browsers: System volume suddenly
 maxed out when playing audio or video
Date: Thu, 09 Jan 2020 23:49:17 +0100

Am Donnerstag, den 09.01.2020, 21:48 +0100 schrieb Marius Bakke:
> 
> I have a preference for making this field empty initially to have a
> 1:1
> compatibility with the current PA client and daemon configuration
> (i.e. nothing).  Then a follow-up patch can add this new
> configuration,
> perhaps with an explaining comment.
Fair enough.  This would mean I'd have to split 0001 into two, but
okay.

> Does it make sense to have default-script and system-script default
> to
> (file-append pulseaudio "...") and avoid the conditional altogether?
The idea behind it was to have the script itself in the code rather
than asking users to construct a mixed-text-file, but I'm fine either
way.

> This means pulseaudio will start looking in /etc/pulse for
> configuration
> files on foreign distributions too, right?
> 
> I wonder if there is better way to give it configuration
> files.  Perhaps
> by patching the D-Bus service files?  Not a blocker for this series,
> but
> something to consider in case /etc/pulse causes trouble.
This is already addressed by the renewed series I sent to guix-patches. 
I know you already found that, but I'd like to repeat it for those who
thus far have only read this thread.

> End on #t.
As above, but thanks for the hint, I missed the warning it seems.

> [...]
> 
> > From e24016f9a44a113847dd937ac47ab4bdb960236d Mon Sep 17 00:00:00
> > 2001
> > From: Leo Prikler <leo.prikler <at> student.tugraz.at>
> > Date: Thu, 9 Jan 2020 01:29:13 +0100
> > Subject: [PATCH 3/3] services: Add pulseaudio to %desktop-services.
> > 
> > * gnu/services/desktop.scm (%desktop-services): Add pulseaudio
> > service.
> 
> This will pull in "swh-plugins" which was the original intention
> behind
> pulseaudio-service before this patch series.  Before adding it to
> %desktop-services, I would prefer if the pulseaudio environment
> configuration could be made modular, so that there are no
> configuration
> differences for end users, i.e. they'd have to actively enable the
> LADSPA plugin.
I think adding a field ladspa-plugins, which accepts a list of packages
and adds their "lib/ladspa" would be the right approach here, but I
also feel, that this perhaps deserves its own service unrelated to
pulseaudio.  WDYT?
Either way, I agree on the "having to actively enable" part. 

> As a final note, can you also update doc/guix.texi accordingly?
I will once I've figured out how to best handle these fields.

Regards,
Leo

Message #14 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: guix-patches <at> gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: [PATCH 1/6] services: Add pulseaudio-configuration.
Date: Fri, 10 Jan 2020 02:48:19 +0100

* 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

Message #17 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: guix-patches <at> gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: [PATCH 2/6] services: pulseaudio-service-type: Honor /etc.
Date: Fri, 10 Jan 2020 02:48:20 +0100

* 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

Message #20 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: guix-patches <at> gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: [PATCH 3/6] services: pulseaudio-configuration: Set flat-volumes to
 no.
Date: Fri, 10 Jan 2020 02:48:21 +0100

* 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

Message #23 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: guix-patches <at> gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: [PATCH 4/6] services: Split ladspa-service-type from
 pulseaudio-service-type.
Date: Fri, 10 Jan 2020 02:48:22 +0100

* 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.")))
 
+
+;;;
+;;; 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

Message #26 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: guix-patches <at> gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: [PATCH 6/6] services: Add pulseaudio to %desktop-services.
Date: Fri, 10 Jan 2020 02:48:24 +0100

* 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

Message #29 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: guix-patches <at> gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: [PATCH 5/6] doc: Add pulseaudio documentation.
Date: Fri, 10 Jan 2020 02:48:23 +0100

* 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

Message #32 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, raingloom <at> riseup.net
Cc: Oleg Pykhalov <go.wigust <at> gmail.com>, 39053 <at> debbugs.gnu.org,
 38172 <at> debbugs.gnu.org
Subject: Re: bug#38172: WebkitGTK-based browsers: System volume suddenly maxed
 out when playing audio or video
Date: Sat, 11 Jan 2020 17:48:01 +0100

[Message part 1 (text/plain, inline)]

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

> Am Donnerstag, den 09.01.2020, 21:48 +0100 schrieb Marius Bakke:
>> 
>> I have a preference for making this field empty initially to have a
>> 1:1
>> compatibility with the current PA client and daemon configuration
>> (i.e. nothing).  Then a follow-up patch can add this new
>> configuration,
>> perhaps with an explaining comment.
> Fair enough.  This would mean I'd have to split 0001 into two, but
> okay.

Excellent.  :-)

>> Does it make sense to have default-script and system-script default
>> to
>> (file-append pulseaudio "...") and avoid the conditional altogether?
> The idea behind it was to have the script itself in the code rather
> than asking users to construct a mixed-text-file, but I'm fine either
> way.

Right.  I just have a preference for the default being "up-front",
instead of magic hiding behind an #f.  :-)

There's also LOCAL-FILE and PLAIN-FILE, which are more "obvious" than
MIXED-TEXT-FILE.

It could be useful to support plain strings for users who don't wish to
much about with G-expressions though, hopefully users will send a bug
report if they find it limiting.

[signature.asc (application/pgp-signature, inline)]

Message #37 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, 39061 <at> debbugs.gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: Re: [bug#39061] [PATCH 2/6] services: pulseaudio-service-type: Honor
 /etc.
Date: Sat, 11 Jan 2020 17:57:56 +0100

[Message part 1 (text/plain, inline)]

Leo Prikler <leo.prikler <at> 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 <at> debbugs.gnu.org.  I've merged the bugs
opened by this series.

[signature.asc (application/pgp-signature, inline)]

Message #40 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, 39063 <at> debbugs.gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: Re: [bug#39063] [PATCH 3/6] services: pulseaudio-configuration: Set
 flat-volumes to no.
Date: Sat, 11 Jan 2020 17:59:23 +0100

[Message part 1 (text/plain, inline)]

Leo Prikler <leo.prikler <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #43 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, 39065 <at> debbugs.gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: Re: [bug#39065] [PATCH 4/6] services: Split ladspa-service-type from
 pulseaudio-service-type.
Date: Sat, 11 Jan 2020 18:00:34 +0100

[Message part 1 (text/plain, inline)]

Leo Prikler <leo.prikler <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #46 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, 39064 <at> debbugs.gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: Re: [bug#39064] [PATCH 5/6] doc: Add pulseaudio documentation.
Date: Sat, 11 Jan 2020 18:09:04 +0100

[Message part 1 (text/plain, inline)]

Leo Prikler <leo.prikler <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #49 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, 39053 <at> debbugs.gnu.org
Subject: Re: [bug#39053] [PATCH 6/6] services: Add pulseaudio to
 %desktop-services.
Date: Sat, 11 Jan 2020 18:10:31 +0100

[Message part 1 (text/plain, inline)]

Leo Prikler <leo.prikler <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #54 received at 39053-done <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, 39053-done <at> debbugs.gnu.org
Cc: 38172-done <at> debbugs.gnu.org
Subject: Re: [bug#39053] [PATCH] Add pulseaudio configuration and fix volume
 bugs
Date: Sat, 11 Jan 2020 18:23:26 +0100

[Message part 1 (text/plain, inline)]

Leo Prikler <leo.prikler <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #97 received at 39053 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, 39064 <at> debbugs.gnu.org
Cc: 39053 <at> debbugs.gnu.org
Subject: Re: [bug#39064] [PATCH 5/6] doc: Add pulseaudio documentation.
Date: Sat, 11 Jan 2020 18:25:46 +0100

[Message part 1 (text/plain, inline)]

Marius Bakke <mbakke <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #100 received at 39053-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: Marius Bakke <mbakke <at> fastmail.com>, 39053-done <at> debbugs.gnu.org
Cc: 38172-done <at> debbugs.gnu.org
Subject: Re: [bug#39053] [PATCH] Add pulseaudio configuration and fix volume
 bugs
Date: Sat, 11 Jan 2020 19:37:54 +0100

Am Samstag, den 11.01.2020, 18:23 +0100 schrieb Marius Bakke:
> Leo Prikler <leo.prikler <at> 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

Message #103 received at 39053-done <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, 39053-done <at> debbugs.gnu.org
Subject: Re: [bug#39053] [PATCH] Add pulseaudio configuration and fix volume
 bugs
Date: Sun, 12 Jan 2020 00:49:00 +0100

[Message part 1 (text/plain, inline)]

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

> Am Samstag, den 11.01.2020, 18:23 +0100 schrieb Marius Bakke:
>> Leo Prikler <leo.prikler <at> 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!

[signature.asc (application/pgp-signature, inline)]

Message #106 received at 39053-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: Marius Bakke <mbakke <at> fastmail.com>, 39053-done <at> debbugs.gnu.org
Subject: Re: [bug#39053] [PATCH] Add pulseaudio configuration and fix volume
 bugs
Date: Sun, 12 Jan 2020 01:32:08 +0100

[Message part 1 (text/plain, inline)]

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

[0001-doc-Fully-document-pulseaudio-daemon-conf.patch (text/x-patch, attachment)]

Message #109 received at 39053-done <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>, 39053-done <at> debbugs.gnu.org
Subject: Re: [bug#39053] [PATCH] Add pulseaudio configuration and fix volume
 bugs
Date: Sun, 12 Jan 2020 20:55:14 +0100

[Message part 1 (text/plain, inline)]

Leo Prikler <leo.prikler <at> 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?

[signature.asc (application/pgp-signature, inline)]

Message #112 received at 39053-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: Marius Bakke <mbakke <at> fastmail.com>, 39053-done <at> debbugs.gnu.org
Subject: Re: [bug#39053] [PATCH] Add pulseaudio configuration and fix volume
 bugs
Date: Sun, 12 Jan 2020 21:22:23 +0100

Am Sonntag, den 12.01.2020, 20:55 +0100 schrieb Marius Bakke:
> Leo Prikler <leo.prikler <at> 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

Message #115 received at 39053-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>
Cc: 39066-done <at> debbugs.gnu.org, 39053-done <at> debbugs.gnu.org
Subject: Re: [bug#39066] [PATCH 6/6] services: Add pulseaudio to
 %desktop-services.
Date: Fri, 17 Jan 2020 17:30:44 +0100

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

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

This was done in 71e33e32fcedbd0aaafda4fd548fb8443064253c.

Closing!

Ludo'.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.