On 19.04.2017 19:46, Georg Chini wrote: > On 19.04.2017 11:03, KimJeongYeon wrote: >> 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> > > We are nearly there. Still some comments ... > >> --- >> src/modules/module-filter-apply.c | 107 >> +++++++++++++++++++++++++++++++------- >> 1 file changed, 89 insertions(+), 18 deletions(-) >> >> diff --git a/src/modules/module-filter-apply.c >> b/src/modules/module-filter-apply.c >> index e91fb37..562221a 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_APPLY_SET_BY_MFA "filter.apply.set_by_mfa" >> #define PA_PROP_MDM_AUTO_FILTERED >> "module-device-manager.auto_filtered" >> PA_MODULE_AUTHOR("Colin Guthrie"); >> @@ -161,6 +162,53 @@ static const char* >> get_filter_parameters(pa_object *o, const char *want, bool is >> return parameters; >> } > > This function is very difficult to read. I'd rename "is_set" to > "set_properties" and also > add another boolean "set_by_mfa" so that you set/unset all filter > properties within > this function. More changes below. > >> +static void set_filter_properties(pa_proplist *pl, struct filter* >> filter, bool is_set) { One more point, it should be "struct filter *filter" instead of "struct filter* filter". I have noticed that there is some inconsistency about the "*" throughout module-filter-apply. Maybe you should put that also right. (If you want to do so, please in a separate patch.)
