[pulseaudio-discuss] [PATCH v2 4/8] core: move sink-inputs conditionally when update default_sink
Georg Chini
georg at chini.tk
Sat Jul 13 08:46:15 UTC 2019
On 30.06.19 13:52, Georg Chini wrote:
> On 17.01.19 07:53, Hui Wang wrote:
>> When the default sink changes, the streams from the old default sink
>> should be moved to the new default sink, unless the preferred_sink
>> string is set to the old default sink and the active port of the old
>> default sink is not unavailable
>>
>> Signed-off-by: Hui Wang <hui.wang at canonical.com>
>> ---
>> src/modules/dbus/iface-core.c | 2 +-
>> src/modules/module-default-device-restore.c | 2 +-
>> src/modules/module-switch-on-connect.c | 27 ++--------------
>> src/pulsecore/cli-command.c | 2 +-
>> src/pulsecore/core.c | 10 ++++--
>> src/pulsecore/core.h | 4 +--
>> src/pulsecore/device-port.c | 2 +-
>> src/pulsecore/protocol-native.c | 2 +-
>> src/pulsecore/sink.c | 35 +++++++++++++++++++--
>> src/pulsecore/sink.h | 6 ++++
>> 10 files changed, 54 insertions(+), 38 deletions(-)
>>
>> diff --git a/src/modules/dbus/iface-core.c
>> b/src/modules/dbus/iface-core.c
>> index 5229c0467..9763480d2 100644
>> --- a/src/modules/dbus/iface-core.c
>> +++ b/src/modules/dbus/iface-core.c
>> @@ -721,7 +721,7 @@ static void
>> handle_set_fallback_sink(DBusConnection *conn, DBusMessage *msg, DBu
>> return;
>> }
>> - pa_core_set_configured_default_sink(c->core,
>> pa_dbusiface_device_get_sink(fallback_sink)->name);
>> + pa_core_set_configured_default_sink(c->core,
>> pa_dbusiface_device_get_sink(fallback_sink)->name, true);
>> pa_dbus_send_empty_reply(conn, msg);
>> }
>> diff --git a/src/modules/module-default-device-restore.c
>> b/src/modules/module-default-device-restore.c
>> index c4dbad99f..33e74c071 100644
>> --- a/src/modules/module-default-device-restore.c
>> +++ b/src/modules/module-default-device-restore.c
>> @@ -69,7 +69,7 @@ static void load(struct userdata *u) {
>> pa_log_warn("Invalid sink name: %s", ln);
>> else {
>> pa_log_info("Restoring default sink '%s'.", ln);
>> - pa_core_set_configured_default_sink(u->core, ln);
>> + pa_core_set_configured_default_sink(u->core, ln, false);
>> }
>> } else if (errno != ENOENT)
>> diff --git a/src/modules/module-switch-on-connect.c
>> b/src/modules/module-switch-on-connect.c
>> index f0cb29a7c..3ceac8b4f 100644
>> --- a/src/modules/module-switch-on-connect.c
>> +++ b/src/modules/module-switch-on-connect.c
>> @@ -54,9 +54,6 @@ struct userdata {
>> };
>> static pa_hook_result_t sink_put_hook_callback(pa_core *c,
>> pa_sink *sink, void* userdata) {
>> - pa_sink_input *i;
>> - uint32_t idx;
>> - pa_sink *old_default_sink;
>> const char *s;
>> struct userdata *u = userdata;
>> @@ -88,7 +85,7 @@ static pa_hook_result_t
>> sink_put_hook_callback(pa_core *c, pa_sink *sink, void*
>> /* No default sink, nothing to move away, just set the new
>> default */
>> if (!c->default_sink) {
>> - pa_core_set_configured_default_sink(c, sink->name);
>> + pa_core_set_configured_default_sink(c, sink->name, false);
>> return PA_HOOK_OK;
>> }
>> @@ -103,28 +100,8 @@ static pa_hook_result_t
>> sink_put_hook_callback(pa_core *c, pa_sink *sink, void*
>> return PA_HOOK_OK;
>> }
>> - old_default_sink = c->default_sink;
>> -
>> /* Actually do the switch to the new sink */
>> - pa_core_set_configured_default_sink(c, sink->name);
>> -
>> - /* Now move all old inputs over */
>> - if (pa_idxset_size(old_default_sink->inputs) <= 0) {
>> - pa_log_debug("No sink inputs to move away.");
>> - return PA_HOOK_OK;
>> - }
>> -
>> - PA_IDXSET_FOREACH(i, old_default_sink->inputs, idx) {
>> - if (pa_safe_streq(i->sink->name, i->preferred_sink) ||
>> !PA_SINK_INPUT_IS_LINKED(i->state))
>> - continue;
>> -
>> - if (pa_sink_input_move_to(i, sink, false) < 0)
>> - pa_log_info("Failed to move sink input %u \"%s\" to
>> %s.", i->index,
>> - pa_strnull(pa_proplist_gets(i->proplist,
>> PA_PROP_APPLICATION_NAME)), sink->name);
>> - else
>> - pa_log_info("Successfully moved sink input %u \"%s\" to
>> %s.", i->index,
>> - pa_strnull(pa_proplist_gets(i->proplist,
>> PA_PROP_APPLICATION_NAME)), sink->name);
>> - }
>> + pa_core_set_configured_default_sink(c, sink->name, false);
>
> I wonder if we could use something like
> pa_core_set_temporary_default_sink() here
> and have a variable core->temporary_default_sink that has even higher
> priority
> than core->configured_default_sink in the default sink selection
> process. The temporary
> default sink could be reset when the sink vanishes again or replaced
> when another new
> sink turns up. What module-switch-on-connect does is to temporarily
> override the default
> sink that the user configured. If we save this in another variable we
> would not overwrite
> the user configured default sink by a sink that is not expected to be
> present the next
> time PA is started. But that would be just nice to have.
>
>> return PA_HOOK_OK;
>> }
>> diff --git a/src/pulsecore/cli-command.c b/src/pulsecore/cli-command.c
>> index 5205349bd..cc7addaa1 100644
>> --- a/src/pulsecore/cli-command.c
>> +++ b/src/pulsecore/cli-command.c
>> @@ -1036,7 +1036,7 @@ static int pa_cli_command_sink_default(pa_core
>> *c, pa_tokenizer *t, pa_strbuf *b
>> }
>> if ((s = pa_namereg_get(c, n, PA_NAMEREG_SINK)))
>> - pa_core_set_configured_default_sink(c, s->name);
>> + pa_core_set_configured_default_sink(c, s->name, true);
>> else
>> pa_strbuf_printf(buf, "Sink %s does not exist.\n", n);
>> diff --git a/src/pulsecore/core.c b/src/pulsecore/core.c
>> index cc4a6f38b..084a9f54b 100644
>> --- a/src/pulsecore/core.c
>> +++ b/src/pulsecore/core.c
>> @@ -226,7 +226,7 @@ static void core_free(pa_object *o) {
>> pa_xfree(c);
>> }
>> -void pa_core_set_configured_default_sink(pa_core *core, const char
>> *sink) {
>> +void pa_core_set_configured_default_sink(pa_core *core, const char
>> *sink, bool from_user) {
>> char *old_sink;
>> pa_assert(core);
>> @@ -241,7 +241,7 @@ void pa_core_set_configured_default_sink(pa_core
>> *core, const char *sink) {
>> pa_log_info("configured_default_sink: %s -> %s",
>> old_sink ? old_sink : "(unset)", sink ? sink :
>> "(unset)");
>> - pa_core_update_default_sink(core);
>> + pa_core_update_default_sink(core, from_user);
>> finish:
>> pa_xfree(old_sink);
>> @@ -306,7 +306,7 @@ static int compare_sinks(pa_sink *a, pa_sink *b) {
>> return 0;
>> }
>> -void pa_core_update_default_sink(pa_core *core) {
>> +void pa_core_update_default_sink(pa_core *core, bool from_user) {
>> pa_sink *best = NULL;
>> pa_sink *sink;
>> uint32_t idx;
>> @@ -343,6 +343,10 @@ void pa_core_update_default_sink(pa_core *core) {
>> pa_subscription_post(core, PA_SUBSCRIPTION_EVENT_SERVER |
>> PA_SUBSCRIPTION_EVENT_CHANGE, PA_INVALID_INDEX);
>> pa_hook_fire(&core->hooks[PA_CORE_HOOK_DEFAULT_SINK_CHANGED],
>> core->default_sink);
>> +
>> + /* try to move the streams from old_default_sink to the new
>> default_sink conditionally */
>> + if (old_default_sink)
>> + pa_sink_move_streams_to_default_sink(core, old_default_sink,
>> from_user);
>> }
>> /* a < b -> return -1
>> diff --git a/src/pulsecore/core.h b/src/pulsecore/core.h
>> index 38622f618..82573f001 100644
>> --- a/src/pulsecore/core.h
>> +++ b/src/pulsecore/core.h
>> @@ -243,7 +243,7 @@ enum {
>> pa_core* pa_core_new(pa_mainloop_api *m, bool shared, bool
>> enable_memfd, size_t shm_size);
>> -void pa_core_set_configured_default_sink(pa_core *core, const char
>> *sink);
>> +void pa_core_set_configured_default_sink(pa_core *core, const char
>> *sink, bool from_user);
>> void pa_core_set_configured_default_source(pa_core *core, const
>> char *source);
>> /* These should be called whenever something changes that may
>> affect the
>> @@ -255,7 +255,7 @@ void
>> pa_core_set_configured_default_source(pa_core *core, const char
>> *source);
>> * pa_core_update_default_source() internally, so it's sufficient
>> to only call
>> * pa_core_update_default_sink() when something happens that
>> affects the sink
>> * ordering. */
>> -void pa_core_update_default_sink(pa_core *core);
>> +void pa_core_update_default_sink(pa_core *core, bool from_user);
>> void pa_core_update_default_source(pa_core *core);
>> void pa_core_set_exit_idle_time(pa_core *core, int time);
>> diff --git a/src/pulsecore/device-port.c b/src/pulsecore/device-port.c
>> index fb1277215..464c3f8a2 100644
>> --- a/src/pulsecore/device-port.c
>> +++ b/src/pulsecore/device-port.c
>> @@ -96,7 +96,7 @@ void pa_device_port_set_available(pa_device_port
>> *p, pa_available_t status) {
>> * default sink/source, so port availability changes may
>> affect the
>> * default sink/source choice. */
>> if (p->direction == PA_DIRECTION_OUTPUT)
>> - pa_core_update_default_sink(p->core);
>> + pa_core_update_default_sink(p->core, false);
>> else
>> pa_core_update_default_source(p->core);
>> diff --git a/src/pulsecore/protocol-native.c
>> b/src/pulsecore/protocol-native.c
>> index 09e5aa3d5..7c6e5fb06 100644
>> --- a/src/pulsecore/protocol-native.c
>> +++ b/src/pulsecore/protocol-native.c
>> @@ -4354,7 +4354,7 @@ static void
>> command_set_default_sink_or_source(pa_pdispatch *pd, uint32_t comman
>> sink = pa_namereg_get(c->protocol->core, s, PA_NAMEREG_SINK);
>> CHECK_VALIDITY(c->pstream, sink, tag, PA_ERR_NOENTITY);
>> - pa_core_set_configured_default_sink(c->protocol->core, sink->name);
>> + pa_core_set_configured_default_sink(c->protocol->core, sink->name,
>> true);
>> }
>> pa_pstream_send_simple_ack(c->pstream, tag);
>> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
>> index d9851ce59..2d77188fd 100644
>> --- a/src/pulsecore/sink.c
>> +++ b/src/pulsecore/sink.c
>> @@ -721,7 +721,7 @@ 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);
>> + pa_core_update_default_sink(s->core, false);
>> }
>> /* Called from main context */
>> @@ -750,7 +750,7 @@ void pa_sink_unlink(pa_sink* s) {
>> pa_namereg_unregister(s->core, s->name);
>> pa_idxset_remove_by_data(s->core->sinks, s, NULL);
>> - pa_core_update_default_sink(s->core);
>> + pa_core_update_default_sink(s->core, false);
>> if (s->card)
>> pa_idxset_remove_by_data(s->card->sinks, s, NULL);
>> @@ -3416,7 +3416,7 @@ int pa_sink_set_port(pa_sink *s, const char
>> *name, bool save) {
>> pa_sink_set_port_latency_offset(s,
>> s->active_port->latency_offset);
>> /* The active port affects the default sink selection. */
>> - pa_core_update_default_sink(s->core);
>> + pa_core_update_default_sink(s->core, false);
>> pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SINK_PORT_CHANGED], s);
>> @@ -3922,3 +3922,32 @@ void
>> pa_sink_set_reference_volume_direct(pa_sink *s, const pa_cvolume
>> *volume) {
>> pa_subscription_post(s->core,
>> PA_SUBSCRIPTION_EVENT_SINK|PA_SUBSCRIPTION_EVENT_CHANGE, s->index);
>> pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SINK_VOLUME_CHANGED], s);
>> }
>> +
>> +void pa_sink_move_streams_to_default_sink(pa_core *core, pa_sink
>> *old_sink, bool from_user) {
>> + pa_sink_input *i;
>> + uint32_t idx;
>> + bool old_sink_is_unavailable;
> Does this not give a warning about using uninitialized variables?
>> +
>> + pa_assert(core);
>> + pa_assert(old_sink);
>> +
>> + if (old_sink == core->default_sink)
>> + return;
>> +
>> + if (old_sink->active_port && old_sink->active_port->available ==
>> PA_AVAILABLE_NO)
>> + old_sink_is_unavailable = true;
> This is not sufficient to determine if the old sink is still
> available. There are sinks
> that do not have ports (null-sink, virtual sinks). I think you will
> need another boolean
> argument to the function which indicates availability of the old sink.
>> +
>> + if (pa_idxset_size(old_sink->inputs) > 0) {
>> + PA_IDXSET_FOREACH(i, old_sink->inputs, idx) {
>> + if (!PA_SINK_INPUT_IS_LINKED(i->state))
>> + continue;
>> +
>> + if (pa_safe_streq(old_sink->name, i->preferred_sink) &&
>> !old_sink_is_unavailable)
>> + continue;
>> +
>> + pa_log_info("The sink input %u \"%s\" is moving to %s
>> due to default_sink is changed.",
>> + i->index,
>> pa_strnull(pa_proplist_gets(i->proplist, PA_PROP_APPLICATION_NAME)),
>> core->default_sink->name);
>> + pa_sink_input_move_to(i, core->default_sink, from_user);
>> + }
>> + }
>> +}
>> diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
>> index b9dd64f6f..b67d579dd 100644
>> --- a/src/pulsecore/sink.h
>> +++ b/src/pulsecore/sink.h
>> @@ -558,6 +558,12 @@ int64_t
>> pa_sink_get_latency_within_thread(pa_sink *s, bool allow_negative);
>> * s->reference_volume and fires change notifications. */
>> void pa_sink_set_reference_volume_direct(pa_sink *s, const
>> pa_cvolume *volume);
>> +/* When the default_sink is changed or the active_port of a sink
>> is changed to
>> + * PA_AVAILABLE_NO, this function is called to move the streams of
>> the old
>> + * default_sink or the sink with active_port equals PA_AVAILABLE_NO
>> to the
>> + * current default_sink conditionally*/
>> +void pa_sink_move_streams_to_default_sink(pa_core *core, pa_sink
>> *old_sink, bool from_user);
>> +
>> /* Verify that we called in IO context (aka 'thread context), or that
>> * the sink is not yet set up, i.e. the thread not set up yet. See
>> * pa_assert_io_context() in thread-mq.h for more information. */
To summarize the outcome of Tanu's and my discussion, the result
is that this patch remains unchanged apart from initializing
old_sink_is_unavailable before use.
More information about the pulseaudio-discuss
mailing list