GNU bug report logs - #38075
[PATCH] services: mpd: add pulseaudio support

Previous Next

Package: guix-patches;

Reported by: Robert Smith <robertsmith <at> posteo.net>

Date: Tue, 5 Nov 2019 22:58:01 UTC

Severity: normal

Tags: patch

Done: Tobias Geerinckx-Rice <me <at> tobias.gr>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#38075; Package guix-patches. (Tue, 05 Nov 2019 22:58:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Robert Smith <robertsmith <at> posteo.net>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Tue, 05 Nov 2019 22:58:02 GMT) Full text and rfc822 format available.

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

From: Robert Smith <robertsmith <at> posteo.net>
To: guix-patches <at> gnu.org
Cc: Robert Smith <robertsmith <at> posteo.net>
Subject: [PATCH] services: mpd: add pulseaudio support
Date: Tue,  5 Nov 2019 23:46:46 +0100
Set the XDG_RUNTIME_PATH environment variable in the mpd service so
that it can detect pulseaudio when run under a user account.
---
 gnu/services/audio.scm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 471c5fd95f..424ccc7c8f 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -140,6 +140,12 @@ audio_output {
                    "--no-daemon"
                    #$(mpd-config->file config))
              #:pid-file #$(mpd-file-name config "pid")
+             #:environment-variables
+             '(#$(string-append
+                   "XDG_RUNTIME_DIR=/run/user/"
+                   (number->string
+                     (passwd:uid 
+                       (getpwnam (mpd-configuration-user config))))))
              #:log-file #$(mpd-file-name config "log")))
    (stop  #~(make-kill-destructor))))
 
-- 
2.23.0





Information forwarded to guix-patches <at> gnu.org:
bug#38075; Package guix-patches. (Wed, 06 Nov 2019 11:23:01 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Robert Smith <robertsmith <at> posteo.net>
Cc: 38075 <at> debbugs.gnu.org, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 Leo Famulari <leo <at> famulari.name>
Subject: Re: [bug#38075] [PATCH] services: mpd: add pulseaudio support
Date: Wed, 06 Nov 2019 12:22:29 +0100
Hi,

(Cc’ing Leo and Tobias who have been taking care of the mpd package and
service.)

Robert Smith <robertsmith <at> posteo.net> skribis:

> Set the XDG_RUNTIME_PATH environment variable in the mpd service so
> that it can detect pulseaudio when run under a user account.
> ---
>  gnu/services/audio.scm | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
> index 471c5fd95f..424ccc7c8f 100644
> --- a/gnu/services/audio.scm
> +++ b/gnu/services/audio.scm
> @@ -140,6 +140,12 @@ audio_output {
>                     "--no-daemon"
>                     #$(mpd-config->file config))
>               #:pid-file #$(mpd-file-name config "pid")
> +             #:environment-variables
> +             '(#$(string-append
> +                   "XDG_RUNTIME_DIR=/run/user/"
> +                   (number->string
> +                     (passwd:uid 
> +                       (getpwnam (mpd-configuration-user config))))))
>               #:log-file #$(mpd-file-name config "log")))

I suppose this is meant for the libpulse client library, right?  (I’m
surprised it’s not the default, but apparently that’s the way it is.)

Note that this also has an effect on mpd itself from what I can see in
the source: mpd will listen to $XDG_RUNTIME_DIR/mpd/socket instead of
(in addition to?) listening on TCP.

I don’t use mpd myself; could you ensure that the socket change is also
fine and doesn’t cause any troubles for mpd clients?

Thanks,
Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#38075; Package guix-patches. (Wed, 06 Nov 2019 13:33:02 GMT) Full text and rfc822 format available.

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

From: Robert Smith <robertsmith <at> posteo.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 38075 <at> debbugs.gnu.org, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 Leo Famulari <leo <at> famulari.name>
Subject: Re: [bug#38075] [PATCH] services: mpd: add pulseaudio support
Date: Wed, 06 Nov 2019 14:32:15 +0100
On 06.11.2019 12:22, Ludovic Courtès wrote:
> 
> Note that this also has an effect on mpd itself from what I can see in
> the source: mpd will listen to $XDG_RUNTIME_DIR/mpd/socket instead of
> (in addition to?) listening on TCP.
> 
> I don’t use mpd myself; could you ensure that the socket change is also
> fine and doesn’t cause any troubles for mpd clients?

I've been using the ncmpcpp client, and the functionality is unchanged.
I can confirm that the mpd still listens to TCP port 6600, the only
difference being that the output is sent to pulseaudio.

-Robert




Information forwarded to guix-patches <at> gnu.org:
bug#38075; Package guix-patches. (Wed, 06 Nov 2019 18:28:01 GMT) Full text and rfc822 format available.

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

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Robert Smith <robertsmith <at> posteo.net>, 38075 <at> debbugs.gnu.org,
 Tobias Geerinckx-Rice <me <at> tobias.gr>
Subject: Re: [bug#38075] [PATCH] services: mpd: add pulseaudio support
Date: Wed, 6 Nov 2019 13:27:02 -0500
On Wed, Nov 06, 2019 at 12:22:29PM +0100, Ludovic Courtès wrote:
> (Cc’ing Leo and Tobias who have been taking care of the mpd package and
> service.)

I haven't been using the MPD service so I don't really have an opinion
on this patch. Starting it "by hand", the pulse output works for me
already. But it would be great if our service worked "out of the box"
with Pulseaudio.




Information forwarded to guix-patches <at> gnu.org:
bug#38075; Package guix-patches. (Wed, 06 Nov 2019 23:55:01 GMT) Full text and rfc822 format available.

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

From: Robert Smith <robertsmith <at> posteo.net>
To: Leo Famulari <leo <at> famulari.name>
Cc: 38075 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>
Subject: Re: [bug#38075] [PATCH] services: mpd: add pulseaudio support
Date: Thu, 07 Nov 2019 00:54:10 +0100

On 06.11.2019 19:27, Leo Famulari wrote:
> I haven't been using the MPD service so I don't really have an opinion
> on this patch. Starting it "by hand", the pulse output works for me
> already. But it would be great if our service worked "out of the box"
> with Pulseaudio.

I'm not sure how common my use case is (running mpd locally under
my own user account) but since we are supporting the pulse output
mode in the service configuration, I think we need to make sure that
it works. Currently when the mpd service is started under my account
with the pulse output, it fails to connect to pulse because of the
missing environment variable, and defaults to alsa. This is unfortunate
because the whole point of using pulse for me is to make sure
different apps "play nicely" with each other when playing audio.
Thankfully this is a relatively simple fix.

I also installed the sonata client to make sure that other clients
weren't acting funny, and everything seems normal!




Information forwarded to guix-patches <at> gnu.org:
bug#38075; Package guix-patches. (Fri, 08 Nov 2019 19:54:02 GMT) Full text and rfc822 format available.

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

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: Ludovic Courtès <ludo <at> gnu.org>, Peter Mikkelsen
 <petermikkelsen10 <at> gmail.com>
Cc: Robert Smith <robertsmith <at> posteo.net>, 38075 <at> debbugs.gnu.org,
 Leo Famulari <leo <at> famulari.name>
Subject: Re: [bug#38075] [PATCH] services: mpd: add pulseaudio support
Date: Fri, 08 Nov 2019 20:53:25 +0100
[Message part 1 (text/plain, inline)]
Peter [CC'd],

I see that you added the Guix MPD service in 2017, and wrote then 
that it ‘uses pulseaudio for output’[0].  Do you have an opinion 
on this patch[1]?  Did something change?

Kind regards,

T G-R

[0]: http://guix.gnu.org/manual/en/html_node/Audio-Services.html
[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38075

Ludovic Courtès 写道:
> Robert Smith <robertsmith <at> posteo.net> skribis:
>
>> Set the XDG_RUNTIME_PATH environment variable in the mpd 
>> service so
>> that it can detect pulseaudio when run under a user account.
>> ---
>>  gnu/services/audio.scm | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
>> index 471c5fd95f..424ccc7c8f 100644
>> --- a/gnu/services/audio.scm
>> +++ b/gnu/services/audio.scm
>> @@ -140,6 +140,12 @@ audio_output {
>>                     "--no-daemon"
>>                     #$(mpd-config->file config))
>>               #:pid-file #$(mpd-file-name config "pid")
>> +             #:environment-variables
>> +             '(#$(string-append
>> +                   "XDG_RUNTIME_DIR=/run/user/"
>> +                   (number->string
>> +                     (passwd:uid 
>> +                       (getpwnam (mpd-configuration-user 
>> config))))))
>>               #:log-file #$(mpd-file-name config "log")))
>
> I suppose this is meant for the libpulse client library, right? 
> (I’m
> surprised it’s not the default, but apparently that’s the way it 
> is.)
>
> Note that this also has an effect on mpd itself from what I can 
> see in
> the source: mpd will listen to $XDG_RUNTIME_DIR/mpd/socket 
> instead of
> (in addition to?) listening on TCP.
>
> I don’t use mpd myself; could you ensure that the socket change 
> is also
> fine and doesn’t cause any troubles for mpd clients?
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#38075; Package guix-patches. (Fri, 08 Nov 2019 20:29:01 GMT) Full text and rfc822 format available.

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

From: Peter Mikkelsen <petermikkelsen10 <at> gmail.com>
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: Robert Smith <robertsmith <at> posteo.net>, 38075 <at> debbugs.gnu.org,
 Ludovic Courtès <ludo <at> gnu.org>,
 Leo Famulari <leo <at> famulari.name>
Subject: Re: [bug#38075] [PATCH] services: mpd: add pulseaudio support
Date: Fri, 8 Nov 2019 21:28:05 +0100
[Message part 1 (text/plain, inline)]
Hi Tobias,

Unfortunately I don't use mpd anymore and I don't have a good understanding
of how it works anymore.

Cheers,
Peter Mikkelsen

fre. 8. nov. 2019 20.53 skrev Tobias Geerinckx-Rice <me <at> tobias.gr>:

> Peter [CC'd],
>
> I see that you added the Guix MPD service in 2017, and wrote then
> that it ‘uses pulseaudio for output’[0].  Do you have an opinion
> on this patch[1]?  Did something change?
>
> Kind regards,
>
> T G-R
>
> [0]: http://guix.gnu.org/manual/en/html_node/Audio-Services.html
> [1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38075
>
> Ludovic Courtès 写道:
> > Robert Smith <robertsmith <at> posteo.net> skribis:
> >
> >> Set the XDG_RUNTIME_PATH environment variable in the mpd
> >> service so
> >> that it can detect pulseaudio when run under a user account.
> >> ---
> >>  gnu/services/audio.scm | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
> >> index 471c5fd95f..424ccc7c8f 100644
> >> --- a/gnu/services/audio.scm
> >> +++ b/gnu/services/audio.scm
> >> @@ -140,6 +140,12 @@ audio_output {
> >>                     "--no-daemon"
> >>                     #$(mpd-config->file config))
> >>               #:pid-file #$(mpd-file-name config "pid")
> >> +             #:environment-variables
> >> +             '(#$(string-append
> >> +                   "XDG_RUNTIME_DIR=/run/user/"
> >> +                   (number->string
> >> +                     (passwd:uid
> >> +                       (getpwnam (mpd-configuration-user
> >> config))))))
> >>               #:log-file #$(mpd-file-name config "log")))
> >
> > I suppose this is meant for the libpulse client library, right?
> > (I’m
> > surprised it’s not the default, but apparently that’s the way it
> > is.)
> >
> > Note that this also has an effect on mpd itself from what I can
> > see in
> > the source: mpd will listen to $XDG_RUNTIME_DIR/mpd/socket
> > instead of
> > (in addition to?) listening on TCP.
> >
> > I don’t use mpd myself; could you ensure that the socket change
> > is also
> > fine and doesn’t cause any troubles for mpd clients?
>
[Message part 2 (text/html, inline)]

Reply sent to Tobias Geerinckx-Rice <me <at> tobias.gr>:
You have taken responsibility. (Fri, 08 Nov 2019 20:36:01 GMT) Full text and rfc822 format available.

Notification sent to Robert Smith <robertsmith <at> posteo.net>:
bug acknowledged by developer. (Fri, 08 Nov 2019 20:36:02 GMT) Full text and rfc822 format available.

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

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: 38075-done <at> debbugs.gnu.org, Robert Smith <robertsmith <at> posteo.net>
Subject: Re: [bug#38075] [PATCH] services: mpd: add pulseaudio support
Date: Fri, 08 Nov 2019 21:34:58 +0100
[Message part 1 (text/plain, inline)]
Robert,

Thanks for the patch!

Robert Smith 写道:
> +             #:environment-variables
> +             '(#$(string-append
> +                   "XDG_RUNTIME_DIR=/run/user/"
> +                   (number->string
> +                     (passwd:uid 
> +                       (getpwnam (mpd-configuration-user 
> config))))))

Like Ludo', I'm surprised this isn't the default.  My XDG-foo is 
weak, though.

Your patch failed to break my set-up (better luck next time) so 
I've pushed it as 970cb5ceceaa85765230a9f896a43783cdcb4e6c with 
the following tweaks:

- Adjusted the commit message to follow the GNU change log 
 standards & Guix's conventions
- Promoted your commenty commit message to first-class comment
- Removed a trailing space after ‘passwd:uid’ (git diff/show/… 
 should highlight this)

Kind regards,

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

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 07 Dec 2019 12:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 140 days ago.

Previous Next


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