[2/4] move stream after default sink is changed.

Submitted by Hui Wang on Nov. 5, 2018, 1:47 a.m.

Details

Message ID 20181105014716.5613-3-hui.wang@canonical.com
State New
Series "The tentative fix of the issue #185"
Headers show

Commit Message

Hui Wang Nov. 5, 2018, 1:47 a.m.
When default sink changes, the stream should be moved to new
default_sink too. If it is user to call change default function,
all stream will move to new default sink unconditionally; if it
is not, will check if stream binds to old_default_sink and the
active_port staus of old_default_sink, then it will move the
stream conditionally.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 src/modules/dbus/iface-core.c               |  2 +-
 src/modules/module-default-device-restore.c |  2 +-
 src/modules/module-switch-on-connect.c      | 28 ++-------------
 src/pulsecore/cli-command.c                 |  2 +-
 src/pulsecore/core.c                        |  9 +++--
 src/pulsecore/core.h                        |  4 +--
 src/pulsecore/device-port.c                 |  2 +-
 src/pulsecore/protocol-native.c             |  2 +-
 src/pulsecore/sink.c                        | 39 +++++++++++++++++++--
 src/pulsecore/sink.h                        |  1 +
 10 files changed, 52 insertions(+), 39 deletions(-)

Patch hide | download patch | download mbox

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 faa0f289f..4f01c701f 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;
 
@@ -85,7 +82,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;
     }
 
@@ -100,29 +97,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 (i->preferred_sink != NULL || !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);
     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..7d85e1788 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,9 @@  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 */
+    pa_sink_move_streams_from_oldsink_to_newsink(old_default_sink, core->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 566367f9e..63a3456e7 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);
@@ -3417,7 +3417,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);
 
@@ -3886,3 +3886,36 @@  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_from_oldsink_to_newsink(pa_sink *old_sink, pa_sink *new_sink, bool from_user) {
+    pa_sink_input *i;
+    uint32_t idx;
+    pa_device_port *o_active_p;
+
+    if (old_sink == new_sink)
+	return;
+
+    if (old_sink == NULL)
+	return;
+
+    o_active_p = old_sink->active_port;
+    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 (!from_user && i->preferred_sink != NULL && pa_safe_streq(old_sink->name, i->preferred_sink))
+		    if (o_active_p->available != PA_AVAILABLE_NO)
+			continue;
+
+		if (pa_sink_input_move_to(i, new_sink, from_user) < 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)), new_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)), new_sink->name);
+	    }
+    }
+
+    return;
+}
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index 8f254408f..f207a094c 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -561,4 +561,5 @@  void pa_sink_set_reference_volume_direct(pa_sink *s, const pa_cvolume *volume);
 #define pa_sink_assert_io_context(s) \
     pa_assert(pa_thread_mq_get() || !PA_SINK_IS_LINKED((s)->state))
 
+void pa_sink_move_streams_from_oldsink_to_newsink(pa_sink *old_sink, pa_sink *new_sink, bool from_user);
 #endif

Comments

Tanu Kaskinen Dec. 13, 2018, 9:16 a.m.
On Mon, 2018-11-05 at 09:47 +0800, Hui Wang wrote:
> When default sink changes, the stream should be moved to new
> default_sink too.

Except if the stream's preferred sink is the old default sink.

> If it is user to call change default function,
> all stream will move to new default sink unconditionally; if it
> is not, will check if stream binds to old_default_sink and the
> active_port staus of old_default_sink, then it will move the
> stream conditionally.

Why does it matter if the default sink changed due to user request or
some other reason? I don't think streams should be moved
unconditionally when the user changes the default sink.

I think it would be logical to not care about the port unavailability
in this patch, because there's a separate patch for handling that.

> diff --git a/src/modules/module-switch-on-connect.c b/src/modules/module-switch-on-connect.c
> index faa0f289f..4f01c701f 100644
> --- a/src/modules/module-switch-on-connect.c
> +++ b/src/modules/module-switch-on-connect.c

> @@ -100,29 +97,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 */

This comment isn't very helpful any more (not sure how helpful it was
before either), I'd remove it.

> -    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 (i->preferred_sink != NULL || !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);
>      return PA_HOOK_OK;
>  }

> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
> index 566367f9e..63a3456e7 100644
> --- a/src/pulsecore/sink.c
> +++ b/src/pulsecore/sink.c

