[pulseaudio-commits] 7 commits - src/modules src/pulsecore

Tanu Kaskinen tanuk at kemper.freedesktop.org
Mon Apr 25 10:54:04 UTC 2016


 src/modules/echo-cancel/module-echo-cancel.c |   28 ++---
 src/modules/module-device-manager.c          |   20 +++
 src/modules/module-filter-apply.c            |   11 +-
 src/modules/module-loopback.c                |   28 ++---
 src/pulsecore/protocol-native.c              |    4 
 src/pulsecore/sink-input.c                   |  145 +++++++++++++++++++++------
 src/pulsecore/sink-input.h                   |    4 
 src/pulsecore/sink.c                         |    7 -
 src/pulsecore/sink.h                         |    6 +
 src/pulsecore/source-output.c                |  145 +++++++++++++++++++++------
 src/pulsecore/source-output.h                |    4 
 src/pulsecore/source.c                       |    5 
 src/pulsecore/source.h                       |    6 +
 13 files changed, 310 insertions(+), 103 deletions(-)

New commits:
commit 09f11ee48279e28a73492ce5ccf7854808913f57
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Fri Mar 18 16:22:00 2016 +0200

    filter-apply: simplify proplist updating
    
    pa_sink_input_set_property() takes care of logging, so the logging
    code is redundant.

