diff mbox

[1/1] add daemon mode support to pulseaudio

Message ID 1367584523-8491-1-git-send-email-jeremy.rosen@openwide.fr
State Accepted
Headers show

Commit Message

Jeremy Rosen May 3, 2013, 12:35 p.m. UTC
pulseaudio documentation recommands not using the daemon mode unless you
are on a headless system, but since that is the common case for a
buildroot installation, install the related user and groups

Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>
---
 package/multimedia/pulseaudio/Config.in     |   10 +++++++
 package/multimedia/pulseaudio/S50pulseaudio |   39 +++++++++++++++++++++++++++
 package/multimedia/pulseaudio/pulseaudio.mk |   13 +++++++++
 3 files changed, 62 insertions(+)
 create mode 100755 package/multimedia/pulseaudio/S50pulseaudio

Comments

Thomas Petazzoni May 3, 2013, 5:33 p.m. UTC | #1
Dear Jérémy Rosen,

The title of the patch should be:

	pulseaudio: add daemon mode support

i.e, always put the name of the affected package first.

On Fri,  3 May 2013 14:35:22 +0200, Jérémy Rosen wrote:
> pulseaudio documentation recommands not using the daemon mode unless you

recommends.

> are on a headless system, but since that is the common case for a
> buildroot installation, install the related user and groups

> +config BR2_PACKAGE_PULSEAUDIO_DAEMON
> +	bool "start pulseaudio as a system daemon"

	bool "start as a daemon"

should be sufficient, since the option will be indented under
pulseaudio.

> +	depends on BR2_PACKAGE_PULSEAUDIO
> +	help
> +	  PulseAudio can be started as a system daemon. This is not the
> +	  recommanded way of using PulseAudio unless you are building a

recommended.

> +	  headless system

Missing final dot. It would be good to explain how pulseaudio is
started when this option is not selected.

Thanks!

Thomas
Peter Korsgaard May 4, 2013, 3:16 p.m. UTC | #2
>>>>> "Jérémy" == Jérémy Rosen <jeremy.rosen@openwide.fr> writes:

 Jérémy> pulseaudio documentation recommands not using the daemon mode unless you
 Jérémy> are on a headless system, but since that is the common case for a
 Jérémy> buildroot installation, install the related user and groups


 Jérémy> Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>
 Jérémy> ---
 Jérémy>  package/multimedia/pulseaudio/Config.in     |   10 +++++++
 Jérémy>  package/multimedia/pulseaudio/S50pulseaudio |   39 +++++++++++++++++++++++++++
 Jérémy>  package/multimedia/pulseaudio/pulseaudio.mk |   13 +++++++++
 Jérémy>  3 files changed, 62 insertions(+)
 Jérémy>  create mode 100755 package/multimedia/pulseaudio/S50pulseaudio

 Jérémy> diff --git a/package/multimedia/pulseaudio/Config.in b/package/multimedia/pulseaudio/Config.in
 Jérémy> index 03c2613..bdc87ba 100644
 Jérémy> --- a/package/multimedia/pulseaudio/Config.in
 Jérémy> +++ b/package/multimedia/pulseaudio/Config.in
 Jérémy> @@ -19,5 +19,15 @@ config BR2_PACKAGE_PULSEAUDIO
 
 Jérémy>  	  http://pulseaudio.org
 
 Jérémy> +config BR2_PACKAGE_PULSEAUDIO_DAEMON
 Jérémy> +	bool "start pulseaudio as a system daemon"
 Jérémy> +	depends on BR2_PACKAGE_PULSEAUDIO
 Jérémy> +	help
 Jérémy> +	  PulseAudio can be started as a system daemon. This is not the
 Jérémy> +	  recommanded way of using PulseAudio unless you are building a
 Jérémy> +	  headless system
 Jérémy> +
 Jérémy> +
 Jérémy>  comment "pulseaudio requires a toolchain with WCHAR, LARGEFILE and threads support"
 Jérémy>  	depends on !BR2_USE_WCHAR || !BR2_LARGEFILE || !BR2_TOOLCHAIN_HAS_THREADS
 Jérémy> +
 Jérémy> diff --git a/package/multimedia/pulseaudio/S50pulseaudio b/package/multimedia/pulseaudio/S50pulseaudio
 Jérémy> new file mode 100755
 Jérémy> index 0000000..be71022
 Jérémy> --- /dev/null
 Jérémy> +++ b/package/multimedia/pulseaudio/S50pulseaudio
 Jérémy> @@ -0,0 +1,39 @@
 Jérémy> +#!/bin/sh
 Jérémy> +#
 Jérémy> +# Starts pulseaudio.
 Jérémy> +#
 Jérémy> +
 Jérémy> +
 Jérémy> +start() {
 Jérémy> + 	echo -n "Starting pulseaudio: "
 Jérémy> +	umask 077
 Jérémy> +	/usr/bin/pulseaudio --system --daemonize
 Jérémy> +	echo "OK"
 Jérémy> +}
 Jérémy> +stop() {
 Jérémy> +	echo -n "Stopping pulseaudio: "
 Jérémy> +	pulseaudio --kill
 Jérémy> +	echo "OK"
 Jérémy> +}
 Jérémy> +restart() {
 Jérémy> +	stop
 Jérémy> +	start
 Jérémy> +}
 Jérémy> +
 Jérémy> +case "$1" in
 Jérémy> +  start)
 Jérémy> +  	start
 Jérémy> +	;;
 Jérémy> +  stop)
 Jérémy> +  	stop
 Jérémy> +	;;

