On 18.04.2017 14:36, 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> Hi JeongYeon, sorry, but I still don't agree with your patch. As already said you do not need the enumeration, a simple boolean should do. Also skip_prop_change seems unnecessary. I would name the new property filter.apply.mfa_loaded because the other properties have a filter.apply prefix (although I suggested the other name myself). I think the following logic is much simpler than your approach and still covers all cases (I always wrote sink but naturally the same applies for the source). Please correct me if I have overlooked anything. static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_input) { ... leave the first bit unchanged ... /* Check if filter.apply property is set and get filter name. */ if ((want = get_filter_name(o, is_sink_input))) { /* We end up here, when filter.apply is set */ /* If module is not set, we don't know which type of sink we are on, * therefore do not try to do anything. (Not sure if this is right. The * filter sinks should always have module set, so we could conclude * that unset means that we are not on a filter sink and can just skip * the next check. But let's leave it like it is for the moment.) */ if (!module) goto done; /* If we are already on the right filter sink, we don't need to do anything */ module_name = pa_sprintf_malloc("module-%s", want); if (pa_streq(module->name, module_name)) { pa_log_debug("Stream appears to be playing on an appropriate sink already. Ignoring."); goto done; } /* Now filter apply is set and the sink is not the right filter sink. * We have two possible cases left: */ /* 1) The stream has filter.apply.mfa_loaded set. This means the stream has been * manually moved away from the filter. */ if (property filter.apply.mfa_loaded) { Remove filter.apply Remove filter.apply.mfa_loaded keep sink as is } else { /* 2) The normal case, filter.apply is set but filter.apply.mfa_loaded not, * so the sink input should be moved to a filter sink */ ... don't change this part, all should be fine ... } else { /* We reach this when filter.apply is unset. Three possible cases here */ /* 1) filter apply has been manually unset */ if (sink is filter sink && called from property change hook) { move sink_input to master sink remove filter.apply.mfa_loaded property } /* 2) Stream has been manually moved to a filter sink without filter_apply set */ if (sink is filter sink && not called from property change hook) { leave sink as it is set filter.apply set filter.apply.mfa_loaded property } /* 3) Sink is not a filter sink. Nothing to do. */ } ... }
