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@xxxxxxxxxxxxx> --- 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. _______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
