Re: [PATCH v2 4/8] core: move sink-inputs conditionally when update default_sink

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
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).



+
+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.
The definition of an unavailable sink is that its active port is
unavailable. I don't know what kind of sinks you're thinking about
here, maybe virtual sinks that are on top of an unavailable hardware
sink? There are many places where we check the availability of a sink
this way, and I don't think it makes sense to be different here.

I don't understand. Virtual sinks don't have ports. So checking only
sinks that have an active port excludes all sinks that don't have
ports like the null-sink and virtual sinks. Following your definition
above, those sinks are never available.


_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss




[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux