<div dir="auto"><div><br><div class="gmail_extra"><br><div class="gmail_quote">2017. 4. 19. 오전 5:10에 "Georg Chini" <<a href="mailto:georg@chini.tk">georg@chini.tk</a>>님이 작성:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text">On 18.04.2017 21:45, Georg Chini wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 18.04.2017 14:36, KimJeongYeon wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
For example, a normal stream tried to attach to filter sink or source, which<br>
filter loaded and managed by filter-apply. But, the stream goes to filter's<br>
***master sink or source*** due to unexpected restoring operation.<br>
It should attached to filter sink or source properly.<br>
<br>
Also, includes further fix according to Georg's comment, [1]<br>
If a stream that had filter.apply set initially is moved to another sink/source, then the filter<br>
should be applied again (a new filter, since the master sink/source has changed).<br>
<br>
If a stream that did not have filter.apply set initially is moved away, the property should<br>
be removed and no new filter applied.<br>
<br>
Also, a property list change might add or remove the filter.apply property. If it is added,<br>
we want that the filter is applied. Your patch does nothing and assumes that the stream<br>
is already filtered, even if the stream is not on a filter sink.<br>
<br>
[1] <a href="https://lists.freedesktop.org/archives/pulseaudio-discuss/2017-April/027980.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>archives/pulseaudio-discuss/20<wbr>17-April/027980.html</a><br>
<br>
Signed-off-by: KimJeongYeon <<a href="mailto:jeongyeon.kim@samsung.com" target="_blank">jeongyeon.kim@samsung.com</a>><br>
</blockquote>
Hi JeongYeon,<br>
<br>
sorry, but I still don't agree with your patch. As already said you do not need the<br>
enumeration, a simple boolean </blockquote></div></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">Ok. I'll do.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">should do. Also skip_prop_change seems unnecessary.<br></blockquote></div></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">About 'skip_prop_change':</div><div dir="auto">Move operation happens twice when call 'move_objects_for_filter'. Because, unexpected proplist-hook comes at 'do_move'.</div><div dir="auto">But, it doesn't harm moving operation.</div><div dir="auto">It would be remove <span style="font-family:sans-serif">'skip_prop_change'.</span></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I would name the new property filter.apply.mfa_loaded because the other properties<br>
have a filter.apply prefix (although I suggested the other name myself).<br>
I think the following logic is much simpler than your approach and still covers all cases<br>
(I always wrote sink but naturally the same applies for the source). Please correct<br>
me if I have overlooked anything.<br>
<br>
static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_input) {<br>
<br>
... leave the first bit unchanged ...<br>
<br>
/* Check if filter.apply property is set and get filter name. */<br>
if ((want = get_filter_name(o, is_sink_input))) {<br>
<br>
/* We end up here, when filter.apply is set */<br>
<br>
/* If module is not set, we don't know which type of sink we are on,<br>
* therefore do not try to do anything. (Not sure if this is right. The<br>
* filter sinks should always have module set, so we could conclude<br>
* that unset means that we are not on a filter sink and can just skip<br>
* the next check. But let's leave it like it is for the moment.) */<br>
if (!module)<br>
goto done;<br>
<br>
/* If we are already on the right filter sink, we don't need to do anything */<br>
module_name = pa_sprintf_malloc("module-%s", want);<br>
if (pa_streq(module->name, module_name)) {<br>
pa_log_debug("Stream appears to be playing on an appropriate sink already. Ignoring.");<br>
goto done;<br>
}<br>
<br>
/* Now filter apply is set and the sink is not the right filter sink.<br>
* We have two possible cases left: */<br>
<br>
/* 1) The stream has filter.apply.mfa_loaded set. This means the stream has been<br>
* manually moved away from the filter. */<br>
<br>
if (property filter.apply.mfa_loaded) {<br>
</blockquote>
<br></div>
I have forgotten one possible case here: The stream may have moved from one filter sink to another.<br>
In this case, filter.apply should be changed to the new filter and filter.apply.mfa_loaded should not be<br>
removed. So it should be:<br>
<br>
if (sink is filter sink) {<br>
change filter.apply<br>
} else {<br>
remove filter.apply<br>
remove filter.apply.mfa_loaded<br>
}<br>
keep sink as is<br>
<br>
instead of<div class="elided-text"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Remove filter.apply<br>
Remove filter.apply.mfa_loaded<br>
keep sink as is<br>
</blockquote>
<br>
<br>
</div></blockquote></div><br></div><div class="gmail_extra" dir="auto">Your logic seems fine for me.</div><div class="gmail_extra" dir="auto">I'll submit patch soon.</div><div class="gmail_extra" dir="auto"><br></div><div class="gmail_extra" dir="auto">Thanks,</div><div class="gmail_extra" dir="auto">KimJeongYeon</div></div></div>