2017. 4. 21. 오전 3:28에 "Georg Chini" <[email protected]>님이 작성:

Still found a few issues, but I think the next version will be final.

  +static void set_filter_properties(pa_proplist *pl, struct filter
> *filter, bool set_properties, bool set_by_mfa) {
>

You either call that function with ..., false, false) or with ..., true,
true), so the "set_by_mfa"
parameter is in fact not necessary. You can set/unset
PA_PROP_FILTER_APPLY_SET_BY_MFA
unconditionally.


Yes. I think so.


+            if ((filter = get_filter_for_object(u, o, is_sink_input))) {
> +                /* Change 'filter.apply'. */
> +                set_filter_properties(pl, NULL, false, false);
> +                set_filter_properties(pl, filter, true, true);
> +            } else {
> +                set_filter_properties(pl, NULL, false, false);
> +            }
>

You can move the "set_filter_properties(pl, NULL, false, false);" before
the if()
because you are doing it in both branches.


You're right. I'll fix.

+
> +            trigger_housekeeping(u);
> +            return PA_HOOK_OK;  /* goto done; */
>

You forget to free module_name. I would move the "done" label before the
"if (done_something)" and do "done_something=true; goto done" here.


Sorry. It was my mistake while doing my local merging. 'goto done;' is
correct.

    static pa_hook_result_t sink_input_proplist_cb(pa_core *core,
> pa_sink_input *i, struct userdata *u) {
>       pa_core_assert_ref(core);
>       pa_sink_input_assert_ref(i);
>

Didn't you want to avoid the double move here? Or will that be another
patch?


I hope submit another patch. It might be regarded as another issue.


    static pa_hook_result_t source_output_proplist_cb(pa_core *core,
> pa_source_output *o, struct userdata *u) {
>       pa_core_assert_ref(core);
>       pa_source_output_assert_ref(o);
>

Didn't you want to avoid the double move here? Or will that be another
patch?


I hope submit another patch. It might be regarded as another issue.

I'll submit patch v7 as soon as possible.
Thanks for reviewing!

Regards,
KimJeongYeon
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to