On 11.04.2017 15:48, KimJeongYeon wrote:
> 2017-04-10 22:59 GMT+09:00 Georg Chini <georg at chini.tk>:
>
>> Thinking again, it looks that my suggestion also does not cover all cases.
>> When the
>> stream is moved away from the filter sink, it will have filter.apply set
>> which it did not
>> have initially. This is obviously wrong, so we need an additional property
>> to indicate
>> that filter.apply was set by module-filter-apply and that it can be removed
>> when the
>> stream moves away from the filter. Also I have only been talking about the
>> put hook.
>> The same applies to the move_finished hook.
>>
>> But you have been working with the code for a while now, so maybe you can
>> come
>> up with a better solution.
> As you reviewed, there were not yet regarded scenarios.
> I submit a patch v3 few minutes ago with covering all scenarios. (e.g
> put / move / proplist change / unlink hooks) But, it become more
> complicated.
> Anyway, it does work well in my desktop environment.
>
>> Also Arun might object to my idea. He asked me to summarize and wanted to
>> reply
>> on the list. So maybe you wait until he confirmed/rejected.
>>
> Sure.
>
> Regards,
> KimJeongYeon
Hi JeongYeon,
sorry, but it still does not look right to me. Now you always remove the
filter.apply
property when the stream is moved away. This should only be done for streams
that have been moved to the filter sink/source and did not have
filter.apply set initially.
That's why I said you need an additional flag to mark those streams
where the property
has been set by module-filter-apply.
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.
So the overall logic is as follows (assuming that filter.mfa_added is
the flag that indicates
whether module-filter-apply has set the filter.apply property):
1) If a stream with filter.apply set and filter.mfa_added unset is
moved/put to a non-filter
sink, the filter should be applied. This is also true, if the stream is
moved away from
another filter sink. From the stream perspective it just means that the
master sink has
changed.
2) If a stream with filter.apply unset is moved/put to a filter sink,
module-filter-apply should
set the filter.apply and the filter.mfa_added properties. Because the
stream is already at the
right sink then, nothing else needs to be done.
3) If a stream with both, filter.apply set and filter.mfa_added set, is
moved to a non-filter sink,
the two properties should be removed. The stream is already at the right
sink then, so
nothing else needs to be done.
4) If the filter.apply property is added/removed manually and so causes
a proplist_change
event, the filter should be added/removed. We can always unset
filter.mfa_added in that
situation because then the user chose what is needed. All other proplist
changes can be
safely ignored (meaning that if the stream is already filtered and
filter.apply is set or the
stream is not filtered and filter.apply is unset we don't need to do
anything).
I hope this is clarifies the approach. If you are unsure or do not
understand what I mean,
please contact me on IRC and we can discuss it further.
Regards
Georg
