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.
