filter-apply: Do not re-route stream to master sink/source when manually moved to...

Currently, if a stream is manually moved to a filter sink or source managed by
module-filter-apply, the stream will be silently re-routed to the master sink
or source, because the filter.apply property is not set on that stream. We can
assume, that the users intention however was to have the stream filtered.

Therefore this patch changes the logic, so that the stream will not be moved
to the master but remains on the filter sink or source. To handle the change
of a property correctly, the filter.apply property must be set temporarily.
An additional property filter.apply.set_by_mfa was introduced to mark those
streams, so that filter.apply can be removed again when the stream moves away
from the filter.

This patch based on:
https://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?h=next&id=b8858ff24d03dcd0828db529d455e3b2d2493b98

Signed-off-by: KimJeongYeon <jeongyeon.kim@samsung.com>
Change-Id: Id9072d92038f3806ff39f9d539c3229cb358afbf

diff --git a/src/modules/module-filter-apply.c b/src/modules/module-filter-apply.c
index ce26d14..a0523a6 100644 (file)
--- a/src/modules/module-filter-apply.c
+++ b/src/modules/module-filter-apply.c
@@ -42,6 +42,7 @@
 #define PA_PROP_FILTER_APPLY_GROUP      PA_PROP_FILTER_APPLY".%s.group"
 #define PA_PROP_FILTER_APPLY_PARAMETERS PA_PROP_FILTER_APPLY".%s.parameters"
 #define PA_PROP_FILTER_APPLY_MOVING     "filter.apply.moving"
+#define PA_PROP_FILTER_APPLY_SET_BY_MFA "filter.apply.set_by_mfa"
 
 PA_MODULE_AUTHOR("Colin Guthrie");
 PA_MODULE_DESCRIPTION("Load filter sinks automatically when needed");
@@ -190,6 +191,67 @@ static const char* get_filter_group(pa_object *o, const char *want, bool is_sink
     return group;
 }
 
+/* This function is used to set or unset the filter related stream properties. This is necessary
+ * if a stream does not have filter.apply set and is manually moved to a filter sink or source.
+ * In this case, the properies must be temporarily set and removed when the stream is moved away
+ * from the filter. */
+static void set_filter_properties(pa_proplist *pl, struct filter *filter, bool set_properties) {
+    char *prop_parameters;
+
+    if (set_properties) {
+        pa_assert(filter);
+
+        pa_proplist_sets(pl, PA_PROP_FILTER_APPLY, filter->name);
+
+        if (filter->parameters) {
+            prop_parameters = pa_sprintf_malloc(PA_PROP_FILTER_APPLY_PARAMETERS, filter->name);
+            pa_proplist_sets(pl, prop_parameters, filter->parameters);
+            pa_xfree(prop_parameters);
+        }
+
+        pa_proplist_sets(pl, PA_PROP_FILTER_APPLY_SET_BY_MFA, "1");
+
+    } else {
+        const char *old_filter_name = NULL;
+
+        if (filter)
+            old_filter_name = filter->name;
+        else
+            old_filter_name = pa_proplist_gets(pl, PA_PROP_FILTER_APPLY);
+
+        /* If the filter name cannot be determined, properties cannot be removed. */
+        if (!old_filter_name)
+            return;
+
+        prop_parameters = pa_sprintf_malloc(PA_PROP_FILTER_APPLY_PARAMETERS, old_filter_name);
+        pa_proplist_unset(pl, prop_parameters);
+        pa_xfree(prop_parameters);
+
+        pa_proplist_unset(pl, PA_PROP_FILTER_APPLY);
+        pa_proplist_unset(pl, PA_PROP_FILTER_APPLY_SET_BY_MFA);
+    }
+}
+
+static struct filter* get_filter_for_object(struct userdata *u, pa_object *o, bool is_sink_input) {
+    pa_sink *sink = NULL;
+    pa_source *source = NULL;
+    struct filter *filter = NULL;
+    void *state;
+
+    if (is_sink_input)
+        sink = PA_SINK_INPUT(o)->sink;
+    else
+        source = PA_SOURCE_OUTPUT(o)->source;
+
+    PA_HASHMAP_FOREACH(filter, u->filters, state) {
+        if ((is_sink_input && sink == filter->sink) || (!is_sink_input && source == filter->source)) {
+            return filter;
+        }
+    }
+
+    return NULL;
+}
+
 static bool should_group_filter(struct filter *filter) {
     return pa_streq(filter->name, "echo-cancel");
 }
@@ -333,7 +395,7 @@ static int do_move(pa_object *obj, pa_object *parent, bool restore, bool is_inpu
         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) {
+static void move_object_for_filter(pa_object *o, struct filter *filter, bool restore, bool is_sink_input) {
     pa_object *parent;
     pa_proplist *pl;
     const char *name;
@@ -367,7 +429,7 @@ static void move_object_for_filter(pa_object *o, struct filter* filter, bool res
     pa_proplist_unset(pl, PA_PROP_FILTER_APPLY_MOVING);
 }
 
-static void move_objects_for_filter(struct userdata *u, pa_object *o, struct filter* filter, bool restore,
+static void move_objects_for_filter(struct userdata *u, pa_object *o, struct filter *filter, bool restore,
         bool is_sink_input) {
 
     if (!should_group_filter(filter))
@@ -455,7 +517,7 @@ static bool can_unload_module(struct userdata *u, uint32_t idx) {
     return true;
 }
 
-static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_input) {
+static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_input, bool is_property_change) {
     const char *want;
     const char *group;
     const char *parameters;
@@ -463,24 +525,23 @@ static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_i
     pa_sink *sink = NULL;
     pa_source *source = NULL;
     pa_module *module = NULL;
+    pa_proplist *pl;
 
     if (is_sink_input) {
-        sink = PA_SINK_INPUT(o)->sink;
-
-        if (sink)
+        if ((sink = PA_SINK_INPUT(o)->sink))
             module = sink->module;
+        pl = PA_SINK_INPUT(o)->proplist;
     } else {
-        source = PA_SOURCE_OUTPUT(o)->source;
-
-        if (source)
+        if ((source = PA_SOURCE_OUTPUT(o)->source))
             module = source->module;
+        pl = PA_SOURCE_OUTPUT(o)->proplist;
     }
 
     /* If there is no sink/source yet, we can't do much */
     if ((is_sink_input && !sink) || (!is_sink_input && !source))
         return PA_HOOK_OK;
 
-    /* If the stream doesn't what any filter, then let it be. */
+    /* If the stream doesn't want any filter, then let it be. */
     if ((want = get_filter_name(o, is_sink_input))) {
         char *module_name;
         struct filter *fltr, *filter;
@@ -498,6 +559,24 @@ static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_i
             return PA_HOOK_OK;
         }
 
+        /* If the stream originally did not have the filter.apply property set and is
+         * manually moved away from the filter, remove the filter properties from the
+         * stream */
+        if (pa_proplist_gets(pl, PA_PROP_FILTER_APPLY_SET_BY_MFA)) {
+
+            set_filter_properties(pl, NULL, false);
+
+            /* If the new sink/source is also a filter, the stream has been moved from
+             * one filter to another, so add the properties for the new filter. */
+            if ((filter = get_filter_for_object(u, o, is_sink_input)))
+                set_filter_properties(pl, filter, true);
+
+            done_something = true;
+            goto done;
+        }
+
+        /* The stream needs be moved to a filter. */
+
         /* Some applications may want group of filters. (optional) */
         group = get_filter_group(o, want, is_sink_input);
 
@@ -551,20 +630,27 @@ static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_i
             done_something = true;
         }
     } else {
-        void *state;
         struct filter *filter = NULL;
 
-        /* We do not want to filter... but are we already filtered?
-         * This can happen if an input's proplist changes */
-        PA_HASHMAP_FOREACH(filter, u->filters, state) {
-            if ((is_sink_input && sink == filter->sink) || (!is_sink_input && source == filter->source)) {
+        /* The filter.apply property is not set. If the stream is nevertheless using a
+         * filter sink/source, it either has been moved to the filter manually or the
+         * user just removed the filter.apply property. */
+
+        if ((filter = get_filter_for_object(u, o, is_sink_input))) {
+            if (is_property_change) {
+                /* 'filter.apply' has been manually unset. Do restore. */
                 move_objects_for_filter(u, o, filter, true, is_sink_input);
+                set_filter_properties(pl, filter, false);
                 done_something = true;
-                break;
+            } else {
+                /* Stream has been manually moved to a filter sink/source
+                 * without 'filter.apply' set. Leave sink as it is. */
+                set_filter_properties(pl, filter, true);
             }
         }
     }
 
+done:
     if (done_something)
         trigger_housekeeping(u);
 
@@ -575,7 +661,7 @@ static pa_hook_result_t sink_input_put_cb(pa_core *core, pa_sink_input *i, struc
     pa_core_assert_ref(core);
     pa_sink_input_assert_ref(i);
 
-    return process(u, PA_OBJECT(i), true);
+    return process(u, PA_OBJECT(i), true, false);
 }
 
 static pa_hook_result_t sink_input_move_finish_cb(pa_core *core, pa_sink_input *i, struct userdata *u) {
@@ -585,14 +671,14 @@ static pa_hook_result_t sink_input_move_finish_cb(pa_core *core, pa_sink_input *
     if (pa_proplist_gets(i->proplist, PA_PROP_FILTER_APPLY_MOVING))
         return PA_HOOK_OK;
 
-    return process(u, PA_OBJECT(i), true);
+    return process(u, PA_OBJECT(i), true, false);
 }
 
 static pa_hook_result_t sink_input_proplist_cb(pa_core *core, pa_sink_input *i, struct userdata *u) {
     pa_core_assert_ref(core);
     pa_sink_input_assert_ref(i);
 
-    return process(u, PA_OBJECT(i), true);
+    return process(u, PA_OBJECT(i), true, true);
 }
 
 static pa_hook_result_t sink_input_unlink_cb(pa_core *core, pa_sink_input *i, struct userdata *u) {
@@ -647,7 +733,7 @@ static pa_hook_result_t source_output_put_cb(pa_core *core, pa_source_output *o,
     pa_core_assert_ref(core);
     pa_source_output_assert_ref(o);
 
-    return process(u, PA_OBJECT(o), false);
+    return process(u, PA_OBJECT(o), false, false);
 }
 
 static pa_hook_result_t source_output_move_finish_cb(pa_core *core, pa_source_output *o, struct userdata *u) {
@@ -657,14 +743,14 @@ static pa_hook_result_t source_output_move_finish_cb(pa_core *core, pa_source_ou
     if (pa_proplist_gets(o->proplist, PA_PROP_FILTER_APPLY_MOVING))
         return PA_HOOK_OK;
 
-    return process(u, PA_OBJECT(o), false);
+    return process(u, PA_OBJECT(o), false, false);
 }
 
 static pa_hook_result_t source_output_proplist_cb(pa_core *core, pa_source_output *o, struct userdata *u) {
     pa_core_assert_ref(core);
     pa_source_output_assert_ref(o);
 
-    return process(u, PA_OBJECT(o), false);
+    return process(u, PA_OBJECT(o), false, true);
 }
 
 static pa_hook_result_t source_output_unlink_cb(pa_core *core, pa_source_output *o, struct userdata *u) {