[pulseaudio-commits] [Git][pulseaudio/pulseaudio][master] 2 commits: sink-input, source-output: Add hooks for preferred device changes

PulseAudio Marge Bot gitlab at gitlab.freedesktop.org
Mon Apr 5 15:20:10 UTC 2021



PulseAudio Marge Bot pushed to branch master at PulseAudio / pulseaudio


Commits:
737ebcdf by Tanu Kaskinen at 2021-04-05T15:17:15+00:00
sink-input, source-output: Add hooks for preferred device changes

The hooks are fired when the preferred device changes. This is useful
for module-stream-restore.

I added new set_preferred_sink/source() functions for firing the hooks.
The functions also log the preferred device changes.

There was already pa_sink_input_set_preferred_sink(), but that had a
side effect of moving the stream, so I needed a new function. Since it
can be confusing when the two similarly named functions should be
called, I added a comment for pa_sink_input_set_preferred_sink() that
explains the different situations.

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/535>

- - - - -
8a87af38 by Tanu Kaskinen at 2021-04-05T15:17:15+00:00
stream-restore: Fix NULL preferred device handling

When an application sets a device for a newly created stream, we treat
that as a temporary setting, and don't save it as the preferred device
for future streams. The handling for this was broken, however: if the
stream already had a preferred device saved in the stream-restore
database, that was unset.

This was a regression introduced in
bc0e72832057c9d2744d95767dff2a48c83082c2 and
70bbbcdc8440a6a616467a24496f497b225a2cee. These commits tried to detect
in subscribe_callback() when the preferred device is cleared, but as a
side effect the preferred device started to get cleared from the
database also when a stream was created with a device set by the
application.

There's no way for subscribe_callback() to distinguish the different
cases of the preferred device being NULL. This problem is solved by
using the PREFERRED_SINK/SOURCE_CHANGED hooks. The hooks are only called
when the preferred device really changes.

Fixes: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/issues/1063
Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/535>

- - - - -


4 changed files:

- src/modules/module-stream-restore.c
- src/pulsecore/core.h
- src/pulsecore/sink-input.c
- src/pulsecore/source-output.c


Changes:

=====================================
src/modules/module-stream-restore.c
=====================================
@@ -1294,7 +1294,6 @@ static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint3
     /* These are only used when D-Bus is enabled, but in order to reduce ifdef
      * clutter these are defined here unconditionally. */
     bool created_new_entry = true;
-    bool device_updated = false;
     bool volume_updated = false;
     bool mute_updated = false;
 
@@ -1350,25 +1349,6 @@ static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint3
 
             mute_updated = !created_new_entry && (!old->muted_valid || entry->muted != old->muted);
         }
-
-        if (sink_input->preferred_sink != NULL || !created_new_entry) {
-            pa_sink *s = NULL;
-
-            pa_xfree(entry->device);
-            entry->device = pa_xstrdup(sink_input->preferred_sink);
-            entry->device_valid = true;
-            if (!entry->device)
-                entry->device_valid = false;
-
-            device_updated = !created_new_entry && !pa_safe_streq(entry->device, old->device);
-            pa_xfree(entry->card);
-            entry->card = NULL;
-            entry->card_valid = false;
-            if (entry->device_valid && (s = pa_namereg_get(c, entry->device, PA_NAMEREG_SINK)) && s->card) {
-                entry->card = pa_xstrdup(s->card->name);
-                entry->card_valid = true;
-            }
-        }
     } else {
         pa_source_output *source_output;
 
@@ -1410,26 +1390,6 @@ static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint3
 
             mute_updated = !created_new_entry && (!old->muted_valid || entry->muted != old->muted);
         }
-
-        if (source_output->preferred_source != NULL || !created_new_entry) {
-            pa_source *s = NULL;
-
-            pa_xfree(entry->device);
-            entry->device = pa_xstrdup(source_output->preferred_source);
-            entry->device_valid = true;
-
-            if (!entry->device)
-                entry->device_valid = false;
-
-            device_updated = !created_new_entry && !pa_safe_streq(entry->device, old->device);
-            pa_xfree(entry->card);
-            entry->card = NULL;
-            entry->card_valid = false;
-            if (entry->device_valid && (s = pa_namereg_get(c, entry->device, PA_NAMEREG_SOURCE)) && s->card) {
-                entry->card = pa_xstrdup(s->card->name);
-                entry->card_valid = true;
-            }
-        }
     }
 
     pa_assert(entry);
@@ -1446,12 +1406,12 @@ static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint3
         entry_free(old);
     }
 