> @@ -3886,3 +3886,36 @@ 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_from_oldsink_to_newsink(pa_sink *old_sink, pa_sink *new_sink, bool from_user) {

I think "pa_sink_move_streams_to_default_sink" would be a better name
for the function. It would also be good to have a comment explaining
when the function is used and which streams are not moved (that could
go to the header file).

> +    pa_sink_input *i;
> +    uint32_t idx;
> +    pa_device_port *o_active_p;
> +
> +    if (old_sink == new_sink)
> +	return;
> +
> +    if (old_sink == NULL)
> +	return;

It doesn't make sense to call this function with NULL, so this check
should be an assertion instead.

> +
> +    o_active_p = old_sink->active_port;

Note that not all sinks have ports, so this can be NULL.

Since we only care about the availability of the active port, I'd add a
"old_sink_is_unavailable" boolean to be used inside the loop.

> +    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 (!from_user && i->preferred_sink != NULL && pa_safe_streq(old_sink->name, i->preferred_sink))

No need to check if i->preferred_sink is NULL, because pa_safe_streq()
checks that anyway.

> +		    if (o_active_p->available != PA_AVAILABLE_NO)
> +			continue;
> +
> +		if (pa_sink_input_move_to(i, new_sink, from_user) < 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)), new_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)), new_sink->name);

Logging the reason for moving the stream before calling
pa_sink_input_move_to() would be very useful.

Here's a suggestion you don't need to implement, but you can if you
want: it would be nice to have good enough logging in
pa_sink_input_move_finish() so that callers don't all need to do their
own logging about the move result. This should be done in a separate
patch.
Hui Wang Dec. 15, 2018, 6:48 a.m.
On 2018/12/13 下午5:16, Tanu Kaskinen wrote:
> On Mon, 2018-11-05 at 09:47 +0800, Hui Wang wrote:
>> When default sink changes, the stream should be moved to new
>> default_sink too.
> Except if the stream's preferred sink is the old default sink.
>
>> If it is user to call change default function,
>> all stream will move to new default sink unconditionally; if it
>> is not, will check if stream binds to old_default_sink and the
>> active_port staus of old_default_sink, then it will move the
>> stream conditionally.
> Why does it matter if the default sink changed due to user request or
> some other reason? I don't think streams should be moved
> unconditionally when the user changes the default sink.

Supposing  the sink0 is hdmi, sink1 is analog-speaker,  and a music is 
playing on sink0 (default_sink is sink0),  if users select the 
analog-speaker from UI (gnome-sound-setting), the sink1 is default_sink 
now, and the music should be played on sink1.  If the streams don't move 
to new default_sink unconditionally, I have no idea how to let music be 
played on sink1 here.


For the rest comments, I will fix them as suggested.


Thanks.