You use a mix of spaces and tabs for indentation in this script.

 Jérémy> +  restart|reload)
 Jérémy> +  	restart
 Jérémy> +	;;
 Jérémy> +  *)
 Jérémy> +	echo "Usage: $0 {start|stop|restart}"
 Jérémy> +	exit 1
 Jérémy> +esac
 Jérémy> +
 Jérémy> +exit $?
 Jérémy> +
 Jérémy> diff --git a/package/multimedia/pulseaudio/pulseaudio.mk b/package/multimedia/pulseaudio/pulseaudio.mk
 Jérémy> index e78eb42..d326dcc 100644
 Jérémy> --- a/package/multimedia/pulseaudio/pulseaudio.mk
 Jérémy> +++ b/package/multimedia/pulseaudio/pulseaudio.mk
 Jérémy> @@ -84,4 +84,17 @@ endef
 Jérémy>  PULSEAUDIO_POST_INSTALL_TARGET_HOOKS += PULSEAUDIO_REMOVE_VALA
 Jérémy>  endif
 
 Jérémy> +ifeq($(BR2_PACKAGE_PULSEAUDIO_DAEMON);y)

That seems wrong. It should be:

ifeq ($(BR2_PACKAGE_PULSEAUDIO_DAEMON),y)

 Jérémy> +define PULSEAUDIO_USERS
 Jérémy> +	pulse -1 pulse -1 * /var/run/pulse - audio,pulse-access
 Jérémy> +endef
 Jérémy> +
 Jérémy> +define PULSEAUDIO_INSTALL_INIT_SYSV
 Jérémy> +	[ -f $(TARGET_DIR)/etc/init.d/S50pulseaudio ] || \
 Jérémy> +		$(INSTALL) -D -m 755 package/multimedia/pulseaudio/S50pulseaudio \
 Jérémy> +			$(TARGET_DIR)/etc/init.d/S50pulseaudio
 Jérémy> +endef

As we now have support for post-build scripts, we no longer need the
hack about checking if the script is already present in the skeleton, so
that can be removed.

Committed with these fixes (and the typos Thomas pointed out), thanks.

Are you actively using pulseaudio? 3.0 got released some months ago,
care to test and send a patch to upgrade to it?
Jeremy Rosen May 6, 2013, 8:17 a.m. UTC | #3
> 
> As we now have support for post-build scripts, we no longer need the
> hack about checking if the script is already present in the skeleton,
> so
> that can be removed.
> 
> Committed with these fixes (and the typos Thomas pointed out),
> thanks.
> 
> Are you actively using pulseaudio? 3.0 got released some months ago,
> care to test and send a patch to upgrade to it?
> 


yes, I have a project using pulseaudio to build a headless remote audio system. i'll give it a try and report my findings
diff mbox

Patch

diff --git a/package/multimedia/pulseaudio/Config.in b/package/multimedia/pulseaudio/Config.in
index 03c2613..bdc87ba 100644
--- a/package/multimedia/pulseaudio/Config.in
+++ b/package/multimedia/pulseaudio/Config.in
@@ -19,5 +19,15 @@  config BR2_PACKAGE_PULSEAUDIO
 
 	  http://pulseaudio.org
 
+config BR2_PACKAGE_PULSEAUDIO_DAEMON
+	bool "start pulseaudio as a system daemon"
+	depends on BR2_PACKAGE_PULSEAUDIO
+	help
+	  PulseAudio can be started as a system daemon. This is not the
+	  recommanded way of using PulseAudio unless you are building a
+	  headless system
+
+
 comment "pulseaudio requires a toolchain with WCHAR, LARGEFILE and threads support"
 	depends on !BR2_USE_WCHAR || !BR2_LARGEFILE || !BR2_TOOLCHAIN_HAS_THREADS
+
diff --git a/package/multimedia/pulseaudio/S50pulseaudio b/package/multimedia/pulseaudio/S50pulseaudio
new file mode 100755
index 0000000..be71022
--- /dev/null
+++ b/package/multimedia/pulseaudio/S50pulseaudio
@@ -0,0 +1,39 @@ 
+#!/bin/sh
+#
+# Starts pulseaudio.
+#
+
+
+start() {
+ 	echo -n "Starting pulseaudio: "
+	umask 077
+	/usr/bin/pulseaudio --system --daemonize
+	echo "OK"
+}
+stop() {
+	echo -n "Stopping pulseaudio: "
+	pulseaudio --kill
+	echo "OK"
+}
+restart() {
+	stop
+	start
+}
+
+case "$1" in
+  start)
+  	start
+	;;
+  stop)
+  	stop
+	;;
+  restart|reload)
+  	restart
+	;;
+  *)
+	echo "Usage: $0 {start|stop|restart}"
+	exit 1
+esac
+
+exit $?
+
diff --git a/package/multimedia/pulseaudio/pulseaudio.mk b/package/multimedia/pulseaudio/pulseaudio.mk
index e78eb42..d326dcc 100644
--- a/package/multimedia/pulseaudio/pulseaudio.mk
+++ b/package/multimedia/pulseaudio/pulseaudio.mk
@@ -84,4 +84,17 @@  endef
 PULSEAUDIO_POST_INSTALL_TARGET_HOOKS += PULSEAUDIO_REMOVE_VALA
 endif
 
+ifeq($(BR2_PACKAGE_PULSEAUDIO_DAEMON);y)
+define PULSEAUDIO_USERS
+	pulse -1 pulse -1 * /var/run/pulse - audio,pulse-access
+endef
+
+define PULSEAUDIO_INSTALL_INIT_SYSV
+	[ -f $(TARGET_DIR)/etc/init.d/S50pulseaudio ] || \
+		$(INSTALL) -D -m 755 package/multimedia/pulseaudio/S50pulseaudio \
+			$(TARGET_DIR)/etc/init.d/S50pulseaudio
+endef
+
+endif
+
 $(eval $(autotools-package))