-    pa_log_info("Storing volume/mute/device for stream %s.", name);
+    pa_log_info("Storing volume/mute for stream %s.", name);
 
     if (entry_write(u, name, entry, true)) {
         trigger_save(u);
     } else {
-        pa_log_error("Could not store volume/mute/device for stream %s.", name);
+        pa_log_error("Could not store volume/mute for stream %s.", name);
     }
 
 #ifdef HAVE_DBUS
@@ -1460,8 +1420,6 @@ static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint3
         pa_assert_se(pa_hashmap_put(u->dbus_entries, de->entry_name, de) == 0);
         send_new_entry_signal(de);
     } else {
-        if (device_updated)
-            send_device_updated_signal(de, entry);
         if (volume_updated)
             send_volume_updated_signal(de, entry);
         if (mute_updated)
@@ -1469,7 +1427,6 @@ static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint3
     }
 #else
     /* Silence compiler warnings */
-    (void) device_updated;
     (void) volume_updated;
     (void) mute_updated;
 #endif
@@ -1582,6 +1539,79 @@ static pa_hook_result_t sink_input_fixate_hook_callback(pa_core *c, pa_sink_inpu
     return PA_HOOK_OK;
 }
 
+static void update_preferred_device(struct userdata *u, const char *name, const char *device, const char *card) {
+    struct entry *old;
+    struct entry *entry;
+#ifdef HAVE_DBUS
+    bool created_new_entry = false;
+    struct dbus_entry *de;
+#endif
+
+    pa_assert(u);
+    pa_assert(name);
+
+    if ((old = entry_read(u, name)))
+        entry = entry_copy(old);
+    else {
+        entry = entry_new();
+#ifdef HAVE_DBUS
+        created_new_entry = true;
+#endif
+    }
+
+    pa_xfree(entry->device);
+    entry->device = pa_xstrdup(device);
+    entry->device_valid = !!device;
+
+    pa_xfree(entry->card);
+    entry->card = pa_xstrdup(card);
+    entry->card_valid = !!card;
+
+    pa_log_info("Storing device for stream %s.", name);
+
+    entry_write(u, name, entry, true);
+    trigger_save(u);
+
+#if HAVE_DBUS
+    if (!(de = pa_hashmap_get(u->dbus_entries, name))) {
+        de = dbus_entry_new(u, name);
+        pa_assert_se(pa_hashmap_put(u->dbus_entries, de->entry_name, de) == 0);
+        send_new_entry_signal(de);
+    } else {
+        /* We send a D-Bus signal when the device changes, but not when the
+         * card changes. That's becaues the D-Bus interface doesn't expose the
+         * card field to clients at all. */
+        if (!created_new_entry && !pa_safe_streq(entry->device, old->device))
+            send_device_updated_signal(de, entry);
+    }
+#endif
+
+    entry_free(entry);
+    if (old)
+        entry_free(old);
+}
+
+static pa_hook_result_t sink_input_preferred_sink_changed_cb(pa_core *c, pa_sink_input *sink_input, struct userdata *u) {
+    char *name;
+    pa_sink *sink;
+    const char *card_name;
+
+    pa_assert(c);
+    pa_assert(sink_input);
+    pa_assert(u);
+
+    if (!(name = pa_proplist_get_stream_group(sink_input->proplist, "sink-input", IDENTIFICATION_PROPERTY)))
+        return PA_HOOK_OK;
+
+    if (sink_input->preferred_sink && (sink = pa_namereg_get(c, sink_input->preferred_sink, PA_NAMEREG_SINK)) && sink->card)
+        card_name = sink->card->name;
+
+    update_preferred_device(u, name, sink_input->preferred_sink, card_name);
+    pa_xfree(name);
+
+    return PA_HOOK_OK;
+}
+
 static pa_hook_result_t source_output_new_hook_callback(pa_core *c, pa_source_output_new_data *new_data, struct userdata *u) {
     char *name;
     struct entry *e;
@@ -1690,6 +1720,27 @@ static pa_hook_result_t source_output_fixate_hook_callback(pa_core *c, pa_source
     return PA_HOOK_OK;
 }
 
+static pa_hook_result_t source_output_preferred_source_changed_cb(pa_core *c, pa_source_output *source_output, struct userdata *u) {
+    char *name;
+    pa_source *source;
+    const char *card_name;
+
+    pa_assert(c);
+    pa_assert(source_output);
+    pa_assert(u);
+
+    if (!(name = pa_proplist_get_stream_group(source_output->proplist, "source-output", IDENTIFICATION_PROPERTY)))
+        return PA_HOOK_OK;
+
+    if (source_output->preferred_source && (source = pa_namereg_get(c, source_output->preferred_source, PA_NAMEREG_SOURCE)) && source->card)
+        card_name = source->card->name;
+
+    update_preferred_device(u, name, source_output->preferred_source, card_name);
+    pa_xfree(name);
+
+    return PA_HOOK_OK;
+}
+
 static int fill_db(struct userdata *u, const char *filename) {
     FILE *f;
     int n = 0;
@@ -2317,6 +2368,11 @@ int pa__init(pa_module*m) {
         /* A little bit earlier than module-intended-roles ... */
         pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SINK_INPUT_NEW], PA_HOOK_EARLY, (pa_hook_cb_t) sink_input_new_hook_callback, u);
         pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_NEW], PA_HOOK_EARLY, (pa_hook_cb_t) source_output_new_hook_callback, u);