>
> I think it would be logical to not care about the port unavailability
> in this patch, because there's a separate patch for handling that.
>
>> diff --git a/src/modules/module-switch-on-connect.c b/src/modules/module-switch-on-connect.c
>> index faa0f289f..4f01c701f 100644
>> --- a/src/modules/module-switch-on-connect.c
>> +++ b/src/modules/module-switch-on-connect.c
>> @@ -100,29 +97,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 */
> This comment isn't very helpful any more (not sure how helpful it was
> before either), I'd remove it.
>
>> -    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 (i->preferred_sink != NULL || !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);
>>       return PA_HOOK_OK;
>>   }
>> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
>> index 566367f9e..63a3456e7 100644
>> --- a/src/pulsecore/sink.c
>> +++ b/src/pulsecore/sink.c
>> @@ -3886,3 +3886,36 @@ 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_from_oldsink_to_newsink(pa_sink *old_sink, pa_sink *new_sink, bool from_user) {
> I think "pa_sink_move_streams_to_default_sink" would be a better name
> for the function. It would also be good to have a comment explaining
> when the function is used and which streams are not moved (that could
> go to the header file).
>
>> +    pa_sink_input *i;
>> +    uint32_t idx;
>> +    pa_device_port *o_active_p;
>> +
>> +    if (old_sink == new_sink)
>> +	return;
>> +
>> +    if (old_sink == NULL)
>> +	return;
> It doesn't make sense to call this function with NULL, so this check
> should be an assertion instead.
>
>> +
>> +    o_active_p = old_sink->active_port;
> Note that not all sinks have ports, so this can be NULL.
>
> Since we only care about the availability of the active port, I'd add a
> "old_sink_is_unavailable" boolean to be used inside the loop.
>
>> +    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 (!from_user && i->preferred_sink != NULL && pa_safe_streq(old_sink->name, i->preferred_sink))
> No need to check if i->preferred_sink is NULL, because pa_safe_streq()
> checks that anyway.
>
>> +		    if (o_active_p->available != PA_AVAILABLE_NO)
>> +			continue;
>> +
>> +		if (pa_sink_input_move_to(i, new_sink, from_user) < 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)), new_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)), new_sink->name);
> Logging the reason for moving the stream before calling
> pa_sink_input_move_to() would be very useful.
>
> Here's a suggestion you don't need to implement, but you can if you
> want: it would be nice to have good enough logging in
> pa_sink_input_move_finish() so that callers don't all need to do their
> own logging about the move result. This should be done in a separate
> patch.
>
Tanu Kaskinen Dec. 22, 2018, 6:57 p.m.
On Sat, 2018-12-15 at 14:48 +0800, Hui Wang wrote:
> On 2018/12/13 下午5:16, Tanu Kaskinen wrote:
> > On Mon, 2018-11-05 at 09:47 +0800, Hui Wang wrote:
> > > When default sink changes, the stream should be moved to new
> > > default_sink too.
> > Except if the stream's preferred sink is the old default sink.
> > 
> > > If it is user to call change default function,
> > > all stream will move to new default sink unconditionally; if it
> > > is not, will check if stream binds to old_default_sink and the
> > > active_port staus of old_default_sink, then it will move the
> > > stream conditionally.
> > Why does it matter if the default sink changed due to user request or
> > some other reason? I don't think streams should be moved
> > unconditionally when the user changes the default sink.
> 
> Supposing  the sink0 is hdmi, sink1 is analog-speaker,  and a music is 
> playing on sink0 (default_sink is sink0),  if users select the 
> analog-speaker from UI (gnome-sound-setting), the sink1 is default_sink 
> now, and the music should be played on sink1.  If the streams don't move 
> to new default_sink unconditionally, I have no idea how to let music be 
> played on sink1 here.

I don't see what the problem is - streams should by default move to the
new default sink, but if the user has at some point manually moved a
stream to sink0, making sink0 the preferred sink, then that stream
shouldn't be moved automatically. If the user doesn't want to keep that
old setting any more, they can move the stream manually to sink1, and
since sink1 is the default sink at this point, the preferred_sink will
be unset, so that stream will from then on behave like all other
streams.
Hui Wang Dec. 26, 2018, 1:58 a.m.
On 2018/12/23 上午2:57, Tanu Kaskinen wrote:
> On Sat, 2018-12-15 at 14:48 +0800, Hui Wang wrote:
>> On 2018/12/13 下午5:16, Tanu Kaskinen wrote:
>>> On Mon, 2018-11-05 at 09:47 +0800, Hui Wang wrote:
>>>> When default sink changes, the stream should be moved to new
>>>> default_sink too.
>>> Except if the stream's preferred sink is the old default sink.
>>>
>>>> If it is user to call change default function,
>>>> all stream will move to new default sink unconditionally; if it
>>>> is not, will check if stream binds to old_default_sink and the
>>>> active_port staus of old_default_sink, then it will move the
>>>> stream conditionally.
>>> Why does it matter if the default sink changed due to user request or
>>> some other reason? I don't think streams should be moved
>>> unconditionally when the user changes the default sink.
>> Supposing  the sink0 is hdmi, sink1 is analog-speaker,  and a music is
>> playing on sink0 (default_sink is sink0),  if users select the
>> analog-speaker from UI (gnome-sound-setting), the sink1 is default_sink
>> now, and the music should be played on sink1.  If the streams don't move
>> to new default_sink unconditionally, I have no idea how to let music be
>> played on sink1 here.
> I don't see what the problem is - streams should by default move to the
> new default sink, but if the user has at some point manually moved a
> stream to sink0, making sink0 the preferred sink, then that stream
> shouldn't be moved automatically. If the user doesn't want to keep that
> old setting any more, they can move the stream manually to sink1, and
> since sink1 is the default sink at this point, the preferred_sink will
> be unset, so that stream will from then on behave like all other
> streams.
OK, got it.  thanks.