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 15.07.19 14:42, Tanu Kaskinen wrote:
On Sat, 2019-07-13 at 11:31 +0200, Georg Chini wrote:
On 13.07.19 10:46, Georg Chini wrote:
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@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.

         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.

I just realized there is one more change necessary. You should
check if the sink input may move to the new default sink with
pa_sink_input_may_move_to() before executing the move and
if it may not move, keep the sink input on the old sink.
Again the example of a virtual sink that you set up on top of the
default sink. If you now configure the virtual sink as default, its
sink input should not move.
Isn't this unnecessary, since pa_move_sink_input_to() will check
pa_sink_input_may_move_to() anyway?

Yes, nothing bad will happen, you are right. But we still should
check it here I think, at least to avoid a false log message.

_______________________________________________
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