+
+        pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SINK_INPUT_PREFERRED_SINK_CHANGED], PA_HOOK_NORMAL,
+                               (pa_hook_cb_t) sink_input_preferred_sink_changed_cb, u);
+        pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_PREFERRED_SOURCE_CHANGED], PA_HOOK_NORMAL,
+                               (pa_hook_cb_t) source_output_preferred_source_changed_cb, u);
     }
 
     if (restore_volume || restore_muted) {


=====================================
src/pulsecore/core.h
=====================================
@@ -104,6 +104,7 @@ typedef enum pa_core_hook {
     PA_CORE_HOOK_SINK_INPUT_PROPLIST_CHANGED,
     PA_CORE_HOOK_SINK_INPUT_VOLUME_CHANGED,
     PA_CORE_HOOK_SINK_INPUT_MUTE_CHANGED,
+    PA_CORE_HOOK_SINK_INPUT_PREFERRED_SINK_CHANGED,
     PA_CORE_HOOK_SINK_INPUT_SEND_EVENT,
     PA_CORE_HOOK_SOURCE_OUTPUT_NEW,
     PA_CORE_HOOK_SOURCE_OUTPUT_FIXATE,
@@ -117,6 +118,7 @@ typedef enum pa_core_hook {
     PA_CORE_HOOK_SOURCE_OUTPUT_PROPLIST_CHANGED,
     PA_CORE_HOOK_SOURCE_OUTPUT_VOLUME_CHANGED,
     PA_CORE_HOOK_SOURCE_OUTPUT_MUTE_CHANGED,
+    PA_CORE_HOOK_SOURCE_OUTPUT_PREFERRED_SOURCE_CHANGED,
     PA_CORE_HOOK_SOURCE_OUTPUT_SEND_EVENT,
     PA_CORE_HOOK_CLIENT_NEW,
     PA_CORE_HOOK_CLIENT_PUT,


=====================================
src/pulsecore/sink-input.c
=====================================
@@ -1888,6 +1888,22 @@ 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_preferred_sink(pa_sink_input *i, const char *sink_name) {
+    pa_assert(i);
+
+    if (pa_safe_streq(i->preferred_sink, sink_name))
+        return;
+
+    pa_log_debug("Sink input %u: preferred_sink: %s -> %s",
+                 i->index, i->preferred_sink ? i->preferred_sink : "(unset)", sink_name ? sink_name : "(unset)");
+    pa_xfree(i->preferred_sink);
+    i->preferred_sink = pa_xstrdup(sink_name);
+
+    pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT | PA_SUBSCRIPTION_EVENT_CHANGE, i->index);
+    pa_hook_fire(&i->core->hooks[PA_CORE_HOOK_SINK_INPUT_PREFERRED_SINK_CHANGED], i);
+}
+
 /* Called from main context */
 int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {
     struct volume_factor_entry *v;
@@ -1930,11 +1946,10 @@ int pa_sink_input_finish_move(pa_sink_input *i, pa_sink *dest, bool save) {
     /* save == true, means user is calling the move_to() and want to
        save the preferred_sink */
     if (save) {
-        pa_xfree(i->preferred_sink);
         if (dest == dest->core->default_sink)
-            i->preferred_sink = NULL;
+            set_preferred_sink(i, NULL);
         else
-            i->preferred_sink = pa_xstrdup(dest->name);
+            set_preferred_sink(i, dest->name);
     }
 
     pa_idxset_put(dest->inputs, pa_sink_input_ref(i), NULL);
@@ -2434,16 +2449,29 @@ void pa_sink_input_set_reference_ratio(pa_sink_input *i, const pa_cvolume *ratio
                  pa_cvolume_snprint_verbose(new_ratio_str, sizeof(new_ratio_str), ratio, &i->channel_map, true));
 }
 
-/* Called from the main thread. */
+/* Called from the main thread.
+ *
+ * This is called when e.g. module-stream-restore wants to change the preferred
+ * sink. As a side effect the stream is moved to the new preferred sink. Note
+ * that things can work also in the other direction: if the user moves
+ * a stream, as a side effect the preferred sink is changed. This could cause
+ * an infinite loop, but it's avoided by these two measures:
+ *   - When pa_sink_input_set_preferred_sink() is called, it calls
+ *     pa_sink_input_move_to() with save=false, which avoids the recursive
+ *     pa_sink_input_set_preferred_sink() call.
+ *   - When the primary operation is to move a stream,
+ *     pa_sink_input_finish_move() calls set_preferred_sink() instead of
+ *     pa_sink_input_set_preferred_sink(). set_preferred_sink() doesn't move
+ *     the stream as a side effect.
+ */
 void pa_sink_input_set_preferred_sink(pa_sink_input *i, pa_sink *s) {
     pa_assert(i);
 
-    pa_xfree(i->preferred_sink);
     if (s) {
-        i->preferred_sink = pa_xstrdup(s->name);
+        set_preferred_sink(i, s->name);
         pa_sink_input_move_to(i, s, false);
     } else {
-        i->preferred_sink = NULL;
+        set_preferred_sink(i, NULL);
         pa_sink_input_move_to(i, i->core->default_sink, false);
     }
 }


=====================================
src/pulsecore/source-output.c
=====================================
@@ -1531,6 +1531,22 @@ 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_preferred_source(pa_source_output *o, const char *source_name) {
+    pa_assert(o);
+
+    if (pa_safe_streq(o->preferred_source, source_name))
+        return;
+
+    pa_log_debug("Source output %u: preferred_source: %s -> %s",
+                 o->index, o->preferred_source ? o->preferred_source : "(unset)", source_name ? source_name : "(unset)");
+    pa_xfree(o->preferred_source);
+    o->preferred_source = pa_xstrdup(source_name);
+
+    pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT | PA_SUBSCRIPTION_EVENT_CHANGE, o->index);
+    pa_hook_fire(&o->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_PREFERRED_SOURCE_CHANGED], o);
+}
+
 /* Called from main context */
 int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save) {
     pa_source_output_assert_ref(o);
@@ -1570,11 +1586,10 @@ int pa_source_output_finish_move(pa_source_output *o, pa_source *dest, bool save
     /* save == true, means user is calling the move_to() and want to
        save the preferred_source */
     if (save) {
-        pa_xfree(o->preferred_source);
         if (dest == dest->core->default_source)
-            o->preferred_source = NULL;
+            set_preferred_source(o, NULL);
         else
-            o->preferred_source = pa_xstrdup(dest->name);
+            set_preferred_source(o, dest->name);
     }
 
     pa_idxset_put(o->source->outputs, pa_source_output_ref(o), NULL);
@@ -1907,16 +1922,29 @@ void pa_source_output_set_reference_ratio(pa_source_output *o, const pa_cvolume
                  pa_cvolume_snprint_verbose(new_ratio_str, sizeof(new_ratio_str), ratio, &o->channel_map, true));
 }
 
-/* Called from the main thread. */
+/* Called from the main thread.
+ *
+ * This is called when e.g. module-stream-restore wants to change the preferred
+ * source. As a side effect the stream is moved to the new preferred source.
+ * Note that things can work also in the other direction: if the user moves
+ * a stream, as a side effect the preferred source is changed. This could cause
+ * an infinite loop, but it's avoided by these two measures:
+ *   - When pa_source_output_set_preferred_source() is called, it calls
+ *     pa_source_output_move_to() with save=false, which avoids the recursive
+ *     pa_source_output_set_preferred_source() call.
+ *   - When the primary operation is to move a stream,
+ *     pa_source_output_finish_move() calls set_preferred_source() instead of
+ *     pa_source_output_set_preferred_source(). set_preferred_source() doesn't
+ *     move the stream as a side effect.
+ */
 void pa_source_output_set_preferred_source(pa_source_output *o, pa_source *s) {
     pa_assert(o);
 
-    pa_xfree(o->preferred_source);
     if (s) {
-        o->preferred_source = pa_xstrdup(s->name);
+        set_preferred_source(o, s->name);
         pa_source_output_move_to(o, s, false);
     } else {
-        o->preferred_source = NULL;
+        set_preferred_source(o, NULL);
         pa_source_output_move_to(o, o->core->default_source, false);
     }
 }



View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/compare/1f204a1357f2d26a4cca12ea9e017db90dd2d961...8a87af380a89bfcc9d29a915828ddc43456569a5

-- 
View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/compare/1f204a1357f2d26a4cca12ea9e017db90dd2d961...8a87af380a89bfcc9d29a915828ddc43456569a5
You're receiving this email because of your account on gitlab.freedesktop.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pulseaudio-commits/attachments/20210405/04593f3c/attachment-0001.htm>


More information about the pulseaudio-commits mailing list