[pulseaudio-discuss] [PATCH v4] filter-apply: Fixed a stream moves to wrong sink or source.

[KimJeongYeon]

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