diff --git a/src/modules/module-filter-apply.c b/src/modules/module-filter-apply.c
index f5ff0a3..d09efb4 100644
--- a/src/modules/module-filter-apply.c
+++ b/src/modules/module-filter-apply.c
@@ -272,55 +272,13 @@ static void trigger_housekeeping(struct userdata *u) {
 
 static int do_move(pa_object *obj, pa_object *parent, bool restore, bool is_input) {
     if (is_input) {
-        if (!restore) {
-            char *old_value;
-
-            if (pa_proplist_contains(PA_SINK_INPUT(obj)->proplist, "module-filter-apply.filter_device")) {
-                old_value = pa_xstrdup(pa_proplist_gets(PA_SINK_INPUT(obj)->proplist, "module-filter-apply.filter_device"));
-                if (!old_value)
-                    old_value = pa_xstrdup("(data)");
-            } else
-                old_value = pa_xstrdup("(unset)");
-
-            if (!pa_streq(PA_SINK(parent)->name, old_value)) {
-                pa_proplist *pl;
-
-                pl = pa_proplist_new();
-                pa_proplist_sets(pl, "module-filter-apply.filter_device", PA_SINK(parent)->name);
-                pa_sink_input_update_proplist(PA_SINK_INPUT(obj), PA_UPDATE_REPLACE, pl);
-                pa_proplist_free(pl);
-                pa_log_debug("Sink input %u: proplist[module-filter-apply.filter_device]: %s -> %s",
-                             PA_SINK_INPUT(obj)->index, old_value, PA_SINK(parent)->name);
-            }
-
-            pa_xfree(old_value);
-        }
+        if (!restore)
+            pa_sink_input_set_property(PA_SINK_INPUT(obj), "module-filter-apply.filter_device", PA_SINK(parent)->name);
 
         return pa_sink_input_move_to(PA_SINK_INPUT(obj), PA_SINK(parent), restore);
     } else {
-        if (!restore) {
-            char *old_value;
-
-            if (pa_proplist_contains(PA_SOURCE_OUTPUT(obj)->proplist, "module-filter-apply.filter_device")) {
-                old_value = pa_xstrdup(pa_proplist_gets(PA_SOURCE_OUTPUT(obj)->proplist, "module-filter-apply.filter_device"));
-                if (!old_value)
-                    old_value = pa_xstrdup("(data)");
-            } else
-                old_value = pa_xstrdup("(unset)");
-
-            if (!pa_streq(PA_SOURCE(parent)->name, old_value)) {
-                pa_proplist *pl;
-
-                pl = pa_proplist_new();
-                pa_proplist_sets(pl, "module-filter-apply.filter_device", PA_SOURCE(parent)->name);
-                pa_source_output_update_proplist(PA_SOURCE_OUTPUT(obj), PA_UPDATE_REPLACE, pl);
-                pa_proplist_free(pl);
-                pa_log_debug("Source output %u: proplist[module-filter-apply.filter_device]: %s -> %s",
-                             PA_SOURCE_OUTPUT(obj)->index, old_value, PA_SOURCE(parent)->name);
-            }
-
-            pa_xfree(old_value);
-        }
+        if (!restore)
+            pa_source_output_set_property(PA_SOURCE_OUTPUT(obj), "module-filter-apply.filter_device", PA_SOURCE(parent)->name);
 
         return pa_source_output_move_to(PA_SOURCE_OUTPUT(obj), PA_SOURCE(parent), restore);
     }

commit 95dd90ce06046fb574e4a2174545bdebeb7c460d
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Thu Mar 10 19:43:01 2016 +0200

    loopback: refactor proplist updating
    
    This saves some proplist allocations and a couple of code lines. Also,
    logging is better, because the set_property() functions work with
    string values, while the update_proplist() functions assume opaque
    binary data, and therefore can't log the property values.

diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
index 37bf7b1..f4d0761 100644
--- a/src/modules/module-loopback.c
+++ b/src/modules/module-loopback.c
@@ -398,9 +398,9 @@ static bool source_output_may_move_to_cb(pa_source_output *o, pa_source *dest) {
 
 /* Called from main thread */
 static void source_output_moving_cb(pa_source_output *o, pa_source *dest) {
-    pa_proplist *p;
-    const char *n;
     struct userdata *u;
+    char *input_description;
+    const char *n;
 
     if (!dest)
         return;
@@ -409,14 +409,13 @@ static void source_output_moving_cb(pa_source_output *o, pa_source *dest) {
     pa_assert_ctl_context();
     pa_assert_se(u = o->userdata);
 
-    p = pa_proplist_new();
-    pa_proplist_setf(p, PA_PROP_MEDIA_NAME, "Loopback of %s", pa_strnull(pa_proplist_gets(dest->proplist, PA_PROP_DEVICE_DESCRIPTION)));
+    input_description = pa_sprintf_malloc("Loopback of %s",
+                                          pa_strnull(pa_proplist_gets(dest->proplist, PA_PROP_DEVICE_DESCRIPTION)));
+    pa_sink_input_set_property(u->sink_input, PA_PROP_MEDIA_NAME, input_description);
+    pa_xfree(input_description);
 
     if ((n = pa_proplist_gets(dest->proplist, PA_PROP_DEVICE_ICON_NAME)))
-        pa_proplist_sets(p, PA_PROP_MEDIA_ICON_NAME, n);
-
-    pa_sink_input_update_proplist(u->sink_input, PA_UPDATE_REPLACE, p);
-    pa_proplist_free(p);
+        pa_sink_input_set_property(u->sink_input, PA_PROP_DEVICE_ICON_NAME, n);
 
     if (pa_source_get_state(dest) == PA_SOURCE_SUSPENDED)
         pa_sink_input_cork(u->sink_input, true);
@@ -671,7 +670,7 @@ static void sink_input_state_change_cb(pa_sink_input *i, pa_sink_input_state_t s
 /* Called from main thread */
 static void sink_input_moving_cb(pa_sink_input *i, pa_sink *dest) {
     struct userdata *u;
-    pa_proplist *p;
+    char *output_description;
     const char *n;
 
     if (!dest)
@@ -681,14 +680,13 @@ static void sink_input_moving_cb(pa_sink_input *i, pa_sink *dest) {
     pa_assert_ctl_context();
     pa_assert_se(u = i->userdata);
 
-    p = pa_proplist_new();
-    pa_proplist_setf(p, PA_PROP_MEDIA_NAME, "Loopback to %s", pa_strnull(pa_proplist_gets(dest->proplist, PA_PROP_DEVICE_DESCRIPTION)));
+    output_description = pa_sprintf_malloc("Loopback to %s",
+                                           pa_strnull(pa_proplist_gets(dest->proplist, PA_PROP_DEVICE_DESCRIPTION)));
+    pa_source_output_set_property(u->source_output, PA_PROP_MEDIA_NAME, output_description);
+    pa_xfree(output_description);
 
     if ((n = pa_proplist_gets(dest->proplist, PA_PROP_DEVICE_ICON_NAME)))
-        pa_proplist_sets(p, PA_PROP_MEDIA_ICON_NAME, n);
-
-    pa_source_output_update_proplist(u->source_output, PA_UPDATE_REPLACE, p);
-    pa_proplist_free(p);
+        pa_source_output_set_property(u->source_output, PA_PROP_MEDIA_ICON_NAME, n);
 
     if (pa_sink_get_state(dest) == PA_SINK_SUSPENDED)
         pa_source_output_cork(u->source_output, true);

commit 16b46249614f23fd6601e34a773b170a526f9b75
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Sun Mar 13 13:57:31 2016 +0200

    sink-input, source-output: remove set_name()
    
    pa_sink_input_set_property() does everything pa_sink_input_set_name()
    does.

diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
index 5697588..93a7ce0 100644
--- a/src/pulsecore/protocol-native.c
+++ b/src/pulsecore/protocol-native.c
@@ -4486,7 +4486,7 @@ static void command_set_stream_name(pa_pdispatch *pd, uint32_t command, uint32_t
         CHECK_VALIDITY(c->pstream, s, tag, PA_ERR_NOENTITY);
         CHECK_VALIDITY(c->pstream, playback_stream_isinstance(s), tag, PA_ERR_NOENTITY);
 
-        pa_sink_input_set_name(s->sink_input, name);
+        pa_sink_input_set_property(s->sink_input, PA_PROP_MEDIA_NAME, name);
 
     } else {
         record_stream *s;
@@ -4495,7 +4495,7 @@ static void command_set_stream_name(pa_pdispatch *pd, uint32_t command, uint32_t
         s = pa_idxset_get_by_index(c->record_streams, idx);
         CHECK_VALIDITY(c->pstream, s, tag, PA_ERR_NOENTITY);
 
-        pa_source_output_set_name(s->source_output, name);
+        pa_source_output_set_property(s->source_output, PA_PROP_MEDIA_NAME, name);
     }
 
     pa_pstream_send_simple_ack(c->pstream, tag);
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 42d0b32..361b445 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -1575,31 +1575,6 @@ int pa_sink_input_set_rate(pa_sink_input *i, uint32_t rate) {
 }
 
 /* Called from main context */
-void pa_sink_input_set_name(pa_sink_input *i, const char *name) {
-    const char *old;
-    pa_sink_input_assert_ref(i);
-    pa_assert_ctl_context();
-
-    if (!name && !pa_proplist_contains(i->proplist, PA_PROP_MEDIA_NAME))
-        return;
-
-    old = pa_proplist_gets(i->proplist, PA_PROP_MEDIA_NAME);
-
-    if (old && name && pa_streq(old, name))
-        return;
-
-    if (name)
-        pa_proplist_sets(i->proplist, PA_PROP_MEDIA_NAME, name);
-    else
-        pa_proplist_unset(i->proplist, PA_PROP_MEDIA_NAME);
-
-    if (PA_SINK_INPUT_IS_LINKED(i->state)) {
-        pa_hook_fire(&i->core->hooks[PA_CORE_HOOK_SINK_INPUT_PROPLIST_CHANGED], i);
-        pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT|PA_SUBSCRIPTION_EVENT_CHANGE, i->index);
-    }
-}
-
-/* Called from main context */
 pa_resample_method_t pa_sink_input_get_resample_method(pa_sink_input *i) {
     pa_sink_input_assert_ref(i);
     pa_assert_ctl_context();
diff --git a/src/pulsecore/sink-input.h b/src/pulsecore/sink-input.h
index 3485d6e..8bbee4e 100644
--- a/src/pulsecore/sink-input.h
+++ b/src/pulsecore/sink-input.h
@@ -335,8 +335,6 @@ int pa_sink_input_new(
 void pa_sink_input_put(pa_sink_input *i);
 void pa_sink_input_unlink(pa_sink_input* i);
 
-void pa_sink_input_set_name(pa_sink_input *i, const char *name);
-
 pa_usec_t pa_sink_input_set_requested_latency(pa_sink_input *i, pa_usec_t usec);
 
 /* Request that the specified number of bytes already written out to
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index f3910c5..d74a60e 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -1227,31 +1227,6 @@ int pa_source_output_set_rate(pa_source_output *o, uint32_t rate) {
 }
 
 /* Called from main context */
-void pa_source_output_set_name(pa_source_output *o, const char *name) {
-    const char *old;
-    pa_assert_ctl_context();
-    pa_source_output_assert_ref(o);
-
-    if (!name && !pa_proplist_contains(o->proplist, PA_PROP_MEDIA_NAME))
-        return;
-
-    old = pa_proplist_gets(o->proplist, PA_PROP_MEDIA_NAME);
-
-    if (old && name && pa_streq(old, name))
-        return;
-
-    if (name)
-        pa_proplist_sets(o->proplist, PA_PROP_MEDIA_NAME, name);
-    else
-        pa_proplist_unset(o->proplist, PA_PROP_MEDIA_NAME);
-
-    if (PA_SOURCE_OUTPUT_IS_LINKED(o->state)) {
-        pa_hook_fire(&o->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_PROPLIST_CHANGED], o);
-        pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT|PA_SUBSCRIPTION_EVENT_CHANGE, o->index);
-    }
-}
-
-/* Called from main context */
 pa_resample_method_t pa_source_output_get_resample_method(pa_source_output *o) {
     pa_source_output_assert_ref(o);
     pa_assert_ctl_context();
diff --git a/src/pulsecore/source-output.h b/src/pulsecore/source-output.h
index b804092..7ac44bd 100644
--- a/src/pulsecore/source-output.h
+++ b/src/pulsecore/source-output.h
@@ -291,8 +291,6 @@ int pa_source_output_new(
 void pa_source_output_put(pa_source_output *o);
 void pa_source_output_unlink(pa_source_output*o);
 
-void pa_source_output_set_name(pa_source_output *o, const char *name);
-
 pa_usec_t pa_source_output_set_requested_latency(pa_source_output *o, pa_usec_t usec);
 
 void pa_source_output_cork(pa_source_output *o, bool b);

commit 3e7e901ba0a8e368b3ac99febebdd3918495e3c6
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Fri Mar 11 12:02:22 2016 +0200

    sink-input, source-output: rework property setting
    
    pa_sink_input_update_proplist() is inconvenient in many cases, because
    it requires allocating a new proplist, even if the goal is to just set
    one property. pa_sink_input_update_properties also can't properly log
    property changes, because it has to assume that all values are
    arbitrary binary data.
    
    This patch adds pa_sink_input_set_property() for setting a string
    value for a single property, and pa_sink_input_set_property_arbitrary()
    for setting a binary value for a single property.
    pa_sink_input_update_properties() is reimplemented as a wrapper around
    pa_sink_input_set_property_arbitrary() to centralize logging and
    sending change notifications.
    
    (The above mentions only sink input functions for brevity, but the
    same changes are implemented for source outputs too.)

diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 843297f..42d0b32 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -1426,17 +1426,124 @@ void pa_sink_input_set_mute(pa_sink_input *i, bool mute, bool save) {
     pa_hook_fire(&i->core->hooks[PA_CORE_HOOK_SINK_INPUT_MUTE_CHANGED], i);
 }
 
+void pa_sink_input_set_property(pa_sink_input *i, const char *key, const char *value) {
+    char *old_value = NULL;
+    const char *new_value;
+
+    pa_assert(i);
+    pa_assert(key);
+
+    if (pa_proplist_contains(i->proplist, key)) {
+        old_value = pa_xstrdup(pa_proplist_gets(i->proplist, key));
+        if (old_value) {
+            if (pa_streq(value, old_value))
+                goto finish;
+        } else
+            old_value = pa_xstrdup("(data)");
+    } else {
+        if (!value)
+            goto finish;
+
+        old_value = pa_xstrdup("(unset)");
+    }
+
+    if (value) {
+        pa_proplist_sets(i->proplist, key, value);
+        new_value = value;
+    } else {
+        pa_proplist_unset(i->proplist, key);
+        new_value = "(unset)";
+    }
+
+    if (PA_SINK_INPUT_IS_LINKED(i->state)) {
+        pa_log_debug("Sink input %u: proplist[%s]: %s -> %s", i->index, key, old_value, new_value);
+        pa_hook_fire(&i->core->hooks[PA_CORE_HOOK_SINK_INPUT_PROPLIST_CHANGED], i);
+        pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT | PA_SUBSCRIPTION_EVENT_CHANGE, i->index);
+    }
+
+finish:
+    pa_xfree(old_value);
+}
+
+void pa_sink_input_set_property_arbitrary(pa_sink_input *i, const char *key, const uint8_t *value, size_t nbytes) {
+    const uint8_t *old_value;
+    size_t old_nbytes;
+    const char *old_value_str;
+    const char *new_value_str;
+
+    pa_assert(i);
+    pa_assert(key);
+
+    if (pa_proplist_get(i->proplist, key, (const void **) &old_value, &old_nbytes) >= 0) {
+        if (value && nbytes == old_nbytes && !memcmp(value, old_value, nbytes))
+            return;
+
+        old_value_str = "(data)";
+
+    } else {
+        if (!value)
+            return;
+
+        old_value_str = "(unset)";
+    }
+
+    if (value) {
+        pa_proplist_set(i->proplist, key, value, nbytes);
+        new_value_str = "(data)";
+    } else {
+        pa_proplist_unset(i->proplist, key);
+        new_value_str = "(unset)";
+    }
+
+    if (PA_SINK_INPUT_IS_LINKED(i->state)) {
+        pa_log_debug("Sink input %u: proplist[%s]: %s -> %s", i->index, key, old_value_str, new_value_str);
+        pa_hook_fire(&i->core->hooks[PA_CORE_HOOK_SINK_INPUT_PROPLIST_CHANGED], i);
+        pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT | PA_SUBSCRIPTION_EVENT_CHANGE, i->index);
+    }
+}
+
 /* Called from main thread */
 void pa_sink_input_update_proplist(pa_sink_input *i, pa_update_mode_t mode, pa_proplist *p) {
+    void *state;
+    const char *key;
+    const uint8_t *value;
+    size_t nbytes;
+
     pa_sink_input_assert_ref(i);
+    pa_assert(p);
     pa_assert_ctl_context();
 
-    if (p)
-        pa_proplist_update(i->proplist, mode, p);
+    switch (mode) {
+        case PA_UPDATE_SET: {
+            /* Delete everything that is not in p. */
+            for (state = NULL; (key = pa_proplist_iterate(i->proplist, &state));) {
+                if (!pa_proplist_contains(p, key))
+                    pa_sink_input_set_property(i, key, NULL);
+            }
 
-    if (PA_SINK_INPUT_IS_LINKED(i->state)) {
-        pa_hook_fire(&i->core->hooks[PA_CORE_HOOK_SINK_INPUT_PROPLIST_CHANGED], i);
-        pa_subscription_post(i->core, PA_SUBSCRIPTION_EVENT_SINK_INPUT|PA_SUBSCRIPTION_EVENT_CHANGE, i->index);
+            /* Fall through. */
+        }
+
+        case PA_UPDATE_REPLACE: {
+            for (state = NULL; (key = pa_proplist_iterate(p, &state));) {
+                pa_proplist_get(p, key, (const void **) &value, &nbytes);
+                pa_sink_input_set_property_arbitrary(i, key, value, nbytes);
+            }
+
+            break;
+        }
+
+        case PA_UPDATE_MERGE: {
+            for (state = NULL; (key = pa_proplist_iterate(p, &state));) {
+                if (pa_proplist_contains(i->proplist, key))
+                    continue;
+
+                pa_proplist_get(p, key, (const void **) &value, &nbytes);
+                pa_sink_input_set_property_arbitrary(i, key, value, nbytes);
+            }
+
+            break;
+        }
     }
 }
 
diff --git a/src/pulsecore/sink-input.h b/src/pulsecore/sink-input.h
index 86deab2..3485d6e 100644
--- a/src/pulsecore/sink-input.h
+++ b/src/pulsecore/sink-input.h
@@ -373,6 +373,8 @@ pa_cvolume *pa_sink_input_get_volume(pa_sink_input *i, pa_cvolume *volume, bool
 
 void pa_sink_input_set_mute(pa_sink_input *i, bool mute, bool save);
 
+void pa_sink_input_set_property(pa_sink_input *i, const char *key, const char *value);
+void pa_sink_input_set_property_arbitrary(pa_sink_input *i, const char *key, const uint8_t *value, size_t nbytes);
 void pa_sink_input_update_proplist(pa_sink_input *i, pa_update_mode_t mode, pa_proplist *p);
 
 pa_resample_method_t pa_sink_input_get_resample_method(pa_sink_input *i);
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 6d54ae8..f3910c5 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -1078,17 +1078,124 @@ void pa_source_output_set_mute(pa_source_output *o, bool mute, bool save) {
     pa_hook_fire(&o->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_MUTE_CHANGED], o);
 }
 
+void pa_source_output_set_property(pa_source_output *o, const char *key, const char *value) {
+    char *old_value = NULL;
+    const char *new_value;
+
+    pa_assert(o);
+    pa_assert(key);
+
+    if (pa_proplist_contains(o->proplist, key)) {
+        old_value = pa_xstrdup(pa_proplist_gets(o->proplist, key));
+        if (old_value) {
+            if (pa_streq(value, old_value))
+                goto finish;
+        } else
+            old_value = pa_xstrdup("(data)");
+    } else {
+        if (!value)
+            goto finish;
+
+        old_value = pa_xstrdup("(unset)");
+    }
+
+    if (value) {
+        pa_proplist_sets(o->proplist, key, value);
+        new_value = value;
+    } else {
+        pa_proplist_unset(o->proplist, key);
+        new_value = "(unset)";
+    }
+
+    if (PA_SINK_INPUT_IS_LINKED(o->state)) {
+        pa_log_debug("Source output %u: proplist[%s]: %s -> %s", o->index, key, old_value, new_value);
+        pa_hook_fire(&o->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_PROPLIST_CHANGED], o);
+        pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT | PA_SUBSCRIPTION_EVENT_CHANGE, o->index);
+    }
+
+finish:
+    pa_xfree(old_value);
+}
+
+void pa_source_output_set_property_arbitrary(pa_source_output *o, const char *key, const uint8_t *value, size_t nbytes) {
+    const uint8_t *old_value;
+    size_t old_nbytes;
+    const char *old_value_str;
+    const char *new_value_str;
+
+    pa_assert(o);
+    pa_assert(key);
+
+    if (pa_proplist_get(o->proplist, key, (const void **) &old_value, &old_nbytes) >= 0) {
+        if (value && nbytes == old_nbytes && !memcmp(value, old_value, nbytes))
+            return;
+
+        old_value_str = "(data)";
+
+    } else {
+        if (!value)
+            return;
+
+        old_value_str = "(unset)";
+    }
+
+    if (value) {
+        pa_proplist_set(o->proplist, key, value, nbytes);
+        new_value_str = "(data)";
+    } else {
+        pa_proplist_unset(o->proplist, key);
+        new_value_str = "(unset)";
+    }
+
+    if (PA_SOURCE_OUTPUT_IS_LINKED(o->state)) {
+        pa_log_debug("Source output %u: proplist[%s]: %s -> %s", o->index, key, old_value_str, new_value_str);
+        pa_hook_fire(&o->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_PROPLIST_CHANGED], o);
+        pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT | PA_SUBSCRIPTION_EVENT_CHANGE, o->index);
+    }
+}
+
 /* Called from main thread */
 void pa_source_output_update_proplist(pa_source_output *o, pa_update_mode_t mode, pa_proplist *p) {
+    void *state;
+    const char *key;
+    const uint8_t *value;
+    size_t nbytes;
+
     pa_source_output_assert_ref(o);
+    pa_assert(p);
     pa_assert_ctl_context();
 
-    if (p)
-        pa_proplist_update(o->proplist, mode, p);
+    switch (mode) {
+        case PA_UPDATE_SET: {
+            /* Delete everything that is not in p. */
+            for (state = NULL; (key = pa_proplist_iterate(o->proplist, &state));) {
+                if (!pa_proplist_contains(p, key))
+                    pa_source_output_set_property(o, key, NULL);
+            }
 
-    if (PA_SOURCE_OUTPUT_IS_LINKED(o->state)) {
-        pa_hook_fire(&o->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_PROPLIST_CHANGED], o);
-        pa_subscription_post(o->core, PA_SUBSCRIPTION_EVENT_SOURCE_OUTPUT|PA_SUBSCRIPTION_EVENT_CHANGE, o->index);
+            /* Fall through. */
+        }
+
+        case PA_UPDATE_REPLACE: {
+            for (state = NULL; (key = pa_proplist_iterate(p, &state));) {
+                pa_proplist_get(p, key, (const void **) &value, &nbytes);
+                pa_source_output_set_property_arbitrary(o, key, value, nbytes);
+            }
+
+            break;
+        }
+
+        case PA_UPDATE_MERGE: {
+            for (state = NULL; (key = pa_proplist_iterate(p, &state));) {
+                if (pa_proplist_contains(o->proplist, key))
+                    continue;
+
+                pa_proplist_get(p, key, (const void **) &value, &nbytes);
+                pa_source_output_set_property_arbitrary(o, key, value, nbytes);
+            }
+
+            break;
+        }
     }
 }
 
diff --git a/src/pulsecore/source-output.h b/src/pulsecore/source-output.h
index 26be484..b804092 100644
--- a/src/pulsecore/source-output.h
+++ b/src/pulsecore/source-output.h
@@ -316,6 +316,8 @@ pa_cvolume *pa_source_output_get_volume(pa_source_output *o, pa_cvolume *volume,
 
 void pa_source_output_set_mute(pa_source_output *o, bool mute, bool save);
 
+void pa_source_output_set_property(pa_source_output *o, const char *key, const char *value);
+void pa_source_output_set_property_arbitrary(pa_source_output *o, const char *key, const uint8_t *value, size_t nbytes);
 void pa_source_output_update_proplist(pa_source_output *o, pa_update_mode_t mode, pa_proplist *p);
 
 pa_resample_method_t pa_source_output_get_resample_method(pa_source_output *o);

commit 085cced42ce0901a2f677fc2f630801b0f4edf80
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Mon Mar 7 13:18:29 2016 +0200

    device-manager, filter-apply: don't reroute streams that have a filter
    
    device-manager reroutes all streams whenever a new device appears.
    When filter-apply has loaded a filter for some stream, the filter
    device may not be what device-manager considers the best device for
    the stream, which means that when an unrelated device appears,
    device-manager may break the filtering that filter-apply had set up.
    
    This patch changes filter-apply so that it saves the filter device
    name to the stream proplist when it sets up a filter. device-manager
    can then check the proplist when it does rerouting, and skip the
    rerouting for streams that have a filter applied to them.
    
    The proplist isn't cleaned up when the stream moves away from the
    filter device, so before doing any decisions based on the
    filter_device property, it should be checked that the stream is
    currently routed to the filter device. It seemed simpler to do it this
    way compared to setting up stream move monitoring in filter-apply and
    removing the property when the stream moves away from the filter
    device.

diff --git a/src/modules/module-device-manager.c b/src/modules/module-device-manager.c
index 1a0a53e..0df9575 100644
--- a/src/modules/module-device-manager.c
+++ b/src/modules/module-device-manager.c
@@ -649,6 +649,7 @@ static void update_highest_priority_device_indexes(struct userdata *u, const cha
 }
 
 static void route_sink_input(struct userdata *u, pa_sink_input *si) {
+    const char *filter_device;
     const char *role;
     uint32_t role_index, device_index;
     pa_sink *sink;
@@ -663,6 +664,15 @@ static void route_sink_input(struct userdata *u, pa_sink_input *si) {
     if (!si->sink)
         return;
 
+    /* If module-filter-apply has loaded a filter for the stream, let's not
+     * break the filtering. The pa_streq() check is needed, because
+     * module-filter-apply doesn't unset the property when the stream moves
+     * away from the filter device for whatever reason (this is ugly, but
+     * easier to do this way than appropriately unsetting the property). */
+    filter_device = pa_proplist_gets(si->proplist, "module-filter-apply.filter_device");
+    if (filter_device && pa_streq(filter_device, si->sink->name))
+        return;
+
     /* It might happen that a stream and a sink are set up at the
     same time, in which case we want to make sure we don't
     interfere with that */
@@ -707,6 +717,7 @@ static pa_hook_result_t route_sink_inputs(struct userdata *u, pa_sink *ignore_si
 }
 
 static void route_source_output(struct userdata *u, pa_source_output *so) {
+    const char *filter_device;
     const char *role;
     uint32_t role_index, device_index;
     pa_source *source;
@@ -724,6 +735,15 @@ static void route_source_output(struct userdata *u, pa_source_output *so) {
     if (!so->source)
         return;
 
+    /* If module-filter-apply has loaded a filter for the stream, let's not
+     * break the filtering. The pa_streq() check is needed, because
+     * module-filter-apply doesn't unset the property when the stream moves
+     * away from the filter device for whatever reason (this is ugly, but
+     * easier to do this way than appropriately unsetting the property). */
+    filter_device = pa_proplist_gets(so->proplist, "module-filter-apply.filter_device");
+    if (filter_device && pa_streq(filter_device, so->source->name))
+        return;
+
     /* It might happen that a stream and a source are set up at the
     same time, in which case we want to make sure we don't
     interfere with that */
diff --git a/src/modules/module-filter-apply.c b/src/modules/module-filter-apply.c
index 7f4a2b7..f5ff0a3 100644
--- a/src/modules/module-filter-apply.c
+++ b/src/modules/module-filter-apply.c
@@ -271,10 +271,59 @@ static void trigger_housekeeping(struct userdata *u) {
 }
 
 static int do_move(pa_object *obj, pa_object *parent, bool restore, bool is_input) {
-    if (is_input)
+    if (is_input) {
+        if (!restore) {
+            char *old_value;
+
+            if (pa_proplist_contains(PA_SINK_INPUT(obj)->proplist, "module-filter-apply.filter_device")) {
+                old_value = pa_xstrdup(pa_proplist_gets(PA_SINK_INPUT(obj)->proplist, "module-filter-apply.filter_device"));
+                if (!old_value)
+                    old_value = pa_xstrdup("(data)");
+            } else
+                old_value = pa_xstrdup("(unset)");
+
+            if (!pa_streq(PA_SINK(parent)->name, old_value)) {
+                pa_proplist *pl;
+
+                pl = pa_proplist_new();
+                pa_proplist_sets(pl, "module-filter-apply.filter_device", PA_SINK(parent)->name);
+                pa_sink_input_update_proplist(PA_SINK_INPUT(obj), PA_UPDATE_REPLACE, pl);
+                pa_proplist_free(pl);
+                pa_log_debug("Sink input %u: proplist[module-filter-apply.filter_device]: %s -> %s",
+                             PA_SINK_INPUT(obj)->index, old_value, PA_SINK(parent)->name);
+            }
+
+            pa_xfree(old_value);
+        }
+
         return pa_sink_input_move_to(PA_SINK_INPUT(obj), PA_SINK(parent), restore);
-    else
+    } else {
+        if (!restore) {
+            char *old_value;
+
+            if (pa_proplist_contains(PA_SOURCE_OUTPUT(obj)->proplist, "module-filter-apply.filter_device")) {
+                old_value = pa_xstrdup(pa_proplist_gets(PA_SOURCE_OUTPUT(obj)->proplist, "module-filter-apply.filter_device"));
+                if (!old_value)
+                    old_value = pa_xstrdup("(data)");
+            } else
+                old_value = pa_xstrdup("(unset)");
+
+            if (!pa_streq(PA_SOURCE(parent)->name, old_value)) {
+                pa_proplist *pl;
+
+                pl = pa_proplist_new();
+                pa_proplist_sets(pl, "module-filter-apply.filter_device", PA_SOURCE(parent)->name);
+                pa_source_output_update_proplist(PA_SOURCE_OUTPUT(obj), PA_UPDATE_REPLACE, pl);
+                pa_proplist_free(pl);
+                pa_log_debug("Source output %u: proplist[module-filter-apply.filter_device]: %s -> %s",
+                             PA_SOURCE_OUTPUT(obj)->index, old_value, PA_SOURCE(parent)->name);
+            }
+
+            pa_xfree(old_value);
+        }
+
         return pa_source_output_move_to(PA_SOURCE_OUTPUT(obj), PA_SOURCE(parent), restore);
+    }
 }
 
 static void move_object_for_filter(pa_object *o, struct filter* filter, bool restore, bool is_sink_input) {

commit 13fc8333873b5cfd5f42abf0b6071bb028d9f096
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Sun Mar 13 14:06:02 2016 +0200

    don't move streams to devices that are going away
    
    Before a device is unlinked, the unlink hook is fired, and it's
    possible that a routing module tries to move streams to the unlinked
    device in that hook, because it doesn't know that the device is being
    unlinked. Of course, the unlinking is obvious when the code is in an
    unlink hook callback, but it's possible that some other module does
    something in the unlink hook that in turn triggers some other hook,
    and it's this second hook where the routing module may get confused.
    This patch adds an "unlink_requested" flag that is set before the
    unlink hook is fired, and moving streams to a device with that flag
    set is prevented.
    
    This patch is motivated by seeing module-device-manager moving a
    stream to a sink that was being unlinked. It was a complex case where
    an alsa card was changing its profile, while an echo-cancel sink was
    connected to the old alsa sink. module-always-sink loaded a null sink
    in the middle of the profile change, and after a stream had been
    rescued to the null sink, module-device-manager decided to move it
    back to the old alsa sink that was being unlinked. That move made no
    sense, so I came up with this patch.

diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 8ec63b5..843297f 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -1538,6 +1538,9 @@ bool pa_sink_input_may_move_to(pa_sink_input *i, pa_sink *dest) {
     if (dest == i->sink)
         return true;
 
+    if (dest->unlink_requested)
+        return false;
+
     if (!pa_sink_input_may_move(i))
         return false;
 
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 0b44fc7..3f1ef72 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -676,9 +676,10 @@ void pa_sink_unlink(pa_sink* s) {
      * reversing pa_sink_put(). It also undoes the registrations
      * already done in pa_sink_new()! */
 
-    /* All operations here shall be idempotent, i.e. pa_sink_unlink()
-     * may be called multiple times on the same sink without bad
-     * effects. */
+    if (s->unlink_requested)
+        return;
+
+    s->unlink_requested = true;
 
     linked = PA_SINK_IS_LINKED(s->state);
 
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index 5df109e..b64a666 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -63,6 +63,12 @@ struct pa_sink {
     pa_core *core;
 
     pa_sink_state_t state;
+
+    /* Set in the beginning of pa_sink_unlink() before setting the sink state
+     * to UNLINKED. The purpose is to prevent moving streams to a sink that is
+     * about to be removed. */
+    bool unlink_requested;
+
     pa_sink_flags_t flags;
     pa_suspend_cause_t suspend_cause;
 
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 9217ad4..6d54ae8 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -1187,6 +1187,9 @@ bool pa_source_output_may_move_to(pa_source_output *o, pa_source *dest) {
     if (dest == o->source)
         return true;
 
+    if (dest->unlink_requested)
+        return false;
+
     if (!pa_source_output_may_move(o))
         return false;
 
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index f4b96ab..98374ae 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -618,6 +618,11 @@ void pa_source_unlink(pa_source *s) {
     /* See pa_sink_unlink() for a couple of comments how this function
      * works. */
 
+    if (s->unlink_requested)
+        return;
+
+    s->unlink_requested = true;
+
     linked = PA_SOURCE_IS_LINKED(s->state);
 
     if (linked)
diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h
index 19fb41b..91e8674 100644
--- a/src/pulsecore/source.h
+++ b/src/pulsecore/source.h
@@ -64,6 +64,12 @@ struct pa_source {
     pa_core *core;
 
     pa_source_state_t state;
+
+    /* Set in the beginning of pa_source_unlink() before setting the source
+     * state to UNLINKED. The purpose is to prevent moving streams to a source
+     * that is about to be removed. */
+    bool unlink_requested;
+
     pa_source_flags_t flags;
     pa_suspend_cause_t suspend_cause;
 

commit 0b6e4694a5a421cfc9eff0c0d259e46dc007f6b0
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Thu Mar 10 19:18:48 2016 +0200

    echo-cancel: rework move handling
    
    When autoloaded, module-echo-cancel doesn't support moving the sink
    input and source output that it creates, but the move prevention was
    implemented by manually requesting module unloading in the middle of
    the stream move procedure, rather than by just setting the DONT_MOVE
    flags. This patch removes the module unloading code from the moving()
    callbacks and adds the DONT_MOVE flags. In addition to saving some
    code, this also prevents problems related to trying to move streams
    connected to the echo cancel sink or source while the echo cancel sink
    or source is in the middle of a move too (a crash will happen in such
    situation, as demonstrated in
    https://bugs.freedesktop.org/show_bug.cgi?id=93443).

diff --git a/src/modules/echo-cancel/module-echo-cancel.c b/src/modules/echo-cancel/module-echo-cancel.c
index 56adf8f..c873648 100644
--- a/src/modules/echo-cancel/module-echo-cancel.c
+++ b/src/modules/echo-cancel/module-echo-cancel.c
@@ -206,7 +206,6 @@ struct userdata {
     pa_core *core;
     pa_module *module;
 
-    bool autoloaded;
     bool dead;
     bool save_aec;
 
@@ -1436,12 +1435,6 @@ static void source_output_moving_cb(pa_source_output *o, pa_source *dest) {
     pa_assert_ctl_context();
     pa_assert_se(u = o->userdata);
 
-    if (u->autoloaded) {
-        /* We were autoloaded, and don't support moving. Let's unload ourselves. */
-        pa_log_debug("Can't move autoloaded streams, unloading");
-        pa_module_unload_request(u->module, true);
-    }
-
     if (dest) {
         pa_source_set_asyncmsgq(u->source, dest->asyncmsgq);
         pa_source_update_flags(u->source, PA_SOURCE_LATENCY|PA_SOURCE_DYNAMIC_LATENCY, dest->flags);
@@ -1473,12 +1466,6 @@ static void sink_input_moving_cb(pa_sink_input *i, pa_sink *dest) {
     pa_sink_input_assert_ref(i);
     pa_assert_se(u = i->userdata);
 
-    if (u->autoloaded) {
-        /* We were autoloaded, and don't support moving. Let's unload ourselves. */
-        pa_log_debug("Can't move autoloaded streams, unloading");
-        pa_module_unload_request(u->module, true);
-    }
-
     if (dest) {
         pa_sink_set_asyncmsgq(u->sink, dest->asyncmsgq);
         pa_sink_update_flags(u->sink, PA_SINK_LATENCY|PA_SINK_DYNAMIC_LATENCY, dest->flags);
@@ -1657,6 +1644,7 @@ int pa__init(pa_module*m) {
     pa_modargs *ma;
     pa_source *source_master=NULL;
     pa_sink *sink_master=NULL;
+    bool autoloaded;
     pa_source_output_new_data source_output_data;
     pa_sink_input_new_data sink_input_data;
     pa_source_new_data source_data;
@@ -1759,8 +1747,8 @@ int pa__init(pa_module*m) {
         goto fail;
     }
 
-    u->autoloaded = DEFAULT_AUTOLOADED;
-    if (pa_modargs_get_value_boolean(ma, "autoloaded", &u->autoloaded) < 0) {
+    autoloaded = DEFAULT_AUTOLOADED;
+    if (pa_modargs_get_value_boolean(ma, "autoloaded", &autoloaded) < 0) {
         pa_log("Failed to parse autoloaded value");
         goto fail;
     }
@@ -1805,7 +1793,7 @@ int pa__init(pa_module*m) {
     pa_source_new_data_set_channel_map(&source_data, &source_map);
     pa_proplist_sets(source_data.proplist, PA_PROP_DEVICE_MASTER_DEVICE, source_master->name);
     pa_proplist_sets(source_data.proplist, PA_PROP_DEVICE_CLASS, "filter");
-    if (!u->autoloaded)
+    if (!autoloaded)
         pa_proplist_sets(source_data.proplist, PA_PROP_DEVICE_INTENDED_ROLES, "phone");
 
     if (pa_modargs_get_proplist(ma, "source_properties", source_data.proplist, PA_UPDATE_REPLACE) < 0) {
@@ -1855,7 +1843,7 @@ int pa__init(pa_module*m) {
     pa_sink_new_data_set_channel_map(&sink_data, &sink_map);
     pa_proplist_sets(sink_data.proplist, PA_PROP_DEVICE_MASTER_DEVICE, sink_master->name);
     pa_proplist_sets(sink_data.proplist, PA_PROP_DEVICE_CLASS, "filter");
-    if (!u->autoloaded)
+    if (!autoloaded)
         pa_proplist_sets(sink_data.proplist, PA_PROP_DEVICE_INTENDED_ROLES, "phone");
 
     if (pa_modargs_get_proplist(ma, "sink_properties", sink_data.proplist, PA_UPDATE_REPLACE) < 0) {
@@ -1907,6 +1895,9 @@ int pa__init(pa_module*m) {
     pa_source_output_new_data_set_sample_spec(&source_output_data, &source_output_ss);
     pa_source_output_new_data_set_channel_map(&source_output_data, &source_output_map);
 
+    if (autoloaded)
+        source_output_data.flags |= PA_SOURCE_OUTPUT_DONT_MOVE;
+
     pa_source_output_new(&u->source_output, m->core, &source_output_data);
     pa_source_output_new_data_done(&source_output_data);
 
@@ -1942,6 +1933,9 @@ int pa__init(pa_module*m) {
     pa_sink_input_new_data_set_channel_map(&sink_input_data, &sink_map);
     sink_input_data.flags = PA_SINK_INPUT_VARIABLE_RATE;
 
+    if (autoloaded)
+        sink_input_data.flags |= PA_SINK_INPUT_DONT_MOVE;
+
     pa_sink_input_new(&u->sink_input, m->core, &sink_input_data);
     pa_sink_input_new_data_done(&sink_input_data);
 



More information about the pulseaudio-commits mailing list