[pulseaudio-discuss] [PATCH 3/4] move streams to new appeared sinks if they prefer these sinks

[Tanu Kaskinen]

On Mon, 2018-11-05 at 09:47 +0800, Hui Wang wrote:
> When a new sink appears, all streams that have their "preferred_sink"
> set to the new sink should be moved to the new sink.
> Signed-off-by: Hui Wang <hui.wang at canonical.com>
> ---
>  src/pulsecore/sink.c | 31 +++++++++++++++++++++++++++++++
>  src/pulsecore/sink.h |  1 +
>  2 files changed, 32 insertions(+)

The sink_put_hook_callback() function in module-stream-restore.c is
obsolete after this change, so it should be removed.

> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
> index 63a3456e7..a2a390beb 100644
> --- a/src/pulsecore/sink.c
> +++ b/src/pulsecore/sink.c
> @@ -722,6 +722,8 @@ void pa_sink_put(pa_sink* s) {
>      /* This function must be called after the PA_CORE_HOOK_SINK_PUT hook,
>       * because module-switch-on-connect needs to know the old default sink */
>      pa_core_update_default_sink(s->core, false);
> +
> +    pa_sink_bind_preferred_stream_to_a_sink(s);
>  }
>  
>  /* Called from main context */
> @@ -3919,3 +3921,32 @@ void pa_sink_move_streams_from_oldsink_to_newsink(pa_sink *old_sink, pa_sink *ne
>  
>      return;
>  }
> +
> +void pa_sink_bind_preferred_stream_to_a_sink(pa_sink *s) {

"Bind" is new terminology, and I'd like to avoid introducing new
terminology if possible. Also, "preferred stream" as a term doesn't
really make sense. So some better name for the function would be
desirable, but I can't immediately think of any obvious names... The
function is about moving streams to a sink that just became available.
"Move" and "streams" should be included in the name... Maybe
"pa_sink_move_streams_to_newly_available_sink()"? I also suggest moving
it away from the pa_sink namespace, because the function operates on
streams, not on a sink. It could be added to the pa_sink_input
namespace, but since the function operates on multiple streams rather
than just one, I think pa_core would be a bit better namespace (but I
don't have a very strong opinion on this).

> +    pa_sink_input *si;
> +    uint32_t idx;
> +    pa_core *c;
> +
> +    if (!s)

This should be an assertion.

> +	return;
> +
> +    c = s->core;
> +    PA_IDXSET_FOREACH(si, c->sink_inputs, idx) {
> +        if (si->sink == s)
> +            continue;
> +
> +        /* Skip this sink input if it is connecting a filter sink to
> +         * the master */
> +        if (si->origin_sink)
> +            continue;

I think it't better to ensure that preferred_sink is not set when it
doesn't make sense (and it doesn't make sense with filter sinks, at
least in most cases). So rather than checking si->origin_sink here,
it's better to check this in the code that sets preferred_sink. There
are two situations where preferred_sink is set: when the user moves a
stream, and when module-stream-restore restores the user choice. It
should be sufficient to check this when the user moves the stream,
because module-stream-restore only replicates the moves done by the
user.

> +
> +        /* It might happen that a stream and a sink are set up at the
> +           same time, in which case we want to make sure we don't
> +           interfere with that */
> +	if (!PA_SINK_INPUT_IS_LINKED(si->state))
> +            continue;
> +
> +        if (si->preferred_sink != NULL && pa_streq(si->preferred_sink, s->name))

pa_safe_streq() can be used to avoid the NULL check.

> +           pa_sink_input_move_to(si, s, false);
> +    }
> +}
> diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
> index f207a094c..24e4678b1 100644
> --- a/src/pulsecore/sink.h
> +++ b/src/pulsecore/sink.h
> @@ -562,4 +562,5 @@ void pa_sink_set_reference_volume_direct(pa_sink *s, const pa_cvolume *volume);
>      pa_assert(pa_thread_mq_get() || !PA_SINK_IS_LINKED((s)->state))
>  
>  void pa_sink_move_streams_from_oldsink_to_newsink(pa_sink *old_sink, pa_sink *new_sink, bool from_user);
> +void pa_sink_bind_preferred_stream_to_a_sink(pa_sink *s);
>  #endif

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk