[pulseaudio-discuss] [PATCH v4] filter-apply: Fixed a stream moves to wrong sink or source.

KimJeongYeon see2002 at gmail.com
Tue Apr 18 12:36:12 UTC 2017


For example, a normal stream tried to attach to filter sink or source, which
filter loaded and managed by filter-apply. But, the stream goes to filter's
***master sink or source*** due to unexpected restoring operation.
It should attached to filter sink or source properly.

Also, includes further fix according to Georg's comment, [1]
  If a stream that had filter.apply set initially is moved to another sink/source, then the filter
  should be applied again (a new filter, since the master sink/source has changed).

  If a stream that did not have filter.apply set initially is moved away, the property should
  be removed and no new filter applied.

  Also, a property list change might add or remove the filter.apply property. If it is added,
  we want that the filter is applied. Your patch does nothing and assumes that the stream
  is already filtered, even if the stream is not on a filter sink.

[1] https://lists.freedesktop.org/archives/pulseaudio-discuss/2017-April/027980.html

Signed-off-by: KimJeongYeon <jeongyeon.kim at samsung.com>
---
 src/modules/module-filter-apply.c | 165 +++++++++++++++++++++++++++++++++++---
 1 file changed, 152 insertions(+), 13 deletions(-)

diff --git a/src/modules/module-filter-apply.c b/src/modules/module-filter-apply.c
index e91fb37..fb93237 100644
--- a/src/modules/module-filter-apply.c
+++ b/src/modules/module-filter-apply.c
@@ -39,6 +39,7 @@
 
 #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_MFA_ADDED        "filter.mfa_added"
 #define PA_PROP_MDM_AUTO_FILTERED       "module-device-manager.auto_filtered"
 
 PA_MODULE_AUTHOR("Colin Guthrie");
