Merge lp:~zequence/pulseaudio/module-jackdbus-detect-channel-config into lp:~ubuntu-audio-dev/pulseaudio/ubuntu

[Kaj Ailomaa]

Add channel configuration for module-jackdbus-detect and set the default to 2, as jack is not well adapted for anything beyond mono or stereo connections (i.e. 5.1, 7.1 and similar are difficult to use with jack).

[David Henningsson (diwic)]

Hi and thanks for your merge proposal,

I'm wondering about "0022-module-jackdbus-detect-channel-config-default.patch" - isn't 2 already the default if no channels argument is specified?

review: Needs Information

[Kaj Ailomaa (zequence)]

It'll create as many channels as there are channels on the audio device
that jack is running.
So, if there are 8 I/O on the audio device, the module will create a 7.1
interface for it with labels like "left", "right", "front", "back", etc.
If more than 8, instead of being called "left", "right", "front", etc in
the PA interface, you get "aux0", "aux1"..
If there are only 2 I/O, then the module creates a stereo interface.

So, making sure the module always uses a stereo interface by default
should help integrate PA easier with jack, as jack thinks more in terms of
multiple mono channels.

On Thu, 21 Feb 2013 23:08:17 +0100, David Henningsson
<email address hidden> wrote:

> Review: Needs Information
>
> Hi and thanks for your merge proposal,
>
> I'm wondering about
> "0022-module-jackdbus-detect-channel-config-default.patch" - isn't 2
> already the default if no channels argument is specified?

[Kaj Ailomaa (zequence)]

On Fri, 22 Feb 2013 01:06:59 +0100, Kaj Ailomaa <email address hidden>
wrote:

> If more than 8, instead of being called "left", "right", "front", etc in
> the PA interface, you get "aux0", "aux1"..

..on top of the existing "left", "right", "front"

[David Henningsson (diwic)]

Well, if a user has a 5.1 card, and wants to use JACK with it and PulseAudio on top of that, is there something wrong with providing a sink with a channel map for all six channels that matches that? I'm not following.

[Kaj Ailomaa (zequence)]

I think the case when someone wants to use 5.1 is much rarer than using
stereo. I also think of this solution as a temporary one, one which I
believe most jack users will prefer, as most jack users use stereo
monitoring for audio production.

It would be best if one could choose a profile for the jack sink and
source from the PA mixer, as you can for devices (I guess those are based
on alsa vard profiles?).

[Kaj Ailomaa (zequence)]

Or, for rather, a profile for the output. The input I don't really know how that should work, what PA is considered.

[David Henningsson (diwic)]

review: Approve

[David Henningsson (diwic)]

Merged and uploaded, thanks for your contribution!

=== modified file 'debian/changelog'
--- debian/changelog	2013-02-21 10:48:08 +0000
+++ debian/changelog	2013-02-21 13:22:53 +0000
@@ -1,3 +1,15 @@
+pulseaudio (1:3.0-0ubuntu4) UNRELEASED; urgency=low
+
+  * 0108-module-jackdbus-detect-channel-config.patch
+    0109-module-jackdbus-detect-channel-config-fix.patch
+    Adds channel configuration to module-jackdbus-detect
+  * 0022-module-jackdbus-detect-channel-config-default.patch
+    Sets default channel config for module-jackdbus-detect to 2,
+    as jack is not well adapted for anything beyond mono or stereo
+    connections.
+
+ -- Kaj Ailomaa <[e-mail]>  Thu, 21 Feb 2013 14:13:50 +0100
+
 pulseaudio (1:3.0-0ubuntu3) raring; urgency=low
 
   * 0106-reserve-Move-get_name_owner-to-the-public-rd_device-.patch,

=== added file 'debian/patches/0022-module-jackdbus-detect-channel-config-default.patch'
--- debian/patches/0022-module-jackdbus-detect-channel-config-default.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/0022-module-jackdbus-detect-channel-config-default.patch	2013-02-21 13:22:53 +0000
@@ -0,0 +1,14 @@
+Description: Sets the default number of channels for module-jackdbus-detect to two
+Author: Kaj Ailomaa <[e-mail]>
+
+--- pulseaudio-3.0.orig/src/daemon/default.pa.in
++++ pulseaudio-3.0/src/daemon/default.pa.in
+@@ -78,7 +78,7 @@ load-module module-detect
+ ### Automatically connect sink and source if JACK server is present
+ .ifexists module-jackdbus-detect@PA_SOEXT@
+ .nofail
+-load-module module-jackdbus-detect
++load-module module-jackdbus-detect channels=2
+ .fail
+ .endif
+ 

=== added file 'debian/patches/0108-module-jackdbus-detect-channel-config.patch'
--- debian/patches/0108-module-jackdbus-detect-channel-config.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/0108-module-jackdbus-detect-channel-config.patch	2013-02-21 13:22:53 +0000
@@ -0,0 +1,78 @@
+From: commit c1f36347298214eca090a394451fb5bc82ae28aa
+Author: Peter Nelson <[e-mail]>
+Date:   Sat Dec 22 22:18:52 2012 +0000
+
+    jackdbus-detect: Add channel count override.
+    
+    Allow configuration of number of channels when using module-jackdbus-detect
+    to load jack-sink and jack-source. This is useful when the default channel
+    count doesn't match the logical channel count desired, e.g. with multi-
+    channel audio interfaces.
+    
+    Signed-off-by: Peter Nelson <[e-mail]>
+
+diff --git a/src/modules/jack/module-jackdbus-detect.c b/src/modules/jack/module-jackdbus-detect.c
+index 10408ea..ba0b364 100644
+--- a/src/modules/jack/module-jackdbus-detect.c
++++ b/src/modules/jack/module-jackdbus-detect.c
+@@ -40,7 +40,9 @@ PA_MODULE_AUTHOR("David Henningsson");
+ PA_MODULE_DESCRIPTION("Adds JACK sink/source ports when JACK is started");
+ PA_MODULE_LOAD_ONCE(TRUE);
+ PA_MODULE_VERSION(PACKAGE_VERSION);
+-PA_MODULE_USAGE("connect=<connect ports?>");
++PA_MODULE_USAGE(
++    "channels=<number of channels> "
++    "connect=<connect ports?>");
+ 
+ #define JACK_SERVICE_NAME "org.jackaudio.service"
+ #define JACK_INTERFACE_NAME "org.jackaudio.JackControl"
+@@ -60,6 +62,7 @@ PA_MODULE_USAGE("connect=<connect ports?>");
+ 	"member='" _a "'"
+ 
+ static const char* const valid_modargs[] = {
++    "channels",
+     "connect",
+     NULL
+ };
+@@ -81,6 +84,7 @@ struct userdata {
+     pa_bool_t filter_added, match_added;
+     pa_bool_t is_service_started;
+     pa_bool_t autoconnect_ports;
++    uint32_t channels;
+     /* Using index here protects us from module unloading without us knowing */
+     int jack_module_index[JACK_SS_COUNT];
+ };
+@@ -106,7 +110,11 @@ static void ensure_ports_started(struct userdata* u) {
+         if (!u->jack_module_index[i]) {
+             char* args;
+             pa_module* m;
+-            args = pa_sprintf_malloc("connect=%s", pa_yes_no(u->autoconnect_ports));
++            if (u->channels > 0) {
++                args = pa_sprintf_malloc("connect=%s channels=%" PRIu32, pa_yes_no(u->autoconnect_ports), u->channels);
++            } else {
++                args = pa_sprintf_malloc("connect=%s", pa_yes_no(u->autoconnect_ports));
++            }
+             m = pa_module_load(u->core, modnames[i], args);
+             pa_xfree(args);
+ 
+@@ -226,12 +234,20 @@ int pa__init(pa_module *m) {
+     u->core = m->core;
+     u->module = m;
+     u->autoconnect_ports = TRUE;
++    u->channels = 0;
+ 
+     if (pa_modargs_get_value_boolean(ma, "connect", &u->autoconnect_ports) < 0) {
+         pa_log("Failed to parse connect= argument.");
+         goto fail;
+     }
+ 
++    if (pa_modargs_get_value_u32(ma, "channels", &u->channels) < 0 ||
++        u->channels <= 0 ||
++        u->channels > PA_CHANNELS_MAX) {
++        pa_log("Failed to parse channels= argument.");
++        goto fail;
++    }
++
+     if (!(connection = pa_dbus_bus_get(m->core, DBUS_BUS_SESSION, &error)) || dbus_error_is_set(&error)) {
+ 
+         if (connection)

=== added file 'debian/patches/0109-module-jackdbus-detect-channel-config-fix.patch'
--- debian/patches/0109-module-jackdbus-detect-channel-config-fix.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/0109-module-jackdbus-detect-channel-config-fix.patch	2013-02-21 13:22:53 +0000
@@ -0,0 +1,27 @@
+From: commit f26bcae28e1064d7eab48ee0d29a8658ac19acf5
+Author: Tanu Kaskinen <[e-mail]>
+Date:   Tue Jan 22 08:42:27 2013 +0200
+
+    jack: Don't fail module-jackdbus-detect loading if the channels argument is not given.
+    
+    The u->channels <= 0 check failed if the channels argument was not
+    given at all, making the whole module loading fail. I don't think the
+    check is necessary at all - negative values are not possible, and if
+    someone gives 0 as the argument, it's probably ok if we act as if
+    there was no channels argument at all.
+
+diff --git a/src/modules/jack/module-jackdbus-detect.c b/src/modules/jack/module-jackdbus-detect.c
+index ba0b364..25ac495 100644
+--- a/src/modules/jack/module-jackdbus-detect.c
++++ b/src/modules/jack/module-jackdbus-detect.c
+@@ -241,9 +241,7 @@ int pa__init(pa_module *m) {
+         goto fail;
+     }
+ 
+-    if (pa_modargs_get_value_u32(ma, "channels", &u->channels) < 0 ||
+-        u->channels <= 0 ||
+-        u->channels > PA_CHANNELS_MAX) {
++    if (pa_modargs_get_value_u32(ma, "channels", &u->channels) < 0 || u->channels > PA_CHANNELS_MAX) {
+         pa_log("Failed to parse channels= argument.");
+         goto fail;
+     }

=== modified file 'debian/patches/series'
--- debian/patches/series	2013-02-21 10:48:08 +0000
+++ debian/patches/series	2013-02-21 13:22:53 +0000
@@ -7,6 +7,7 @@
 0019-disable-dbus-protocol.patch
 0020-stream-Return-error-in-case-a-client-peeks-to-early.patch
 0021-disable-ucm.patch
+0022-module-jackdbus-detect-channel-config-default.patch
 
 # From upstream
 0101-alsa-mixer-Fix-the-analog-output-speaker-always-path.patch
@@ -16,3 +17,4 @@
 0105-udev-Add-use_ucm-module-argument.patch
 0106-reserve-Move-get_name_owner-to-the-public-rd_device-.patch
 0107-reserve-Fix-leaking-NameLost-signals-after-release-a.patch
+0108-module-jackdbus-detect-channel-config.patch