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
