[pulseaudio-discuss] [PATCH v2 4/8] core: move sink-inputs conditionally when update default_sink
Georg Chini
georg at chini.tk
Tue Jul 2 07:08:02 UTC 2019
On 02.07.19 08:43, Tanu Kaskinen wrote:
> On Mon, 2019-07-01 at 08:03 +0200, Georg Chini wrote:
>> On 01.07.19 07:08, Tanu Kaskinen wrote:
>>> On Sun, 2019-06-30 at 13:52 +0200, 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.
>>> I'm against adding that kind of extra complexity without a very good
>>> justification. module-switch-on-connect should become nearly obsolete
>>> anyway after this patch set has been merged.
>> We already talked about this some time ago and you agreed
>> that this would be useful. Maybe you remember that discussion.
> I remember that we discussed my initial attempt at implementing what
> this patch set does. I probably should go back to that discussion to
> find out why I agreed that we should add a separate "temporary default
> sink" variable.
You did not exactly agree to adding a temporary_default_sink
variable. You agreed however (after some discussion) that
what module-switch-on-connect does is a temporary override
of the configured default sink and that there should be a way
to restore the original default sink. (The user may have set
another than the highest priority sink as default, see also
below).
It seems quite simple to add such a variable and replace it
when a new sink turns up or set it to NULL if the current
temporary default sink is removed. This would then make
it really easy to implement the module-switch-on-connect
functionality in the core at some later point.
>
>> Consider you add your BT speakers and module-switch-on-connect
>> makes them the default sink. When you remove them, there is
>> no guarantee that the default sink will revert to what the user
>> configured before. Instead PA will choose the sink which it thinks
>> is best. Also, if you remove the device after a shutdown, PA will
>> still keep it as the configured default sink and the user will loose
>> what was configured.
>>
>> So in my opinion we should distinguish if the server changed the
>> default sink or the user did, though as already said above, it is
>> not mandatory but only nice to have.
>>
>> I do not see why module-switch-on-connect would become obsolete
>> - it switches the default sink to a newly appeared sink and there is
>> nothing in these patches which does that (unless the newly appeared
>> sink is the configured default sink).
> When a new sink appears, pa_core_update_default_sink() is called. Since
> PulseAudio 11.1, bluetooth and USB sinks have higher priority than the
> internal sound card, so pa_core_update_default_sink() will switch to
> the BT speakers in your example. The main benefit of module-switch-on-
> connect was that it moved existing streams to the new sink, but after
> this patch set that's handled by the core. Therefore there's much less
> need for module-switch-on-connect.
>
This is true, but only relevant if there is no configured default
sink. If the configured default sink is set, there will be no switch
to a newly appearing sink because the configured default sink
is prioritized over all other sinks. In this case you still need
module-switch-on-connect.
More information about the pulseaudio-discuss
mailing list