[pulseaudio-discuss] [V2 PATCH] rescue-streams: try to find best one using availability and priority

[David Henningsson]

Hi Hui,

Please see comments below:

On 2014-05-30 05:26, Hui Wang wrote:
> Recently met a problem: when I disconnect the bluetooth headset, the
> pulseaudio automatically switch the sound to sink of HDMI output
> instead of the sink of internal speaker even though there is no HDMI
> cable connected.
>
> To fix this problem, I want to change the rule of selecting the target
> sink(same rules apply to the source selecting):
> construct a new hashmap with all ports (of all relevant sinks) and
> then call find_best on the new hashmap to find the best port, finally
> find the corresponding sink using the best port.
>
> Signed-off-by: Hui Wang <hui.wang at canonical.com>
> ---
>   src/modules/module-rescue-streams.c | 95 +++++++++++++++++++++++++++----------
>   1 file changed, 71 insertions(+), 24 deletions(-)
>
> diff --git a/src/modules/module-rescue-streams.c b/src/modules/module-rescue-streams.c
> index 7035a35..eb40f90 100644
> --- a/src/modules/module-rescue-streams.c
> +++ b/src/modules/module-rescue-streams.c
> @@ -52,34 +52,77 @@ struct userdata {
>           *source_output_move_fail_slot;
>   };
>
> +static pa_source* find_source_from_port(pa_core *c, pa_device_port *port) {
> +    pa_source *target;
> +    uint32_t idx;
> +
> +    if (!port)
> +        return NULL;
> +
> +    PA_IDXSET_FOREACH(target, c->sources, idx)
> +	if (port == pa_hashmap_get(target->ports, port->name))
> +	    return target;
> +
> +    return NULL;
> +}
> +
> +static pa_sink* find_sink_from_port(pa_core *c, pa_device_port *port) {
> +    pa_sink *target;
> +    uint32_t idx;
> +
> +    if (!port)
> +        return NULL;
> +
> +    PA_IDXSET_FOREACH(target, c->sinks, idx)
> +	if (port == pa_hashmap_get(target->ports, port->name))
> +	    return target;
> +
> +    return NULL;
> +}
> +
> +static void build_group_ports(pa_hashmap *g_ports, pa_hashmap *s_ports) {
> +    void *state;
> +    pa_device_port *p;
> +
> +    if (!g_ports || !s_ports)
> +        return;
> +
> +    PA_HASHMAP_FOREACH(p, s_ports, state)
> +        pa_hashmap_put(g_ports, p->name, p);
> +}
> +
>   static pa_sink* find_evacuation_sink(pa_core *c, pa_sink_input *i, pa_sink *skip) {
> -    pa_sink *target, *def;
> +    pa_sink *target;
>       uint32_t idx;
> +    pa_hashmap *all_ports;
> +    pa_device_port *best_port;
>
>       pa_assert(c);
>       pa_assert(i);
>
> -    def = pa_namereg_get_default_sink(c);
> -
> -    if (def && def != skip && pa_sink_input_may_move_to(i, def))
> -        return def;

You have removed the rows above. Was that really necessary, i e, was the 
HDMI sink the default sink in this case?

> +    all_ports = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);

You probably want the  pa_idxset_trivial_* funcs here instead (and use 
port pointers as key rather than the name), because ports may have the 
same name if they belong to different sinks.

>
>       PA_IDXSET_FOREACH(target, c->sinks, idx) {
> -        if (target == def)
> -            continue;
> -
>           if (target == skip)
>               continue;
>
>           if (!PA_SINK_IS_LINKED(pa_sink_get_state(target)))
>               continue;
>
> -        if (pa_sink_input_may_move_to(i, target))
> -            return target;
> +        if (!pa_sink_input_may_move_to(i, target))
> +            continue;
> +
> +	build_group_ports(all_ports, target->ports);

The line above has wrong indentation.

>       }
>
> -    pa_log_debug("No evacuation sink found.");
> -    return NULL;
> +    best_port = pa_device_port_find_best(all_ports);
> +    if(!best_port)
> +	pa_log_debug("No evacuation sink found.");

We're still missing a fallback here: some sinks have no ports. I think 
we should still try to switch to them in case we did not find a best_port.

> +
> +    pa_hashmap_free(all_ports);
> +
> +    target = find_sink_from_port(c, best_port);
> +    return target;
>   }
>
>   static pa_hook_result_t sink_unlink_hook_callback(pa_core *c, pa_sink *sink, void* userdata) {
> @@ -141,21 +184,17 @@ static pa_hook_result_t sink_input_move_fail_hook_callback(pa_core *c, pa_sink_i
>   }
>
>   static pa_source* find_evacuation_source(pa_core *c, pa_source_output *o, pa_source *skip) {
> -    pa_source *target, *def;
> +    pa_source *target;
>       uint32_t idx;
> +    pa_hashmap *all_ports;
> +    pa_device_port *best_port;
>
>       pa_assert(c);
>       pa_assert(o);
>
> -    def = pa_namereg_get_default_source(c);
> -
> -    if (def && def != skip && pa_source_output_may_move_to(o, def))
> -        return def;
> +    all_ports = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
>
>       PA_IDXSET_FOREACH(target, c->sources, idx) {
> -        if (target == def)
> -            continue;
> -
>           if (target == skip)
>               continue;
>
> @@ -165,12 +204,20 @@ static pa_source* find_evacuation_source(pa_core *c, pa_source_output *o, pa_sou
>           if (!PA_SOURCE_IS_LINKED(pa_source_get_state(target)))
>               continue;
>
> -        if (pa_source_output_may_move_to(o, target))
> -            return target;
> +        if (!pa_source_output_may_move_to(o, target))
> +            continue;
> +
> +	build_group_ports(all_ports, target->ports);
>       }
>
> -    pa_log_debug("No evacuation source found.");
> -    return NULL;
> +    best_port = pa_device_port_find_best(all_ports);
> +    if(!best_port)
> +        pa_log_debug("No evacuation source found.");
> +
> +    pa_hashmap_free(all_ports);
> +
> +    target = find_source_from_port(c, best_port);
> +    return target;
>   }
>
>   static pa_hook_result_t source_unlink_hook_callback(pa_core *c, pa_source *source, void* userdata) {
>

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