[pulseaudio-discuss] [PATCH v3] filter-apply, ladspa-sink, virtual-surround-sink: filter-apply supports ladspa-sink and virtual-surround-sink properly

Peter Meerwald-Stadler pmeerw at pmeerw.net
Fri Mar 10 09:46:20 UTC 2017


> Currently, filter-apply doesn't support ladspa-sink based filters, because:
> * loading ladspa-sink failed due to some argument mismatched.
>   (e.g 'master' used instead of 'sink_master')
> * ladspa-sink required additional parameters by default.
>   (e.g plugin/label/control/...)

some quick comments below
 
> Changes in v1:
> * filter-apply able to load ladspa-sink.
> * This patch introduces new sink-input(source-output) property to append extra parameters.
>   #define PA_PROP_FILTER_APPLY_EXTRA_PARAMETERS  "filter.apply.extra.parameters"
>   e.g) paplay file.wav --property=filter.apply=ladspa-sink \
>                        --property=filter.apply.extra.parameters="plugin=ladspa label=ladspa_stereo control=0"
> * ladspa-sink support autoloaded feature.
> 
> Changes in v2:
> * Additionally, support virtual-surround-sink too.
> 
> Changes in v3:
> * Currently, filter-apply manages filters using name of filter module and master sink.
> * Even if a filter able to load various plugins, it cannot run as multiple instances.
> * New 'group' of filter will provide to run multiple instances in this case.
> * For example, a filter group 'group1' of module-ladspa-sink has opened 'ladspa1' plugin.
>   Other group 'group2' able to open 'ladspa2' plugin in same time.
>   e.g) paplay file1.wav --property=filter.apply=ladspa-sink \
>                         --property=filter.apply.extra.group=group1 \
>                         --property=filter.apply.extra.parameters="plugin=ladspa1 label=stereo control=0"
>        paplay file2.wav --property=filter.apply=ladspa-sink \
>                         --property=filter.apply.extra.group=group2 \
>                         --property=filter.apply.extra.parameters="plugin=ladspa2 label=stereo control=0"
> 
> Signed-off-by: KimJeongYeon <jeongyeon.kim at samsung.com>
> ---
>  src/modules/module-filter-apply.c          | 72 ++++++++++++++++++++++++++----
>  src/modules/module-ladspa-sink.c           | 32 +++++++++++--
>  src/modules/module-virtual-surround-sink.c | 31 +++++++++++--
>  src/pulse/proplist.h                       |  6 +++
>  4 files changed, 125 insertions(+), 16 deletions(-)
> 
> diff --git a/src/modules/module-filter-apply.c b/src/modules/module-filter-apply.c
> index 364d68b..bf0ee5d 100644
> --- a/src/modules/module-filter-apply.c
> +++ b/src/modules/module-filter-apply.c
> @@ -56,6 +56,7 @@ static const char* const valid_modargs[] = {
>  
>  struct filter {
>      char *name;
> +    char *extra_group;
>      uint32_t module_index;
>      pa_sink *sink;
>      pa_sink *sink_master;
> @@ -93,17 +94,26 @@ static int filter_compare(const void *a, const void *b) {
>          return 1;
>      if ((r = strcmp(fa->name, fb->name)))
>          return r;
> +    else {
> +        if (fa->extra_group || fb->extra_group) {

what if one is NULL?
maybe use pa_safe_streq()?

> +            if ((r = strcmp(fa->extra_group, fb->extra_group)))
> +                return r;
> +            else
> +                return 0;
> +        }
> +    }
>  
>      return 0;
>  }
>  
> -static struct filter *filter_new(const char *name, pa_sink *sink, pa_source *source) {
> +static struct filter *filter_new(const char *name, const char *extra_group, pa_sink *sink, pa_source *source) {
>      struct filter *f;
>  
>      pa_assert(sink || source);
>  
>      f = pa_xnew(struct filter, 1);
>      f->name = pa_xstrdup(name);
> +    f->extra_group = pa_xstrdup(extra_group);
>      f->sink_master = sink;
>      f->source_master = source;
>      f->module_index = PA_INVALID_INDEX;
> @@ -117,6 +127,7 @@ static void filter_free(struct filter *f) {
>      pa_assert(f);
>  
>      pa_xfree(f->name);
> +    pa_xfree(f->extra_group);
>      pa_xfree(f);
>  }
>  
> @@ -140,6 +151,38 @@ static const char* should_filter(pa_object *o, bool is_sink_input) {
>      return NULL;
>  }
>  
> +static const char* should_filter_extra_parameters(pa_object *o, bool is_sink_input) {
> +    const char *extra_parameters;
> +    pa_proplist *pl;
> +
> +    if (is_sink_input)
> +        pl = PA_SINK_INPUT(o)->proplist;
> +    else
> +        pl = PA_SOURCE_OUTPUT(o)->proplist;
> +
> +    /* If the stream need extra parameters, append them to module. */

needs

> +    if ((extra_parameters = pa_proplist_gets(pl, PA_PROP_FILTER_APPLY_EXTRA_PARAMETERS)) && !pa_streq(extra_parameters, ""))
> +        return extra_parameters;
> +
> +    return NULL;
> +}
> +
> +static const char* should_filter_extra_group(pa_object *o, bool is_sink_input) {
> +    const char *extra_group;
> +    pa_proplist *pl;
> +
> +    if (is_sink_input)
> +        pl = PA_SINK_INPUT(o)->proplist;
> +    else
> +        pl = PA_SOURCE_OUTPUT(o)->proplist;
> +
> +    /* If the stream need extra group of filter, allow multiple instances. */

needs
group of filter_s_

> +    if ((extra_group = pa_proplist_gets(pl, PA_PROP_FILTER_APPLY_EXTRA_GROUP)) && !pa_streq(extra_group, ""))
> +        return extra_group;
> +
> +    return NULL;
> +}
> +
>  static bool should_group_filter(struct filter *filter) {
>      return pa_streq(filter->name, "echo-cancel");
>  }
> @@ -252,7 +295,7 @@ static void housekeeping_time_callback(pa_mainloop_api*a, pa_time_event* e, cons
>          if (nothing_attached(filter)) {
>              uint32_t idx;
>  
> -            pa_log_debug("Detected filter %s as no longer used. Unloading.", filter->name);
> +            pa_log_debug("Detected filter %s(%s) as no longer used. Unloading.", filter->name, filter->extra_group ? filter->extra_group : "no group");
>              idx = filter->module_index;
>              pa_hashmap_remove(u->filters, filter);
>              filter_free(filter);
> @@ -359,7 +402,7 @@ static void move_objects_for_filter(struct userdata *u, pa_object *o, struct fil
>  
>  /* Note that we assume a filter will provide at most one sink and at most one
>   * source (and at least one of either). */
> -static void find_filters_for_module(struct userdata *u, pa_module *m, const char *name) {
> +static void find_filters_for_module(struct userdata *u, pa_module *m, const char *name, const char *extra_group) {
>      uint32_t idx;
>      pa_sink *sink;
>      pa_source *source;
> @@ -369,7 +412,7 @@ static void find_filters_for_module(struct userdata *u, pa_module *m, const char
>          if (sink->module == m) {
>              pa_assert(pa_sink_is_filter(sink));
>  
> -            fltr = filter_new(name, sink->input_to_master->sink, NULL);
> +            fltr = filter_new(name, extra_group, sink->input_to_master->sink, NULL);
>              fltr->module_index = m->index;
>              fltr->sink = sink;
>  
> @@ -382,7 +425,7 @@ static void find_filters_for_module(struct userdata *u, pa_module *m, const char
>              pa_assert(pa_source_is_filter(source));
>  
>              if (!fltr) {
> -                fltr = filter_new(name, NULL, source->output_from_master->source);
> +                fltr = filter_new(name, extra_group, NULL, source->output_from_master->source);
>                  fltr->module_index = m->index;
>                  fltr->source = source;
>              } else {
> @@ -437,6 +480,9 @@ static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_i
>  
>      /* If the stream doesn't what any filter, then let it be. */
>      if ((want = should_filter(o, is_sink_input))) {
> +        const char *extra_parameters;
> +        const char *extra_group;
> +
>          /* We need to ensure the SI is playing on a sink of this type
>           * attached to the sink it's "officially" playing on */
>  
> @@ -449,7 +495,14 @@ static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_i
>              goto done;
>          }
>  
> -        fltr = filter_new(want, sink, source);
> +        /* Some filter modules might be required extra parameters by default.

might require

> +         * (e.g 'plugin', 'label', 'control' of module-ladspa-sink) */

e.g.

> +        extra_parameters = should_filter_extra_parameters(o, is_sink_input);
> +
> +        /* Some applications may want to extra group of filters. (optional) */

may want extra

> +        extra_group = should_filter_extra_group(o, is_sink_input);
> +
> +        fltr = filter_new(want, extra_group, sink, source);
>  
>          if (should_group_filter(fltr) && !find_paired_master(u, fltr, o, is_sink_input)) {
>              pa_log_debug("Want group filtering but don't have enough streams.");
> @@ -460,16 +513,17 @@ static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_i
>              char *args;
>              pa_module *m;
>  
> -            args = pa_sprintf_malloc("autoloaded=1 %s%s %s%s",
> +            args = pa_sprintf_malloc("autoloaded=1 %s%s %s%s %s",
>                      fltr->sink_master ? "sink_master=" : "",
>                      fltr->sink_master ? fltr->sink_master->name : "",
>                      fltr->source_master ? "source_master=" : "",
> -                    fltr->source_master ? fltr->source_master->name : "");
> +                    fltr->source_master ? fltr->source_master->name : "",
> +                    extra_parameters ? extra_parameters : "");
>  
>              pa_log_debug("Loading %s with arguments '%s'", module_name, args);
>  
>              if ((m = pa_module_load(u->core, module_name, args))) {
> -                find_filters_for_module(u, m, want);
> +                find_filters_for_module(u, m, want, extra_group);
>                  filter = pa_hashmap_get(u->filters, fltr);
>                  done_something = true;
>              }
> diff --git a/src/modules/module-ladspa-sink.c b/src/modules/module-ladspa-sink.c
> index c11fa5e..4fbf05e 100644
> --- a/src/modules/module-ladspa-sink.c
> +++ b/src/modules/module-ladspa-sink.c
> @@ -54,7 +54,7 @@ PA_MODULE_LOAD_ONCE(false);
>  PA_MODULE_USAGE(
>      _("sink_name=<name for the sink> "
>        "sink_properties=<properties for the sink> "
> -      "master=<name of sink to filter> "
> +      "sink_master=<name of sink to filter> "
>        "format=<sample format> "
>        "rate=<sample rate> "
>        "channels=<number of channels> "
> @@ -63,9 +63,11 @@ PA_MODULE_USAGE(
>        "label=<ladspa plugin label> "
>        "control=<comma separated list of input control values> "
>        "input_ladspaport_map=<comma separated list of input LADSPA port names> "
> -      "output_ladspaport_map=<comma separated list of output LADSPA port names> "));
> +      "output_ladspaport_map=<comma separated list of output LADSPA port names> "
> +      "autoloaded=<set if this module is being loaded automatically> "));
>  
>  #define MEMBLOCKQ_MAXLENGTH (16*1024*1024)
> +#define DEFAULT_AUTOLOADED false
>  
>  /* PLEASE NOTICE: The PortAudio ports and the LADSPA ports are two different concepts.
>  They are not related and where possible the names of the LADSPA port variables contains "ladspa" to avoid confusion */
> @@ -99,12 +101,13 @@ struct userdata {
>  #endif
>  
>      bool auto_desc;
> +    bool autoloaded;
>  };
>  
>  static const char* const valid_modargs[] = {
>      "sink_name",
>      "sink_properties",
> -    "master",
> +    "sink_master",
>      "format",
>      "rate",
>      "channels",
> @@ -114,6 +117,7 @@ static const char* const valid_modargs[] = {
>      "control",
>      "input_ladspaport_map",
>      "output_ladspaport_map",
> +    "autoloaded",
>      NULL
>  };
>  
> @@ -640,6 +644,19 @@ static void sink_input_state_change_cb(pa_sink_input *i, pa_sink_input_state_t s
>  }
>  
>  /* Called from main context */
> +static bool sink_input_may_move_to_cb(pa_sink_input *i, pa_sink *dest) {
> +    struct userdata *u;
> +
> +    pa_sink_input_assert_ref(i);
> +    pa_assert_se(u = i->userdata);
> +
> +    if (u->autoloaded)
> +        return false;
> +
> +    return u->sink != dest;
> +}
> +
> +/* Called from main context */
>  static void sink_input_moving_cb(pa_sink_input *i, pa_sink *dest) {
>      struct userdata *u;
>  
> @@ -968,7 +985,7 @@ int pa__init(pa_module*m) {
>          goto fail;
>      }
>  
> -    if (!(master = pa_namereg_get(m->core, pa_modargs_get_value(ma, "master", NULL), PA_NAMEREG_SINK))) {
> +    if (!(master = pa_namereg_get(m->core, pa_modargs_get_value(ma, "sink_master", NULL), PA_NAMEREG_SINK))) {
>          pa_log("Master sink not found");
>          goto fail;
>      }
> @@ -1231,6 +1248,12 @@ int pa__init(pa_module*m) {
>          goto fail;
>      }
>  
> +    u->autoloaded = DEFAULT_AUTOLOADED;
> +    if (pa_modargs_get_value_boolean(ma, "autoloaded", &u->autoloaded) < 0) {
> +        pa_log("Failed to parse autoloaded value");
> +        goto fail;
> +    }
> +
>      if ((u->auto_desc = !pa_proplist_contains(sink_data.proplist, PA_PROP_DEVICE_DESCRIPTION))) {
>          const char *z;
>  
> @@ -1283,6 +1306,7 @@ int pa__init(pa_module*m) {
>      u->sink_input->attach = sink_input_attach_cb;
>      u->sink_input->detach = sink_input_detach_cb;
>      u->sink_input->state_change = sink_input_state_change_cb;
> +    u->sink_input->may_move_to = sink_input_may_move_to_cb;
>      u->sink_input->moving = sink_input_moving_cb;
>      u->sink_input->mute_changed = sink_input_mute_changed_cb;
>      u->sink_input->userdata = u;
> diff --git a/src/modules/module-virtual-surround-sink.c b/src/modules/module-virtual-surround-sink.c
> index 6c7120a..0cf3910 100644
> --- a/src/modules/module-virtual-surround-sink.c
> +++ b/src/modules/module-virtual-surround-sink.c
> @@ -50,7 +50,7 @@ PA_MODULE_LOAD_ONCE(false);
>  PA_MODULE_USAGE(
>          _("sink_name=<name for the sink> "
>            "sink_properties=<properties for the sink> "
> -          "master=<name of sink to filter> "
> +          "sink_master=<name of sink to filter> "
>            "format=<sample format> "
>            "rate=<sample rate> "
>            "channels=<number of channels> "
> @@ -58,9 +58,11 @@ PA_MODULE_USAGE(
>            "use_volume_sharing=<yes or no> "
>            "force_flat_volume=<yes or no> "
>            "hrir=/path/to/left_hrir.wav "
> +          "autoloaded=<set if this module is being loaded automatically> "
>          ));
>  
>  #define MEMBLOCKQ_MAXLENGTH (16*1024*1024)
> +#define DEFAULT_AUTOLOADED false
>  
>  struct userdata {
>      pa_module *module;
> @@ -87,12 +89,14 @@ struct userdata {
>  
>      float *input_buffer;
>      int input_buffer_offset;
> +
> +    bool autoloaded;
>  };
>  
>  static const char* const valid_modargs[] = {
>      "sink_name",
>      "sink_properties",
> -    "master",
> +    "sink_master",
>      "format",
>      "rate",
>      "channels",
> @@ -100,6 +104,7 @@ static const char* const valid_modargs[] = {
>      "use_volume_sharing",
>      "force_flat_volume",
>      "hrir",
> +    "autoloaded",
>      NULL
>  };
>  
> @@ -428,6 +433,19 @@ static void sink_input_state_change_cb(pa_sink_input *i, pa_sink_input_state_t s
>  }
>  
>  /* Called from main context */
> +static bool sink_input_may_move_to_cb(pa_sink_input *i, pa_sink *dest) {
> +    struct userdata *u;
> +
> +    pa_sink_input_assert_ref(i);
> +    pa_assert_se(u = i->userdata);
> +
> +    if (u->autoloaded)
> +        return false;
> +
> +    return u->sink != dest;
> +}
> +
> +/* Called from main context */
>  static void sink_input_moving_cb(pa_sink_input *i, pa_sink *dest) {
>      struct userdata *u;
>  
> @@ -591,7 +609,7 @@ int pa__init(pa_module*m) {
>          goto fail;
>      }
>  
> -    if (!(master = pa_namereg_get(m->core, pa_modargs_get_value(ma, "master", NULL), PA_NAMEREG_SINK))) {
> +    if (!(master = pa_namereg_get(m->core, pa_modargs_get_value(ma, "sink_master", NULL), PA_NAMEREG_SINK))) {
>          pa_log("Master sink not found");
>          goto fail;
>      }
> @@ -672,6 +690,12 @@ int pa__init(pa_module*m) {
>          goto fail;
>      }
>  
> +    u->autoloaded = DEFAULT_AUTOLOADED;
> +    if (pa_modargs_get_value_boolean(ma, "autoloaded", &u->autoloaded) < 0) {
> +        pa_log("Failed to parse autoloaded value");
> +        goto fail;
> +    }
> +
>      if ((u->auto_desc = !pa_proplist_contains(sink_data.proplist, PA_PROP_DEVICE_DESCRIPTION))) {
>          const char *z;
>  
> @@ -731,6 +755,7 @@ int pa__init(pa_module*m) {
>      u->sink_input->attach = sink_input_attach_cb;
>      u->sink_input->detach = sink_input_detach_cb;
>      u->sink_input->state_change = sink_input_state_change_cb;
> +    u->sink_input->may_move_to = sink_input_may_move_to_cb;
>      u->sink_input->moving = sink_input_moving_cb;
>      u->sink_input->volume_changed = use_volume_sharing ? NULL : sink_input_volume_changed_cb;
>      u->sink_input->mute_changed = sink_input_mute_changed_cb;
> diff --git a/src/pulse/proplist.h b/src/pulse/proplist.h
> index bc9e8f8..7451624 100644
> --- a/src/pulse/proplist.h
> +++ b/src/pulse/proplist.h
> @@ -69,6 +69,12 @@ PA_C_DECL_BEGIN
>  /** For streams: the name of a filter that is desired, e.g.\ "echo-cancel" or "equalizer-sink". Differs from PA_PROP_FILTER_WANT in that it forces PulseAudio to apply the filter, regardless of whether PulseAudio thinks it makes sense to do so or not. If this is set, PA_PROP_FILTER_WANT is ignored. In other words, you almost certainly do not want to use this. \since 1.0 */
>  #define PA_PROP_FILTER_APPLY                   "filter.apply"
>  
> +/** For streams: some module required extra parameters, e.g.\ "plugin=ladspa label=ladspa_stereo control=0" in case of "ladspa-sink" filter */

some modules require

> +#define PA_PROP_FILTER_APPLY_EXTRA_PARAMETERS  "filter.apply.extra.parameters"
> +
> +/** For streams: some applications may want to classify sub filter groups and they can be used as multiple instances. (e.g load multiple plugin instances of "ladspa-sink" filter) */

e.g.

> +#define PA_PROP_FILTER_APPLY_EXTRA_GROUP       "filter.apply.extra.group"
> +
>  /** For streams: the name of a filter that should specifically suppressed (i.e.\ overrides PA_PROP_FILTER_WANT). Useful for the times that PA_PROP_FILTER_WANT is automatically added (e.g. echo-cancellation for phone streams when $VOIP_APP does its own, internal AEC) \since 1.0 */

that should specifically be suppressed

>  #define PA_PROP_FILTER_SUPPRESS                "filter.suppress"

-- 

Peter Meerwald-Stadler
Mobile: +43 664 24 44 418


More information about the pulseaudio-discuss mailing list