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

[Georg Chini]

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. */

    }

  ...

}