[pulseaudio-discuss] [PATCH] module-switch-on-connect: add an argument for conditionally connecting

[David Henningsson]

On 2014-09-15 11:49, Hui Wang wrote:
> On a machine without fixed connecting audio devices like internal
> microphone or internal speaker, and when there is no external audio
> devices plugging in, the default source/sink is alsa_input/alsa_output
> and there is no input devices/output devices listed in the gnome
> sound-setting.
>
> Under this situation, if we connect a bluetooth headset, the gnome
> sound-setting will list bluez input/output devices, but they are not
> active devices by default. This looks very weird that sound-setting
> lists only one input device and one output device, but they are not
> active. To change this situation, we add an argument, the policy is
> if a new source/sink is connected and current default source/sink's
> active_port is AVAILABLE_NO, we let the new added one switch to
> default one.

Hi Hui,

In general I think this idea is good.

For the rest of you - as a reference, when I and Hui discussed this a 
while ago I suggested to make this part of module-switch-on-connect 
rather than module-switch-on-port-available. This is because I want, per 
Colin's old recommendation, to keep set_default_source and 
set_default_sink out of module-switch-on-port-available. It should 
mostly be set on direct user interaction, but since we already have 
set_default_source/sink in module-switch-on-connect I figured this would 
be an useful addition to this module.

But all in all, both module-switch-on-* modules are just bandaid after 
bandaid until Colin's priority lists are in place. IMO.

>
> BugLink: http://bugs.launchpad.net/bugs/1369476
> Signed-off-by: Hui Wang <hui.wang at canonical.com>
> ---
>   src/modules/module-switch-on-connect.c | 21 ++++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/src/modules/module-switch-on-connect.c b/src/modules/module-switch-on-connect.c
> index d9275d3..658f0141 100644
> --- a/src/modules/module-switch-on-connect.c
> +++ b/src/modules/module-switch-on-connect.c
> @@ -38,11 +38,15 @@
>   #include "module-switch-on-connect-symdef.h"
>
>   PA_MODULE_AUTHOR("Michael Terry");
> -PA_MODULE_DESCRIPTION("When a sink/source is added, switch to it");
> +PA_MODULE_DESCRIPTION("When a sink/source is added, switch to it or conditionally switch to it");
>   PA_MODULE_VERSION(PACKAGE_VERSION);
>   PA_MODULE_LOAD_ONCE(true);
> +PA_MODULE_USAGE(
> +        "switch_on_unavailable=<only switch when default source/sink's active_port is unavailable> "
> +);

I think "only_from_unavailable" would be a better name, thus this 
becomes something like:
"only_from_unavailable=<boolean, only switch from unavailable ports>"

>
>   static const char* const valid_modargs[] = {
> +    "switch_on_unavailable",
>       NULL,
>   };
>
> @@ -50,6 +54,7 @@ struct userdata {
>       pa_hook_slot
>           *sink_put_slot,
>           *source_put_slot;
> +    bool switch_on_unavailable;
>   };
>
>   static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void* userdata) {
> @@ -57,9 +62,11 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void*
>       uint32_t idx;
>       pa_sink *def;
>       const char *s;
> +    struct userdata *u = userdata;
>
>       pa_assert(c);
>       pa_assert(sink);
> +    pa_assert(userdata);
>
>       /* Don't want to run during startup or shutdown */
>       if (c->state != PA_CORE_RUNNING)
> @@ -77,6 +84,10 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void*
>       if (def == sink)
>           return PA_HOOK_OK;
>
> +    if (u->switch_on_unavailable)
> +        if (def->active_port && def->active_port->available != PA_AVAILABLE_NO)
> +            return PA_HOOK_OK;

This looks a bit wrong. It looks like you will still switch from 
sinks/sources that don't have ports, even if switch_on_unavailable=true.

> +
>       /* Actually do the switch to the new sink */
>       pa_namereg_set_default_sink(c, sink);
>
> @@ -106,9 +117,11 @@ static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source,
>       uint32_t idx;
>       pa_source *def;
>       const char *s;
> +    struct userdata *u = userdata;
>
>       pa_assert(c);
>       pa_assert(source);
> +    pa_assert(userdata);
>
>       /* Don't want to run during startup or shutdown */
>       if (c->state != PA_CORE_RUNNING)
> @@ -130,6 +143,10 @@ static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source,
>       if (def == source)
>           return PA_HOOK_OK;
>
> +    if (u->switch_on_unavailable)
> +        if (def->active_port && def->active_port->available != PA_AVAILABLE_NO)
> +            return PA_HOOK_OK;
> +
>       /* Actually do the switch to the new source */
>       pa_namereg_set_default_source(c, source);
>
> @@ -171,6 +188,8 @@ int pa__init(pa_module*m) {
>       u->sink_put_slot = pa_hook_connect(&m->core->hooks[PA_CORE_HOOK_SINK_PUT], PA_HOOK_LATE+30, (pa_hook_cb_t) sink_put_hook_callback, u);
>       u->source_put_slot = pa_hook_connect(&m->core->hooks[PA_CORE_HOOK_SOURCE_PUT], PA_HOOK_LATE+20, (pa_hook_cb_t) source_put_hook_callback, u);
>
> +    pa_modargs_get_value_boolean(ma, "switch_on_unavailable", &u->switch_on_unavailable);
> +

Missing error check of pa_modargs_get_value_boolean.

>       pa_modargs_free(ma);
>       return 0;
>   }
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic