[pulseaudio-discuss] move streams when the default sink or source changes

Submitted by Tanu Kaskinen on Oct. 25, 2016, 1:02 p.m.

Details

Message ID 20161025130208.19853-1-tanuk@iki.fi
State New
Headers show
Series "move streams when the default sink or source changes" ( rev: 1 ) in PulseAudio

Not browsing as part of any series.

Commit Message

Tanu Kaskinen Oct. 25, 2016, 1:02 p.m.
This adds a new "use_default_sink" field to pa_sink_input. If
use_default_sink is set, the stream will be automatically moved when the
default sink changes. Usually if the user changes the default sink, the
user also wants to move existing streams there. If a stream is manually
routed, use_default_sink isn't set, so automatic moving doesn't happen
in that case.

pa_sink_input_move_to() and other move functions take use_default_sink
as a parameter so that every call site is forced to make a conscious
decision whether the stream should stick to the move target or not.

There are FIXME items added for some corner cases, because it's not
currently possible to properly remember and prioritize the various
automatic routing decisions that modules do. The different routing
modules tend to step on each other's toes, and this new
"use_default_sink" feature adds to the problem.

The same changes are made for source outputs too.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=93006
---

This patch depends on the previously submitted patches for improving
default sink/source handling:
https://lists.freedesktop.org/archives/pulseaudio-discuss/2016-September/026852.html

 src/modules/alsa/module-alsa-card.c    | 10 +++++++--
 src/modules/dbus/iface-stream.c        |  4 ++--
 src/modules/module-allow-passthrough.c | 20 ++++++++++++-----
 src/modules/module-device-manager.c    | 22 +++++++++++++++----
 src/modules/module-filter-apply.c      | 14 ++++++++++--
 src/modules/module-intended-roles.c    | 32 ++++++++++++++++++++++------
 src/modules/module-rescue-streams.c    | 32 ++++++++++++++++++++++++----
 src/modules/module-stream-restore.c    | 12 +++++------
 src/modules/module-switch-on-connect.c |  4 ++--
 src/pulsecore/cli-command.c            |  4 ++--
 src/pulsecore/core.c                   | 39 ++++++++++++++++++++++++++++++++++
 src/pulsecore/protocol-native.c        |  4 ++--
 src/pulsecore/sink-input.c             | 23 +++++++++++++++++---
 src/pulsecore/sink-input.h             | 18 ++++++++++++++--
 src/pulsecore/sink.c                   |  4 ++--
 src/pulsecore/sink.h                   | 12 ++++++++++-
 src/pulsecore/source-output.c          | 23 +++++++++++++++++---
 src/pulsecore/source-output.h          | 19 +++++++++++++++--
 src/pulsecore/source.c                 |  4 ++--
 src/pulsecore/source.h                 | 12 ++++++++++-
 20 files changed, 259 insertions(+), 53 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/modules/alsa/module-alsa-card.c b/src/modules/alsa/module-alsa-card.c
index 4f1236e..6272ce2 100644
--- a/src/modules/alsa/module-alsa-card.c
+++ b/src/modules/alsa/module-alsa-card.c
@@ -198,6 +198,7 @@  static int card_set_profile(pa_card *c, pa_card_profile *new_profile) {
     pa_alsa_mapping *am;
     pa_queue *sink_inputs = NULL, *source_outputs = NULL;
     int ret = 0;
+    bool use_default_device;
 
     pa_assert(c);
     pa_assert(new_profile);
@@ -245,6 +246,11 @@  static int card_set_profile(pa_card *c, pa_card_profile *new_profile) {
         }
     }
 
+    /* We don't have any particular reason to make the streams stick to the
+     * device we're moving them to, so let's allow the streams to be moved if
+     * the default sink or source changes later. */
+    use_default_device = true;
+
     if (nd->profile && nd->profile->output_mappings)
         PA_IDXSET_FOREACH(am, nd->profile->output_mappings, idx) {
 
@@ -252,7 +258,7 @@  static int card_set_profile(pa_card *c, pa_card_profile *new_profile) {
                 am->sink = pa_alsa_sink_new(c->module, u->modargs, __FILE__, c, am);
 
             if (sink_inputs && am->sink) {
-                pa_sink_move_all_finish(am->sink, sink_inputs, false);
+                pa_sink_move_all_finish(am->sink, sink_inputs, false, use_default_device);
                 sink_inputs = NULL;
             }
         }
@@ -264,7 +270,7 @@  static int card_set_profile(pa_card *c, pa_card_profile *new_profile) {
                 am->source = pa_alsa_source_new(c->module, u->modargs, __FILE__, c, am);
 
             if (source_outputs && am->source) {
-                pa_source_move_all_finish(am->source, source_outputs, false);
+                pa_source_move_all_finish(am->source, source_outputs, false, use_default_device);
                 source_outputs = NULL;
             }
         }
diff --git a/src/modules/dbus/iface-stream.c b/src/modules/dbus/iface-stream.c
index ade62ca..8aee23a 100644
--- a/src/modules/dbus/iface-stream.c
+++ b/src/modules/dbus/iface-stream.c
@@ -623,7 +623,7 @@  static void handle_move(DBusConnection *conn, DBusMessage *msg, void *userdata)
             return;
         }
 
-        if (pa_sink_input_move_to(s->sink_input, sink, true) < 0) {
+        if (pa_sink_input_move_to(s->sink_input, sink, true, false) < 0) {
             pa_dbus_send_error(conn, msg, DBUS_ERROR_FAILED,
                                "Moving playback stream %u to sink %s failed.", s->sink_input->index, sink->name);
             return;
@@ -636,7 +636,7 @@  static void handle_move(DBusConnection *conn, DBusMessage *msg, void *userdata)
             return;
         }
 
-        if (pa_source_output_move_to(s->source_output, source, true) < 0) {
+        if (pa_source_output_move_to(s->source_output, source, true, false) < 0) {
             pa_dbus_send_error(conn, msg, DBUS_ERROR_FAILED,
                                "Moving record stream %u to source %s failed.", s->source_output->index, source->name);
             return;
diff --git a/src/modules/module-allow-passthrough.c b/src/modules/module-allow-passthrough.c
index 4b801e4..085ae3a 100644
--- a/src/modules/module-allow-passthrough.c
+++ b/src/modules/module-allow-passthrough.c
@@ -99,9 +99,9 @@  static void unload_null_sink_module_for_sink(struct userdata *u, pa_sink *s, pa_
     pa_hashmap_remove(u->null_sinks, s);
 }
 
-static void move_stream(struct userdata *u, pa_sink_input *i, pa_sink *target) {
+static void move_stream(struct userdata *u, pa_sink_input *i, pa_sink *target, bool use_default_sink) {
     u->moving = true;
-    if (pa_sink_input_move_to(i, target, false) < 0)
+    if (pa_sink_input_move_to(i, target, false, use_default_sink) < 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)), target->name);
     else
@@ -147,7 +147,7 @@  static pa_hook_result_t new_passthrough_stream(struct userdata *u, pa_core *c, p
     PA_IDXSET_FOREACH(stream, sink->inputs, idx) {
         /* We don't want to move the stream which just moved to the sink and trigger this re-routing */
         if (stream != i)
-          move_stream(u, stream, null_sink);
+            move_stream(u, stream, null_sink, false);
     }
 
     return PA_HOOK_OK;
@@ -213,7 +213,17 @@  static pa_hook_result_t passthrough_stream_removed(struct userdata *u, pa_core *
     pa_log_info("Passthrough stream removed; restore all streams");
 
     PA_IDXSET_FOREACH(stream, null_sink->inputs, idx) {
-        move_stream(u, stream, i->sink);
+        bool use_default_sink;
+
+        /* FIXME: We set use_default_sink unconditionally here, so even if
+         * use_default_sink wasn't originally set for the stream, it will be
+         * set after we've moved it back to the original sink. This is
+         * consistent with how we deal with the "save" flag: even if the save
+         * flag was originally set, we unset it when we move the stream back to
+         * the original sink. */
+        use_default_sink = true;
+
+        move_stream(u, stream, i->sink, use_default_sink);
     }
 
     unload_null_sink_module_for_sink(u, i->sink, c);
@@ -254,7 +264,7 @@  static pa_hook_result_t sink_input_move_finish_cb(pa_core *core, pa_sink_input *
     null_sink = new_normal_stream(u, core, i->sink);
     if (null_sink) {
         pa_log_info("Already playing a passthrough stream; re-routing moved stream to the null sink");
-        move_stream(u, i, null_sink);
+        move_stream(u, i, null_sink, false);
     }
 
     return PA_HOOK_OK;
diff --git a/src/modules/module-device-manager.c b/src/modules/module-device-manager.c
index 2a0a67f..695409a 100644
--- a/src/modules/module-device-manager.c
+++ b/src/modules/module-device-manager.c
@@ -697,8 +697,15 @@  static void route_sink_input(struct userdata *u, pa_sink_input *si) {
             return;
     }
 
-    if (si->sink != sink)
-        pa_sink_input_move_to(si, sink, false);
+    if (si->sink != sink) {
+        bool use_default_sink;
+
+        /* When module-device-manager manages the routing, the default sink
+         * configuration should not have effect. */
+        use_default_sink = false;
+
+        pa_sink_input_move_to(si, sink, false, use_default_sink);
+    }
 }
 
 static pa_hook_result_t route_sink_inputs(struct userdata *u, pa_sink *ignore_sink) {
@@ -771,8 +778,15 @@  static void route_source_output(struct userdata *u, pa_source_output *so) {
             return;
     }
 
-    if (so->source != source)
-        pa_source_output_move_to(so, source, false);
+    if (so->source != source) {
+        bool use_default_source;
+
+        /* When module-device-manager manages the routing, the default source
+         * configuration should not have effect. */
+        use_default_source = false;
+
+        pa_source_output_move_to(so, source, false, use_default_source);
+    }
 }
 
 static pa_hook_result_t route_source_outputs(struct userdata *u, pa_source* ignore_source) {
diff --git a/src/modules/module-filter-apply.c b/src/modules/module-filter-apply.c
index 364d68b..a5b49fa 100644
--- a/src/modules/module-filter-apply.c
+++ b/src/modules/module-filter-apply.c
@@ -276,15 +276,25 @@  static void trigger_housekeeping(struct userdata *u) {
 }
 
 static int do_move(struct userdata *u, pa_object *obj, pa_object *parent, bool is_input) {
+    bool use_default_device;
+
     /* Keep track of objects that we've marked for module-device-manager to ignore */
     pa_hashmap_put(is_input ? u->mdm_ignored_inputs : u->mdm_ignored_outputs, obj, obj);
 
+    /* If the default sink or source changes, that shouldn't cause filtered
+     * streams to become unfiltered. FIXME: This can probably cause trouble
+     * too. If the stream is initially routed to the default device with a
+     * filter device in between, the stream probably should follow the default
+     * device selection, but rather than removing the filtering, the filtering
+     * should move to the new sink too. */
+    use_default_device = false;
+
     if (is_input) {
         pa_sink_input_set_property(PA_SINK_INPUT(obj), PA_PROP_MDM_AUTO_FILTERED, "1");
-        return pa_sink_input_move_to(PA_SINK_INPUT(obj), PA_SINK(parent), false);
+        return pa_sink_input_move_to(PA_SINK_INPUT(obj), PA_SINK(parent), false, use_default_device);
     } else {
         pa_source_output_set_property(PA_SOURCE_OUTPUT(obj), PA_PROP_MDM_AUTO_FILTERED, "1");
-        return pa_source_output_move_to(PA_SOURCE_OUTPUT(obj), PA_SOURCE(parent), false);
+        return pa_source_output_move_to(PA_SOURCE_OUTPUT(obj), PA_SOURCE(parent), false, use_default_device);
     }
 }
 
diff --git a/src/modules/module-intended-roles.c b/src/modules/module-intended-roles.c
index f906b88..bb19427 100644
--- a/src/modules/module-intended-roles.c
+++ b/src/modules/module-intended-roles.c
@@ -173,6 +173,7 @@  static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, struct
 
     PA_IDXSET_FOREACH(si, c->sink_inputs, idx) {
         const char *role;
+        bool use_default_sink;
 
         if (si->sink == sink)
             continue;
@@ -200,7 +201,11 @@  static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, struct
         if (!role_match(sink->proplist, role))
             continue;
 
-        pa_sink_input_move_to(si, sink, false);
+        /* We don't want the default sink configuration to affect streams that
+         * are routed by module-intended-roles. */
+        use_default_sink = false;
+
+        pa_sink_input_move_to(si, sink, false, use_default_sink);
     }
 
     return PA_HOOK_OK;
@@ -220,6 +225,7 @@  static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source,
 
     PA_IDXSET_FOREACH(so, c->source_outputs, idx) {
         const char *role;
+        bool use_default_source;
 
         if (so->source == source)
             continue;
@@ -250,7 +256,11 @@  static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source,
         if (!role_match(source->proplist, role))
             continue;
 
-        pa_source_output_move_to(so, source, false);
+        /* We don't want the default source configuration to affect streams
+         * that are routed by module-intended-roles. */
+        use_default_source = false;
+
+        pa_source_output_move_to(so, source, false, use_default_source);
     }
 
     return PA_HOOK_OK;
@@ -275,6 +285,7 @@  static pa_hook_result_t sink_unlink_hook_callback(pa_core *c, pa_sink *sink, str
 
     PA_IDXSET_FOREACH(si, sink->inputs, idx) {
         const char *role;
+        bool use_default_sink;
         uint32_t jdx;
         pa_sink *d;
 
@@ -284,9 +295,13 @@  static pa_hook_result_t sink_unlink_hook_callback(pa_core *c, pa_sink *sink, str
         if (!(role = pa_proplist_gets(si->proplist, PA_PROP_MEDIA_ROLE)))
             continue;
 
+        /* We don't want the default sink configuration to affect streams that
+         * are routed by module-intended-roles. */
+        use_default_sink = false;
+
         /* Would the default sink fit? If so, let's use it */
         if (c->default_sink != sink && role_match(c->default_sink->proplist, role))
-            if (pa_sink_input_move_to(si, c->default_sink, false) >= 0)
+            if (pa_sink_input_move_to(si, c->default_sink, false, use_default_sink) >= 0)
                 continue;
 
         /* Try to find some other fitting sink */
@@ -299,7 +314,7 @@  static pa_hook_result_t sink_unlink_hook_callback(pa_core *c, pa_sink *sink, str
                 continue;
 
             if (role_match(d->proplist, role))
-                if (pa_sink_input_move_to(si, d, false) >= 0)
+                if (pa_sink_input_move_to(si, d, false, use_default_sink) >= 0)
                     break;
         }
     }
@@ -326,6 +341,7 @@  static pa_hook_result_t source_unlink_hook_callback(pa_core *c, pa_source *sourc
 
     PA_IDXSET_FOREACH(so, source->outputs, idx) {
         const char *role;
+        bool use_default_source;
         uint32_t jdx;
         pa_source *d;
 
@@ -338,10 +354,14 @@  static pa_hook_result_t source_unlink_hook_callback(pa_core *c, pa_source *sourc
         if (!(role = pa_proplist_gets(so->proplist, PA_PROP_MEDIA_ROLE)))
             continue;
 
+        /* We don't want the default source configuration to affect streams
+         * that are routed by module-intended-roles. */
+        use_default_source = false;
+
         /* Would the default source fit? If so, let's use it */
         if (c->default_source != source && role_match(c->default_source->proplist, role)
                 && !source->monitor_of == !c->default_source->monitor_of) {
-            pa_source_output_move_to(so, c->default_source, false);
+            pa_source_output_move_to(so, c->default_source, false, use_default_source);
             continue;
         }
 
@@ -356,7 +376,7 @@  static pa_hook_result_t source_unlink_hook_callback(pa_core *c, pa_source *sourc
 
             /* If moving from a monitor, move to another monitor */
             if (!source->monitor_of == !d->monitor_of && role_match(d->proplist, role)) {
-                pa_source_output_move_to(so, d, false);
+                pa_source_output_move_to(so, d, false, use_default_source);
                 break;
             }
         }
diff --git a/src/modules/module-rescue-streams.c b/src/modules/module-rescue-streams.c
index d3c9953..3c1df99 100644
--- a/src/modules/module-rescue-streams.c
+++ b/src/modules/module-rescue-streams.c
@@ -161,11 +161,17 @@  static pa_hook_result_t sink_unlink_hook_callback(pa_core *c, pa_sink *sink, voi
 
     PA_IDXSET_FOREACH(i, sink->inputs, idx) {
         pa_sink *target;
+        bool use_default_sink;
 
         if (!(target = find_evacuation_sink(c, i, sink)))
             continue;
 
-        if (pa_sink_input_move_to(i, target, false) < 0)
+        /* We don't have any reason to want the stream to stick to the sink we
+         * choose here if the default sink changes later, so let's set
+         * use_default_sink to true. */
+        use_default_sink = true;
+
+        if (pa_sink_input_move_to(i, target, false, use_default_sink) < 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)), target->name);
         else
@@ -178,6 +184,7 @@  static pa_hook_result_t sink_unlink_hook_callback(pa_core *c, pa_sink *sink, voi
 
 static pa_hook_result_t sink_input_move_fail_hook_callback(pa_core *c, pa_sink_input *i, void *userdata) {
     pa_sink *target;
+    bool use_default_sink;
 
     pa_assert(c);
     pa_assert(i);
@@ -189,7 +196,12 @@  static pa_hook_result_t sink_input_move_fail_hook_callback(pa_core *c, pa_sink_i
     if (!(target = find_evacuation_sink(c, i, NULL)))
         return PA_HOOK_OK;
 
-    if (pa_sink_input_finish_move(i, target, false) < 0) {
+    /* We don't have any reason to want the stream to stick to the sink we
+     * choose here if the default sink changes later, so let's set
+     * use_default_sink to true. */
+    use_default_sink = true;
+
+    if (pa_sink_input_finish_move(i, target, false, use_default_sink) < 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)), target->name);
         return PA_HOOK_OK;
@@ -271,11 +283,17 @@  static pa_hook_result_t source_unlink_hook_callback(pa_core *c, pa_source *sourc
 
     PA_IDXSET_FOREACH(o, source->outputs, idx) {
         pa_source *target;
+        bool use_default_source;
 
         if (!(target = find_evacuation_source(c, o, source)))
             continue;
 
-        if (pa_source_output_move_to(o, target, false) < 0)
+        /* We don't have any reason to want the stream to stick to the source
+         * we choose here if the default source changes later, so let's set
+         * use_default_source to true. */
+        use_default_source = true;
+
+        if (pa_source_output_move_to(o, target, false, use_default_source) < 0)
             pa_log_info("Failed to move source output %u \"%s\" to %s.", o->index,
                         pa_strnull(pa_proplist_gets(o->proplist, PA_PROP_APPLICATION_NAME)), target->name);
         else
@@ -288,6 +306,7 @@  static pa_hook_result_t source_unlink_hook_callback(pa_core *c, pa_source *sourc
 
 static pa_hook_result_t source_output_move_fail_hook_callback(pa_core *c, pa_source_output *i, void *userdata) {
     pa_source *target;
+    bool use_default_source;
 
     pa_assert(c);
     pa_assert(i);
@@ -299,7 +318,12 @@  static pa_hook_result_t source_output_move_fail_hook_callback(pa_core *c, pa_sou
     if (!(target = find_evacuation_source(c, i, NULL)))
         return PA_HOOK_OK;
 
-    if (pa_source_output_finish_move(i, target, false) < 0) {
+    /* We don't have any reason to want the stream to stick to the source we
+     * choose here if the default source changes later, so let's set
+     * use_default_source to true. */
+    use_default_source = true;
+
+    if (pa_source_output_finish_move(i, target, false, use_default_source) < 0) {
         pa_log_info("Failed to move source output %u \"%s\" to %s.", i->index,
                         pa_strnull(pa_proplist_gets(i->proplist, PA_PROP_APPLICATION_NAME)), target->name);
         return PA_HOOK_OK;
diff --git a/src/modules/module-stream-restore.c b/src/modules/module-stream-restore.c
index d1c2597..cb1ce03 100644
--- a/src/modules/module-stream-restore.c
+++ b/src/modules/module-stream-restore.c
@@ -1633,7 +1633,7 @@  static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, struct
 
         if ((e = entry_read(u, name))) {
             if (e->device_valid && pa_streq(e->device, sink->name))
-                pa_sink_input_move_to(si, sink, true);
+                pa_sink_input_move_to(si, sink, true, false);
 
             entry_free(e);
         }
@@ -1681,7 +1681,7 @@  static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source,
 
         if ((e = entry_read(u, name))) {
             if (e->device_valid && pa_streq(e->device, source->name))
-                pa_source_output_move_to(so, source, true);
+                pa_source_output_move_to(so, source, true, false);
 
             entry_free(e);
         }
@@ -1723,7 +1723,7 @@  static pa_hook_result_t sink_unlink_hook_callback(pa_core *c, pa_sink *sink, str
                 if ((d = pa_namereg_get(c, e->device, PA_NAMEREG_SINK)) &&
                     d != sink &&
                     PA_SINK_IS_LINKED(pa_sink_get_state(d)))
-                    pa_sink_input_move_to(si, d, true);
+                    pa_sink_input_move_to(si, d, true, false);
             }
 
             entry_free(e);
@@ -1769,7 +1769,7 @@  static pa_hook_result_t source_unlink_hook_callback(pa_core *c, pa_source *sourc
                 if ((d = pa_namereg_get(c, e->device, PA_NAMEREG_SOURCE)) &&
                     d != source &&
                     PA_SOURCE_IS_LINKED(pa_source_get_state(d)))
-                    pa_source_output_move_to(so, d, true);
+                    pa_source_output_move_to(so, d, true, false);
             }
 
             entry_free(e);
@@ -1909,7 +1909,7 @@  static void entry_apply(struct userdata *u, const char *name, struct entry *e) {
                 }
             } else if ((s = pa_namereg_get(u->core, e->device, PA_NAMEREG_SINK))) {
                 pa_log_info("Restoring device for stream %s.", name);
-                pa_sink_input_move_to(si, s, true);
+                pa_sink_input_move_to(si, s, true, false);
             }
         }
     }
@@ -1957,7 +1957,7 @@  static void entry_apply(struct userdata *u, const char *name, struct entry *e) {
                 }
             } else if ((s = pa_namereg_get(u->core, e->device, PA_NAMEREG_SOURCE))) {
                 pa_log_info("Restoring device for stream %s.", name);
-                pa_source_output_move_to(so, s, true);
+                pa_source_output_move_to(so, s, true, false);
             }
         }
     }
diff --git a/src/modules/module-switch-on-connect.c b/src/modules/module-switch-on-connect.c
index 776c923..5aa08e8 100644
--- a/src/modules/module-switch-on-connect.c
+++ b/src/modules/module-switch-on-connect.c
@@ -97,7 +97,7 @@  static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void*
         if (i->save_sink || !PA_SINK_INPUT_IS_LINKED(i->state))
             continue;
 
-        if (pa_sink_input_move_to(i, sink, false) < 0)
+        if (pa_sink_input_move_to(i, sink, false, true) < 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
@@ -157,7 +157,7 @@  static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source,
         if (o->save_source || !PA_SOURCE_OUTPUT_IS_LINKED(o->state))
             continue;
 
-        if (pa_source_output_move_to(o, source, false) < 0)
+        if (pa_source_output_move_to(o, source, false, true) < 0)
             pa_log_info("Failed to move source output %u \"%s\" to %s.", o->index,
                         pa_strnull(pa_proplist_gets(o->proplist, PA_PROP_APPLICATION_NAME)), source->name);
         else
diff --git a/src/pulsecore/cli-command.c b/src/pulsecore/cli-command.c
index 5a632be..e8c93eb 100644
--- a/src/pulsecore/cli-command.c
+++ b/src/pulsecore/cli-command.c
@@ -1336,7 +1336,7 @@  static int pa_cli_command_move_sink_input(pa_core *c, pa_tokenizer *t, pa_strbuf
         return -1;
     }
 
-    if (pa_sink_input_move_to(si, sink, true) < 0) {
+    if (pa_sink_input_move_to(si, sink, true, false) < 0) {
         pa_strbuf_puts(buf, "Moved failed.\n");
         return -1;
     }
@@ -1379,7 +1379,7 @@  static int pa_cli_command_move_source_output(pa_core *c, pa_tokenizer *t, pa_str
         return -1;
     }
 
-    if (pa_source_output_move_to(so, source, true) < 0) {
+    if (pa_source_output_move_to(so, source, true, false) < 0) {
         pa_strbuf_puts(buf, "Moved failed.\n");
         return -1;
     }
diff --git a/src/pulsecore/core.c b/src/pulsecore/core.c
index 93a4201..927227f 100644
--- a/src/pulsecore/core.c
+++ b/src/pulsecore/core.c
@@ -314,6 +314,25 @@  void pa_core_update_default_sink(pa_core *core) {
     pa_log_info("default_sink: %s -> %s",
                 old_default_sink ? old_default_sink->name : "(unset)", best ? best->name : "(unset)");
 
+    if (core->default_sink) {
+        pa_sink_input *input;
+
+        /* Move streams to the new default sink. */
+        PA_IDXSET_FOREACH(input, core->sink_inputs, idx) {
+            if (!input->use_default_sink)
+                continue;
+
+            if (!input->sink)
+                continue;
+
+            if (input->sink == core->default_sink)
+                continue;
+
+            pa_log_debug("Moving input %u to the new default sink.", input->index);
+            pa_sink_input_move_to(input, core->default_sink, false, true);
+        }
+    }
+
     /* If the default sink changed, it may be that the default source has to be
      * changed too, because monitor sources are prioritized partly based on the
      * priorities of the monitored sinks. */
@@ -390,6 +409,26 @@  void pa_core_update_default_source(pa_core *core) {
     core->default_source = best;
     pa_log_info("default_source: %s -> %s",
                 old_default_source ? old_default_source->name : "(unset)", best ? best->name : "(unset)");
+
+    if (core->default_source) {
+        pa_source_output *output;
+
+        /* Move streams to the new default source. */
+        PA_IDXSET_FOREACH(output, core->source_outputs, idx) {
+            if (!output->use_default_source)
+                continue;
+
+            if (!output->source)
+                continue;
+
+            if (output->source == core->default_source)
+                continue;
+
+            pa_log_debug("Moving output %u to the new default source.", output->index);
+            pa_source_output_move_to(output, core->default_source, false, true);
+        }
+    }
+
     pa_subscription_post(core, PA_SUBSCRIPTION_EVENT_SERVER | PA_SUBSCRIPTION_EVENT_CHANGE, PA_INVALID_INDEX);
     pa_hook_fire(&core->hooks[PA_CORE_HOOK_DEFAULT_SOURCE_CHANGED], core->default_source);
 }
diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
index 272156b..b912dd5 100644
--- a/src/pulsecore/protocol-native.c
+++ b/src/pulsecore/protocol-native.c
@@ -4529,7 +4529,7 @@  static void command_move_stream(pa_pdispatch *pd, uint32_t command, uint32_t tag
 
         CHECK_VALIDITY(c->pstream, si && sink, tag, PA_ERR_NOENTITY);
 
-        if (pa_sink_input_move_to(si, sink, true) < 0) {
+        if (pa_sink_input_move_to(si, sink, true, false) < 0) {
             pa_pstream_send_error(c->pstream, tag, PA_ERR_INVALID);
             return;
         }
@@ -4548,7 +4548,7 @@  static void command_move_stream(pa_pdispatch *pd, uint32_t command, uint32_t tag
 
         CHECK_VALIDITY(c->pstream, so && source, tag, PA_ERR_NOENTITY);
 
-        if (pa_source_output_move_to(so, source, true) < 0) {
+        if (pa_source_output_move_to(so, source, true, false) < 0) {
             pa_pstream_send_error(c->pstream, tag, PA_ERR_INVALID);
             return;
         }
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 0dda204..732b68e 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -288,6 +288,7 @@  int pa_sink_input_new(
     char st[PA_SAMPLE_SPEC_SNPRINT_MAX], cm[PA_CHANNEL_MAP_SNPRINT_MAX], fmt[PA_FORMAT_INFO_SNPRINT_MAX];
     pa_channel_map volume_map;
     int r;
+    bool use_default_sink;
     char *pt;
     char *memblockq_name;
 
@@ -326,6 +327,8 @@  int pa_sink_input_new(
 
     pa_return_val_if_fail(!data->driver || pa_utf8_valid(data->driver), -PA_ERR_INVALID);
 
+    use_default_sink = !data->sink;
+
     if (!data->sink) {
         pa_sink *sink = pa_namereg_get(core, NULL, PA_NAMEREG_SINK);
         pa_return_val_if_fail(sink, -PA_ERR_NOENTITY);
@@ -473,6 +476,7 @@  int pa_sink_input_new(
     i->driver = pa_xstrdup(pa_path_get_filename(data->driver));
     i->module = data->module;
     i->sink = data->sink;
+    i->use_default_sink = use_default_sink;
     i->origin_sink = data->origin_sink;
     i->client = data->client;
 
@@ -1828,8 +1832,19 @@  static void update_volume_due_to_moving(pa_sink_input *i, pa_sink *dest) {
         pa_sink_set_volume(i->sink, NULL, false, i->save_volume);
 }
 
+/* Called from the main thread. */
+static void set_use_default_sink(pa_sink_input *input, bool use_default_sink) {
+    if (use_default_sink == input->use_default_sink)
+        return;
+
+    input->use_default_sink = use_default_sink;
+
+    pa_log_info("Sink input %u: use_default_sink: %s -> %s",
+                input->index, use_default_sink ? "false" : "true", use_default_sink ? "true" : "false");
+}
+
 /* Called from main context */
-int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {
+int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save, bool use_default_sink) {
     struct volume_factor_entry *v;
     void *state = NULL;
 
@@ -1838,6 +1853,7 @@  int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {
     pa_assert(PA_SINK_INPUT_IS_LINKED(i->state));
     pa_assert(!i->sink);
     pa_sink_assert_ref(dest);
+    pa_assert(!save || !use_default_sink);
 
     if (!pa_sink_input_may_move_to(i, dest))
         return -PA_ERR_NOTSUPPORTED;
@@ -1869,6 +1885,7 @@  int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {
 
     i->sink = dest;
     i->save_sink = save;
+    set_use_default_sink(i, use_default_sink);
     pa_idxset_put(dest->inputs, pa_sink_input_ref(i), NULL);
 
     PA_HASHMAP_FOREACH(v, i->volume_factor_sink_items, state)
@@ -1918,7 +1935,7 @@  void pa_sink_input_fail_move(pa_sink_input *i) {
 }
 
 /* Called from main context */
-int pa_sink_input_move_to(pa_sink_input *i, pa_sink *dest, bool save) {
+int pa_sink_input_move_to(pa_sink_input *i, pa_sink *dest, bool save, bool use_default_sink) {
     int r;
 
     pa_sink_input_assert_ref(i);
@@ -1940,7 +1957,7 @@  int pa_sink_input_move_to(pa_sink_input *i, pa_sink *dest, bool save) {
         return r;
     }
 
-    if ((r = pa_sink_input_finish_move(i, dest, save)) < 0) {
+    if ((r = pa_sink_input_finish_move(i, dest, save, use_default_sink)) < 0) {
         pa_sink_input_fail_move(i);
         pa_sink_input_unref(i);
         return r;
diff --git a/src/pulsecore/sink-input.h b/src/pulsecore/sink-input.h
index 8bbee4e..2ba1779 100644
--- a/src/pulsecore/sink-input.h
+++ b/src/pulsecore/sink-input.h
@@ -121,6 +121,11 @@  struct pa_sink_input {
      * this.*/
     bool save_sink:1, save_volume:1, save_muted:1;
 
+    /* If true, then this sink input is automatically moved to the new default
+     * sink when the default sink changes. If save_sink is true, this is always
+     * false, but if save_sink is false, this can be either true or false. */
+    bool use_default_sink;
+
     pa_resample_method_t requested_resample_method, actual_resample_method;
 
     /* Returns the chunk of audio data and drops it from the
@@ -379,7 +384,16 @@  pa_resample_method_t pa_sink_input_get_resample_method(pa_sink_input *i);
 
 void pa_sink_input_send_event(pa_sink_input *i, const char *name, pa_proplist *data);
 
-int pa_sink_input_move_to(pa_sink_input *i, pa_sink *dest, bool save);
+/* Move i to dest.
+ *
+ * save: Should the new sink be saved by module-stream-restore?
+ *
+ * use_default_sink: If the default sink changes later, should the stream that
+ * is now being moved move automatically to the new default sink, or should it
+ * stick to the current sink? If "save" is true, "use_default_sink" has to be
+ * false. */
+int pa_sink_input_move_to(pa_sink_input *i, pa_sink *dest, bool save, bool use_default_sink);
+
 bool pa_sink_input_may_move(pa_sink_input *i); /* may this sink input move at all? */
 bool pa_sink_input_may_move_to(pa_sink_input *i, pa_sink *dest); /* may this sink input move to this sink? */
 
@@ -387,7 +401,7 @@  bool pa_sink_input_may_move_to(pa_sink_input *i, pa_sink *dest); /* may this sin
  * first the detaching from the old sink, then the attaching to the
  * new sink */
 int pa_sink_input_start_move(pa_sink_input *i);
-int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save);
+int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save, bool use_default_sink);
 void pa_sink_input_fail_move(pa_sink_input *i);
 
 pa_sink_input_state_t pa_sink_input_get_state(pa_sink_input *i);
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index aba11aa..8a00419 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -911,7 +911,7 @@  pa_queue *pa_sink_move_all_start(pa_sink *s, pa_queue *q) {
 }
 
 /* Called from main context */
-void pa_sink_move_all_finish(pa_sink *s, pa_queue *q, bool save) {
+void pa_sink_move_all_finish(pa_sink *s, pa_queue *q, bool save, bool use_default_sink) {
     pa_sink_input *i;
 
     pa_sink_assert_ref(s);
@@ -920,7 +920,7 @@  void pa_sink_move_all_finish(pa_sink *s, pa_queue *q, bool save) {
     pa_assert(q);
 
     while ((i = PA_SINK_INPUT(pa_queue_pop(q)))) {
-        if (pa_sink_input_finish_move(i, s, save) < 0)
+        if (pa_sink_input_finish_move(i, s, save, use_default_sink) < 0)
             pa_sink_input_fail_move(i);
 
         pa_sink_input_unref(i);
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index c549869..a7f4a72 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -471,7 +471,17 @@  unsigned pa_sink_check_suspend(pa_sink *s); /* Returns how many streams are acti
 
 /* Moves all inputs away, and stores them in pa_queue */
 pa_queue *pa_sink_move_all_start(pa_sink *s, pa_queue *q);
-void pa_sink_move_all_finish(pa_sink *s, pa_queue *q, bool save);
+
+/* Move the queued streams to s.
+ *
+ * save: Should the new sink be saved by module-stream-restore?
+ *
+ * use_default_sink: If the default sink changes later, should the streams that
+ * are now being moved move automatically to the new default sink, or should
+ * they stick to the current sink? If "save" is true, "use_default_sink" has to
+ * be false. */
+void pa_sink_move_all_finish(pa_sink *s, pa_queue *q, bool save, bool use_default_sink);
+
 void pa_sink_move_all_fail(pa_queue *q);
 
 /* Returns a copy of the sink formats. TODO: Get rid of this function (or at
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 35ef1c5..1ea3fea 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -223,6 +223,7 @@  int pa_source_output_new(
     char st[PA_SAMPLE_SPEC_SNPRINT_MAX], cm[PA_CHANNEL_MAP_SNPRINT_MAX], fmt[PA_FORMAT_INFO_SNPRINT_MAX];
     pa_channel_map volume_map;
     int r;
+    bool use_default_source;
     char *pt;
 
     pa_assert(_o);
@@ -260,6 +261,8 @@  int pa_source_output_new(
 
     pa_return_val_if_fail(!data->driver || pa_utf8_valid(data->driver), -PA_ERR_INVALID);
 
+    use_default_source = !data->source;
+
     if (!data->source) {
         pa_source *source;
 
@@ -418,6 +421,7 @@  int pa_source_output_new(
     o->driver = pa_xstrdup(pa_path_get_filename(data->driver));
     o->module = data->module;
     o->source = data->source;
+    o->use_default_source = use_default_source;
     o->destination_source = data->destination_source;
     o->client = data->client;
 
@@ -1466,13 +1470,25 @@  static void update_volume_due_to_moving(pa_source_output *o, pa_source *dest) {
         pa_source_set_volume(o->source, NULL, false, o->save_volume);
 }
 
+/* Called from the main thread. */
+static void set_use_default_source(pa_source_output *output, bool use_default_source) {
+    if (use_default_source == output->use_default_source)
+        return;
+
+    output->use_default_source = use_default_source;
+
+    pa_log_info("Source output %u: use_default_source: %s -> %s",
+                output->index, use_default_source ? "false" : "true", use_default_source ? "true" : "false");
+}
+
 /* Called from main context */
-int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save) {
+int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save, bool use_default_source) {
     pa_source_output_assert_ref(o);
     pa_assert_ctl_context();
     pa_assert(PA_SOURCE_OUTPUT_IS_LINKED(o->state));
     pa_assert(!o->source);
     pa_source_assert_ref(dest);
+    pa_assert(!save || !use_default_source);
 
     if (!pa_source_output_may_move_to(o, dest))
         return -PA_ERR_NOTSUPPORTED;
@@ -1504,6 +1520,7 @@  int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save
 
     o->source = dest;
     o->save_source = save;
+    set_use_default_source(o, use_default_source);
     pa_idxset_put(o->source->outputs, pa_source_output_ref(o), NULL);
 
     pa_cvolume_remap(&o->volume_factor_source, &o->channel_map, &o->source->channel_map);
@@ -1550,7 +1567,7 @@  void pa_source_output_fail_move(pa_source_output *o) {
 }
 
 /* Called from main context */
-int pa_source_output_move_to(pa_source_output *o, pa_source *dest, bool save) {
+int pa_source_output_move_to(pa_source_output *o, pa_source *dest, bool save, bool use_default_source) {
     int r;
 
     pa_source_output_assert_ref(o);
@@ -1572,7 +1589,7 @@  int pa_source_output_move_to(pa_source_output *o, pa_source *dest, bool save) {
         return r;
     }
 
-    if ((r = pa_source_output_finish_move(o, dest, save)) < 0) {
+    if ((r = pa_source_output_finish_move(o, dest, save, use_default_source)) < 0) {
         pa_source_output_fail_move(o);
         pa_source_output_unref(o);
         return r;
diff --git a/src/pulsecore/source-output.h b/src/pulsecore/source-output.h
index 7ac44bd..55cdf55 100644
--- a/src/pulsecore/source-output.h
+++ b/src/pulsecore/source-output.h
@@ -103,6 +103,12 @@  struct pa_source_output {
      * this.*/
     bool save_source:1, save_volume:1, save_muted:1;
 
+    /* If true, then this source output is automatically moved to the new
+     * default source when the default source changes. If save_source is true,
+     * this is always false, but if save_source is false, this can be either
+     * true or false. */
+    bool use_default_source;
+
     pa_resample_method_t requested_resample_method, actual_resample_method;
 
     /* Pushes a new memchunk into the output. Called from IO thread
@@ -322,15 +328,24 @@  pa_resample_method_t pa_source_output_get_resample_method(pa_source_output *o);
 
 void pa_source_output_send_event(pa_source_output *o, const char *name, pa_proplist *data);
 
+/* Move o to dest.
+ *
+ * save: Should the new source be saved by module-stream-restore?
+ *
+ * use_default_source: If the default source changes later, should the stream
+ * that is now being moved move automatically to the new default source, or
+ * should it stick to the current source? If "save" is true,
+ * "use_default_source" has to be false. */
+int pa_source_output_move_to(pa_source_output *o, pa_source *dest, bool save, bool use_default_source);
+
 bool pa_source_output_may_move(pa_source_output *o);
 bool pa_source_output_may_move_to(pa_source_output *o, pa_source *dest);
-int pa_source_output_move_to(pa_source_output *o, pa_source *dest, bool save);
 
 /* The same as pa_source_output_move_to() but in two separate steps,
  * first the detaching from the old source, then the attaching to the
  * new source */
 int pa_source_output_start_move(pa_source_output *o);
-int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save);
+int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save, bool use_default_source);
 void pa_source_output_fail_move(pa_source_output *o);
 
 #define pa_source_output_get_state(o) ((o)->state)
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index 5dae761..9539f57 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -851,7 +851,7 @@  pa_queue *pa_source_move_all_start(pa_source *s, pa_queue *q) {
 }
 
 /* Called from main context */
-void pa_source_move_all_finish(pa_source *s, pa_queue *q, bool save) {
+void pa_source_move_all_finish(pa_source *s, pa_queue *q, bool save, bool use_default_source) {
     pa_source_output *o;
 
     pa_source_assert_ref(s);
@@ -860,7 +860,7 @@  void pa_source_move_all_finish(pa_source *s, pa_queue *q, bool save) {
     pa_assert(q);
 
     while ((o = PA_SOURCE_OUTPUT(pa_queue_pop(q)))) {
-        if (pa_source_output_finish_move(o, s, save) < 0)
+        if (pa_source_output_finish_move(o, s, save, use_default_source) < 0)
             pa_source_output_fail_move(o);
 
         pa_source_output_unref(o);
diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h
index 3a6b5c3..8a408be 100644
--- a/src/pulsecore/source.h
+++ b/src/pulsecore/source.h
@@ -405,7 +405,17 @@  unsigned pa_source_check_suspend(pa_source *s); /* Returns how many streams are
 
 /* Moves all inputs away, and stores them in pa_queue */
 pa_queue *pa_source_move_all_start(pa_source *s, pa_queue *q);
-void pa_source_move_all_finish(pa_source *s, pa_queue *q, bool save);
+
+/* Move the queued streams to s.
+ *
+ * save: Should the new source be saved by module-stream-restore?
+ *
+ * use_default_source: If the default source changes later, should the streams
+ * that are now being moved move automatically to the new default source, or
+ * should they stick to the current source? If "save" is true,
+ * "use_default_source" has to be false. */
+void pa_source_move_all_finish(pa_source *s, pa_queue *q, bool save, bool use_default_source);
+
 void pa_source_move_all_fail(pa_queue *q);
 
 /* Returns a copy of the source formats. TODO: Get rid of this function (or at

Comments

On 25.10.2016 15:02, Tanu Kaskinen wrote:
> This adds a new "use_default_sink" field to pa_sink_input. If
> use_default_sink is set, the stream will be automatically moved when the
> default sink changes. Usually if the user changes the default sink, the
> user also wants to move existing streams there. If a stream is manually
> routed, use_default_sink isn't set, so automatic moving doesn't happen
> in that case.
>
> pa_sink_input_move_to() and other move functions take use_default_sink
> as a parameter so that every call site is forced to make a conscious
> decision whether the stream should stick to the move target or not.
>
> There are FIXME items added for some corner cases, because it's not
> currently possible to properly remember and prioritize the various
> automatic routing decisions that modules do. The different routing
> modules tend to step on each other's toes, and this new
> "use_default_sink" feature adds to the problem.
>
> The same changes are made for source outputs too.
>
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=93006
> ---
>
I don't understand why you need that flag at all. Why not simply
take all streams that are on the old default sink and move them
to the new default sink?
It's really weired that streams that are currently not on the default
sink should jump to the new default sink if the default changes.
 From a user perspective I would not expect that (and personally
would not want to have it that way).

Regards
              Georg
On Thu, 2017-04-06 at 11:52 +0200, Georg Chini wrote:
> On 25.10.2016 15:02, Tanu Kaskinen wrote:
> > This adds a new "use_default_sink" field to pa_sink_input. If
> > use_default_sink is set, the stream will be automatically moved when the
> > default sink changes. Usually if the user changes the default sink, the
> > user also wants to move existing streams there. If a stream is manually
> > routed, use_default_sink isn't set, so automatic moving doesn't happen
> > in that case.
> > 
> > pa_sink_input_move_to() and other move functions take use_default_sink
> > as a parameter so that every call site is forced to make a conscious
> > decision whether the stream should stick to the move target or not.
> > 
> > There are FIXME items added for some corner cases, because it's not
> > currently possible to properly remember and prioritize the various
> > automatic routing decisions that modules do. The different routing
> > modules tend to step on each other's toes, and this new
> > "use_default_sink" feature adds to the problem.
> > 
> > The same changes are made for source outputs too.
> > 
> > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=93006
> > ---
> > 
> 
> I don't understand why you need that flag at all. Why not simply
> take all streams that are on the old default sink and move them
> to the new default sink?

If you manually move a stream to a different sink (let's call that sink
S), that stream isn't any more routed to the default sink. If S at some
point becomes the default sink, do you really want to forget the manual
routing choice? With your suggestion the manually-moved stream would
move if some other sink becomes the default later.

> It's really weired that streams that are currently not on the default
> sink should jump to the new default sink if the default changes.
>  From a user perspective I would not expect that (and personally
> would not want to have it that way).

Can you give a concrete example where the code should be changed?
Looking at the patch, I can identify these cases that enable "use
default device" for streams that previously might not have been routed
to the default device:

When changing profiles, module-alsa-card moves streams from the old
devices to the new devices. "Use default device" is enabled for those
streams. As a comment says, "we don't have any particular reason to
make the streams stick to the device we're moving them to". I
acknowledge that the chosen approach isn't ideal, but I think it's
better than disabling "use default device". The real fix would involve
improving the routing system so that module-alsa-card wouldn't have to
care about moving the streams at all.

module-allow-passthrough enables "use default device" when restoring
the old routing. This isn't ideal, and there's a FIXME comment. Again,
just disabling "use default device" wouldn't be an improvement in this
case, the routing system needs some bigger changes to deal with this
properly.

module-rescue-streams enables "use default device" for rescued streams.
It should ideally pick the default device anyway as the "evacuation"
target, but there's the problem that if the removed sink is the current
default, it's not yet known at the time of stream evacuation which sink
will become the new default, and the current code for selecting the
evacuation sink probably doesn't always pick the "right" sink. Fixing
this problem shouldn't be very difficult. Would you prefer me to fix
module-rescue-streams before applying this patch (assuming that you
agree with the general idea of this patch at all)?
On 06.04.2017 22:34, Tanu Kaskinen wrote:
> On Thu, 2017-04-06 at 11:52 +0200, Georg Chini wrote:
>> On 25.10.2016 15:02, Tanu Kaskinen wrote:
>>> This adds a new "use_default_sink" field to pa_sink_input. If
>>> use_default_sink is set, the stream will be automatically moved when the
>>> default sink changes. Usually if the user changes the default sink, the
>>> user also wants to move existing streams there. If a stream is manually
>>> routed, use_default_sink isn't set, so automatic moving doesn't happen
>>> in that case.
>>>
>>> pa_sink_input_move_to() and other move functions take use_default_sink
>>> as a parameter so that every call site is forced to make a conscious
>>> decision whether the stream should stick to the move target or not.
>>>
>>> There are FIXME items added for some corner cases, because it's not
>>> currently possible to properly remember and prioritize the various
>>> automatic routing decisions that modules do. The different routing
>>> modules tend to step on each other's toes, and this new
>>> "use_default_sink" feature adds to the problem.
>>>
>>> The same changes are made for source outputs too.
>>>
>>> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=93006
>>> ---
>>>
>> I don't understand why you need that flag at all. Why not simply
>> take all streams that are on the old default sink and move them
>> to the new default sink?
> If you manually move a stream to a different sink (let's call that sink
> S), that stream isn't any more routed to the default sink. If S at some
> point becomes the default sink, do you really want to forget the manual
> routing choice? With your suggestion the manually-moved stream would
> move if some other sink becomes the default later.

Yes, I would think it is OK to "forget" the manual routing in that case.
Also consider the case where you move a stream manually to the
default sink. If the default sink changes later, this stream would not
be moved to the new default.

>
>> It's really weired that streams that are currently not on the default
>> sink should jump to the new default sink if the default changes.
>>   From a user perspective I would not expect that (and personally
>> would not want to have it that way).
> Can you give a concrete example where the code should be changed?
> Looking at the patch, I can identify these cases that enable "use
> default device" for streams that previously might not have been routed
> to the default device:
>
> When changing profiles, module-alsa-card moves streams from the old
> devices to the new devices. "Use default device" is enabled for those
> streams. As a comment says, "we don't have any particular reason to
> make the streams stick to the device we're moving them to". I
> acknowledge that the chosen approach isn't ideal, but I think it's
> better than disabling "use default device". The real fix would involve
> improving the routing system so that module-alsa-card wouldn't have to
> care about moving the streams at all.

Let's assume that a profile change causes a stream to be moved to
the new sink S which is not your default. Apart from S you have your
default sink D. Now if D changes, why should that have any impact on
a stream playing on sink S? What is actually the advantage of setting
use_default_sink in the case of a profile switch?

>
> module-allow-passthrough enables "use default device" when restoring
> the old routing. This isn't ideal, and there's a FIXME comment. Again,
> just disabling "use default device" wouldn't be an improvement in this
> case, the routing system needs some bigger changes to deal with this
> properly.

You say not setting the flag would be no improvement. What is the
improvement when you set the flag? I have to say that I cannot really
see it.

>
> module-rescue-streams enables "use default device" for rescued streams.
> It should ideally pick the default device anyway as the "evacuation"
> target, but there's the problem that if the removed sink is the current
> default, it's not yet known at the time of stream evacuation which sink
> will become the new default, and the current code for selecting the
> evacuation sink probably doesn't always pick the "right" sink. Fixing
> this problem shouldn't be very difficult. Would you prefer me to fix
> module-rescue-streams before applying this patch (assuming that you
> agree with the general idea of this patch at all)?
>
Module-rescue-stream just need not do anything if the default sink
goes away. Rescuing the streams could then be done when the new
default sink is elected. All streams that still have the old sink (or no 
sink
at all) can then be moved to the new default.