2017-04-19 17:58 GMT+09:00 <pulseaudio-discuss-request at lists.freedesktop.org>:
>> Hi JeongYeon,
>>
>> sorry, but I still don't agree with your patch. As already
>> said you do not need the
>> enumeration, a simple boolean
>>
>>
>> Ok. I'll do.
Few hours ago, I did submit patch v5 with this fix.
>>
>> should do. Also skip_prop_change seems unnecessary.
>>
>>
>> About 'skip_prop_change':
>> Move operation happens twice when call 'move_objects_for_filter'.
>> Because, unexpected proplist-hook comes at 'do_move'.
>> But, it doesn't harm moving operation.
>
> That's what I mean. I know there will be an unnecessary call to
> process() due to
> the property list change, but it should not have any effect and so there
> is no need
> to prevent it. Actually the move operation should not happen twice,
> instead you
> should see the "Stream appears to be playing on an appropriate sink
> already. Ignoring."
> message. Or am I wrong there?
No, It does move operation twice actually.
But, I think it is not related with this patch. Might be improved next patchset.
>
> Apart from PA_PROP_FILTER_APPLY_MOVING there are also the other changes
> to the
> property list when we set/remove the filter.apply and
> filter.apply.set_by_mfa properties.
> They also should have no effect, but better test it out. If there are
> too many superfluous
> messages in the log, I am not completely against using skip_prop_changes
> to suppress
> them. In this case you would only need to set the variable before the
> property list
> operation and it can be checked and reset early in process().
In my short investment, below 'pa_sink_input_set_property' triggers
nested proplist-hooking. Therefore, 'process' called again then, tries
to move again.
Fortunately, not happens recursively.
code:
static int do_move(struct userdata *u, pa_object *obj, pa_object
*parent, bool is_input) {
...
pa_sink_input_set_property(PA_SINK_INPUT(obj),
PA_PROP_MDM_AUTO_FILTERED, "1");
return pa_sink_input_move_to(PA_SINK_INPUT(obj),
PA_SINK(parent), false);
...
}
I think it is able to eliminate redundant moving by check
PA_PROP_FILTER_APPLY_MOVING at 'sink_input_proplist_cb'.
No more need additional 'skip_prop_change' flag.
code:
if (pa_proplist_gets(i->proplist, PA_PROP_FILTER_APPLY_MOVING))
return PA_HOOK_OK;
How do you think? Are you agree that would be fix at new patch?
Thanks for reviewing and help.
Regards,
KimJeongYeon