@@ -55,6 +56,12 @@ static const char* const valid_modargs[] = {
 #define DEFAULT_AUTOCLEAN true
 #define HOUSEKEEPING_INTERVAL (10 * PA_USEC_PER_SEC)
 
+typedef enum _process_cmd_type {
+    PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_PUT,
+    PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_MOVE_FINISH,
+    PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_PROPLIST,
+} process_cmd_type_t;
+
 struct filter {
     char *name;
     char *parameters;
@@ -74,6 +81,7 @@ struct userdata {
     pa_hashmap *mdm_ignored_inputs, *mdm_ignored_outputs;
     bool autoclean;
     pa_time_event *housekeeping_time_event;
+    bool skip_prop_change;
 };
 
 static unsigned filter_hash(const void *p) {
@@ -161,6 +169,29 @@ static const char* get_filter_parameters(pa_object *o, const char *want, bool is
     return parameters;
 }
 
+static void set_filter_properties(pa_proplist *pl, struct filter* filter, bool is_set) {
+    const char *apply;
+    char *prop_parameters;
+
+    if (is_set) {
+        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);
+        }
+    } else {
+        if ((apply = pa_proplist_gets(pl, PA_PROP_FILTER_APPLY))) {
+            prop_parameters = pa_sprintf_malloc(PA_PROP_FILTER_APPLY_PARAMETERS, apply);
+            pa_proplist_unset(pl, prop_parameters);
+            pa_xfree(prop_parameters);
+            pa_proplist_unset(pl, PA_PROP_FILTER_APPLY);
+        }
+    }
+}
+
 static bool should_group_filter(struct filter *filter) {
     return pa_streq(filter->name, "echo-cancel");
 }
@@ -431,7 +462,106 @@ 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 bool make_sure_filter_apply(struct userdata *u, pa_object *o, process_cmd_type_t cmd, bool is_sink_input) {
+    pa_sink *sink = NULL;
+    pa_source *source = NULL;
+    pa_proplist *pl;
+    void *state;
+    struct filter *filter;
+    bool is_attached = false;
+
+    if (is_sink_input) {
+        sink = PA_SINK_INPUT(o)->sink;
+        pl = PA_SINK_INPUT(o)->proplist;
+    } else {
+        source = PA_SOURCE_OUTPUT(o)->source;
+        pl = PA_SOURCE_OUTPUT(o)->proplist;
+    }
+
+    /* Make sure apply filter to stream that puts or moves to filter sink or
+     * source according to request. Marking at 'filter.mfa_added' if stream
+     * has not 'filter.apply' property. Then, apply filter to stream.
+     * No action required if stream has 'filter.apply' property. */
+    PA_HASHMAP_FOREACH(filter, u->filters, state) {
+        if ((is_sink_input && sink == filter->sink) || (!is_sink_input && source == filter->source)) {
+            is_attached = true;
+
+            switch (cmd) {
+                case PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_PUT:
+                case PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_MOVE_FINISH:
+                    if (!pa_proplist_gets(pl, PA_PROP_FILTER_APPLY)) {
+                        /* Assumes that normal stream goes to filter. Marking
+                         * at 'filter.mfa_added' that means temporarily
+                         * associates with filter-apply. */
+                        pa_proplist_sets(pl, PA_PROP_FILTER_MFA_ADDED, "1");
+                        set_filter_properties(pl, filter, true);
+                        goto do_housekeeping_only;
+                    }
+                    /* Else, apply filter on this sink or source. */
+                    break;
+                case PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_PROPLIST:
+                    if (pa_proplist_gets(pl, PA_PROP_FILTER_APPLY) || u->skip_prop_change) {
+                        /* Assumes that stream has 'filter.apply' already.
+                         * Other properties seems be modified. */
+                        goto do_nothing;
+                    } else {
+                        /* Assumes that 'filter.apply' removed manually.
+                         * Restore to master sink or source. */
+                        pa_proplist_unset(pl, PA_PROP_FILTER_MFA_ADDED);
+                        set_filter_properties(pl, NULL, false);
+                    }
+                    break;
+            }
+            break;
+        }
+    }
+
+    /* Make sure remove or apply filter to stream that puts or moves to normal
+     * sink or source according to request. Remove 'filter.mfa_added' property
+     * if stream has it. Then, do not apply  filter to stream anymore.
+     * No action required if stream has not 'filter.apply' property. */
+    if (!is_attached) {
+        switch (cmd) {
+            case PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_PUT:
+            case PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_MOVE_FINISH:
+                if (!pa_proplist_gets(pl, PA_PROP_FILTER_APPLY)) {
+                    /* Assumes that stream is not related with filter-apply. */
+                    goto do_nothing;
+                } else if (pa_proplist_gets(pl, PA_PROP_FILTER_MFA_ADDED)) {
+                    /* Assumes that normal stream goes away from filter.
+                     * It goes to normal sink/source and drop associates with
+                     * filter-apply. */
+                    pa_proplist_unset(pl, PA_PROP_FILTER_MFA_ADDED);
+                    set_filter_properties(pl, NULL, false);
+                    goto do_housekeeping_only;
+                }
+                /* Else, apply filter on this sink or source. */
+                break;
+            case PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_PROPLIST:
+                if (!pa_proplist_gets(pl, PA_PROP_FILTER_APPLY) || u->skip_prop_change) {
+                    /* Assumes that stream changed properties but they are not
+                     * related with filter-apply. */
+                    goto do_nothing;
+                } else {
+                    /* Assumes that 'filter.apply' added manually.
+                     * Apply filter on this sink or source. */
+                    pa_proplist_unset(pl, PA_PROP_FILTER_MFA_ADDED);
+                }
+                break;
+        }
+    }
+
+    return true;
+
+do_housekeeping_only:
+    if (pa_hashmap_size(u->filters) > 0)
+        trigger_housekeeping(u);
+
+do_nothing:
+    return false;
+}
+
+static pa_hook_result_t process(struct userdata *u, pa_object *o, process_cmd_type_t cmd, bool is_sink_input) {
     const char *want;
     const char *parameters;
     bool done_something = false;
@@ -442,14 +572,10 @@ static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_i
     struct filter *fltr = NULL, *filter = NULL;
 
     if (is_sink_input) {
-        sink = PA_SINK_INPUT(o)->sink;
-
-        if (sink)
+        if ((sink = PA_SINK_INPUT(o)->sink))
             module = sink->module;
     } else {
-        source = PA_SOURCE_OUTPUT(o)->source;
-
-        if (source)
+        if ((source = PA_SOURCE_OUTPUT(o)->source))
             module = source->module;
     }
 
@@ -457,6 +583,10 @@ static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_i
     if ((is_sink_input && !sink) || (!is_sink_input && !source))
         goto done;
 
+    /* Before processing hook events, make sure who apply filter or not. */
+    if (!make_sure_filter_apply(u, o, cmd, is_sink_input))
+        goto done;
+
     /* If the stream doesn't what any filter, then let it be. */
     if ((want = get_filter_name(o, is_sink_input))) {
         /* We need to ensure the SI is playing on a sink of this type
@@ -542,7 +672,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), PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_PUT, true);
 }
 
 static pa_hook_result_t sink_input_move_finish_cb(pa_core *core, pa_sink_input *i, struct userdata *u) {
@@ -552,17 +682,19 @@ 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;
 
+    u->skip_prop_change = true;
     /* If we're managing m-d-m.auto_filtered on this, remove and re-add if we're continuing to manage it */
     pa_hashmap_remove(u->mdm_ignored_inputs, i);
+    u->skip_prop_change = false;
 
-    return process(u, PA_OBJECT(i), true);
+    return process(u, PA_OBJECT(i), PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_MOVE_FINISH, true);
 }
 
 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), PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_PROPLIST, true);
 }
 
 static pa_hook_result_t sink_input_unlink_cb(pa_core *core, pa_sink_input *i, struct userdata *u) {
@@ -574,7 +706,9 @@ static pa_hook_result_t sink_input_unlink_cb(pa_core *core, pa_sink_input *i, st
     if (pa_hashmap_size(u->filters) > 0)
         trigger_housekeeping(u);
 
+    u->skip_prop_change = true;
     pa_hashmap_remove(u->mdm_ignored_inputs, i);
+    u->skip_prop_change = false;
 
     return PA_HOOK_OK;
 }
@@ -619,7 +753,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), PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_PUT, false);
 }
 
 static pa_hook_result_t source_output_move_finish_cb(pa_core *core, pa_source_output *o, struct userdata *u) {
@@ -629,17 +763,19 @@ 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;
 
+    u->skip_prop_change = true;
     /* If we're managing m-d-m.auto_filtered on this, remove and re-add if we're continuing to manage it */
     pa_hashmap_remove(u->mdm_ignored_outputs, o);
+    u->skip_prop_change = false;
 
-    return process(u, PA_OBJECT(o), false);
+    return process(u, PA_OBJECT(o), PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_MOVE_FINISH, 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), PROCESS_CMD_SINK_INPUT_SOURCE_OUTPUT_PROPLIST, false);
 }
 
 static pa_hook_result_t source_output_unlink_cb(pa_core *core, pa_source_output *o, struct userdata *u) {
@@ -651,7 +787,9 @@ static pa_hook_result_t source_output_unlink_cb(pa_core *core, pa_source_output
     if (pa_hashmap_size(u->filters) > 0)
         trigger_housekeeping(u);
 
+    u->skip_prop_change = true;
     pa_hashmap_remove(u->mdm_ignored_outputs, o);
+    u->skip_prop_change = false;
 
     return PA_HOOK_OK;
 }
@@ -724,6 +862,7 @@ int pa__init(pa_module *m) {
     }
 
     u->filters = pa_hashmap_new(filter_hash, filter_compare);
+    u->skip_prop_change = false;
     u->mdm_ignored_inputs = pa_hashmap_new_full(NULL, NULL, (pa_free_cb_t) unset_mdm_ignore_input, NULL);
     u->mdm_ignored_outputs = pa_hashmap_new_full(NULL, NULL, (pa_free_cb_t) unset_mdm_ignore_output, NULL);
 
-- 
2.7.4



More information about the pulseaudio-discuss mailing list