[PATCH 3/3] alsa-ucm: Add the ability to set modargs from UCM config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/07/2013 05:39 PM, Arun Raghavan wrote:
> This adds mechanism to use a custom UCM value on verbs and modifiers to
> allow providing sink/source module arguments that are
> PulseAudio-speicific, such as mmap/tsched mode and ignore_dB.
> ---
>  src/modules/alsa/alsa-mixer.c       |  9 +++++++
>  src/modules/alsa/alsa-mixer.h       | 11 +++++++++
>  src/modules/alsa/alsa-ucm.c         | 41 +++++++++++++++++++++++++++----
>  src/modules/alsa/alsa-ucm.h         |  4 +++
>  src/modules/alsa/module-alsa-card.c | 49 +++++++++++++++++++++++++++++++------
>  5 files changed, 101 insertions(+), 13 deletions(-)
> 
> diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
> index 71dfa79..b51f8ea 100644
> --- a/src/modules/alsa/alsa-mixer.c
> +++ b/src/modules/alsa/alsa-mixer.c
> @@ -3310,6 +3310,15 @@ static void mapping_free(pa_alsa_mapping *m) {
>      pa_assert(!m->input_pcm);
>      pa_assert(!m->output_pcm);
>  
> +    if (m->sink_arguments) {
> +        pa_xfree(m->sink_arguments);
> +        pa_modargs_free(m->sink_modargs);

Would be more resilient to future refactorings to do:

  pa_xfree(m->sink_arguments);
  if (m->sink_modargs) {
    pa_modargs_free(m->sink_modargs);


> +    }
> +    if (m->source_arguments) {
> +        pa_xfree(m->source_arguments);
> +        pa_modargs_free(m->source_modargs);
> +    }
> +
>      pa_alsa_ucm_mapping_context_free(&m->ucm_context);
>  
>      pa_xfree(m);
> diff --git a/src/modules/alsa/alsa-mixer.h b/src/modules/alsa/alsa-mixer.h
> index 995a34b..79516db 100644
> --- a/src/modules/alsa/alsa-mixer.h
> +++ b/src/modules/alsa/alsa-mixer.h
> @@ -31,6 +31,7 @@
>  #include <pulse/volume.h>
>  
>  #include <pulsecore/llist.h>
> +#include <pulsecore/modargs.h>
>  #include <pulsecore/rtpoll.h>
>  
>  typedef struct pa_alsa_fdlist pa_alsa_fdlist;
> @@ -272,6 +273,16 @@ struct pa_alsa_mapping {
>      pa_sink *sink;
>      pa_source *source;
>  
> +    /* Module arguments from config, overriden by config/command line module

s/overriden/overridden

> +     * arguments */
> +    char *sink_arguments;
> +    char *source_arguments;
> +
> +    /* Parsed modargs, merged with the card module arguments. If null,
> +     * signifies that card modargs should directly be used. */
> +    pa_modargs *sink_modargs;
> +    pa_modargs *source_modargs;
> +
>      /* ucm device context*/
>      pa_alsa_ucm_mapping_context ucm_context;
>  };
> diff --git a/src/modules/alsa/alsa-ucm.c b/src/modules/alsa/alsa-ucm.c
> index a451c88..212942e 100644
> --- a/src/modules/alsa/alsa-ucm.c
> +++ b/src/modules/alsa/alsa-ucm.c
> @@ -92,6 +92,8 @@ static struct ucm_items item[] = {
>      {"CaptureRate", PA_ALSA_PROP_UCM_CAPTURE_RATE},
>      {"CaptureChannels", PA_ALSA_PROP_UCM_CAPTURE_CHANNELS},
>      {"TQ", PA_ALSA_PROP_UCM_QOS},
> +    {"PulseAudioSinkModargs", PA_ALSA_PROP_UCM_SINK_MODARGS},
> +    {"PulseAudioSourceModargs", PA_ALSA_PROP_UCM_SOURCE_MODARGS},

Hmm, we should probably document this behaviour (including the name of
this key) somewhere...

Also since this is called alsa.ucm.source_arguments maybe
PulseAudioSourceArguments would be a more consistent name here.

>      {NULL, NULL},
>  };
>  
> @@ -1244,6 +1246,7 @@ static int ucm_create_mapping_direction(
>          pa_alsa_profile_set *ps,
>          pa_alsa_profile *p,
>          pa_alsa_ucm_device *device,
> +        pa_alsa_ucm_verb *verb,
>          const char *verb_name,
>          const char *device_name,
>          const char *device_str,
> @@ -1251,6 +1254,7 @@ static int ucm_create_mapping_direction(
>  
>      pa_alsa_mapping *m;
>      char *mapping_name;
> +    const char *value;
>      unsigned priority, rate, channels;
>  
>      mapping_name = pa_sprintf_malloc("Mapping %s: %s: %s", verb_name, device_str, is_sink ? "sink" : "source");
> @@ -1281,6 +1285,16 @@ static int ucm_create_mapping_direction(
>          if (rate)
>              m->sample_spec.rate = rate;
>          pa_channel_map_init_extend(&m->channel_map, channels, PA_CHANNEL_MAP_ALSA);
> +
> +        if (is_sink) {
> +            value = pa_proplist_gets(verb->proplist, PA_ALSA_PROP_UCM_SINK_MODARGS);
> +            if (value)
> +                m->sink_arguments = pa_xstrdup(value);
> +        } else {
> +            value = pa_proplist_gets(verb->proplist, PA_ALSA_PROP_UCM_SOURCE_MODARGS);
> +            if (value)
> +                m->source_arguments = pa_xstrdup(value);
> +        }
>      }
>  
>      /* mapping priority is the highest one of ucm devices */
> @@ -1301,6 +1315,7 @@ static int ucm_create_mapping_for_modifier(
>          pa_alsa_profile_set *ps,
>          pa_alsa_profile *p,
>          pa_alsa_ucm_modifier *modifier,
> +        pa_alsa_ucm_verb *verb,
>          const char *verb_name,
>          const char *mod_name,
>          const char *device_str,
> @@ -1308,6 +1323,7 @@ static int ucm_create_mapping_for_modifier(
>  
>      pa_alsa_mapping *m;
>      char *mapping_name;
> +    const char *value;
>  
>      mapping_name = pa_sprintf_malloc("Mapping %s: %s: %s", verb_name, device_str, is_sink ? "sink" : "source");
>  
> @@ -1333,6 +1349,20 @@ static int ucm_create_mapping_for_modifier(
>          m->priority = 0;
>  
>          ucm_add_mapping(p, m);
> +
> +        if (is_sink) {
> +            value = pa_proplist_gets(modifier->proplist, PA_ALSA_PROP_UCM_SINK_MODARGS);
> +            if (!value)
> +                value = pa_proplist_gets(verb->proplist, PA_ALSA_PROP_UCM_SINK_MODARGS);
> +            if (value)
> +                m->sink_arguments = pa_xstrdup(value);
> +        } else {
> +            value = pa_proplist_gets(modifier->proplist, PA_ALSA_PROP_UCM_SOURCE_MODARGS);
> +            if (!value)
> +                value = pa_proplist_gets(verb->proplist, PA_ALSA_PROP_UCM_SOURCE_MODARGS);
> +            if (value)
> +                m->source_arguments = pa_xstrdup(value);
> +        }
>      } else if (!m->ucm_context.ucm_modifiers) /* share pcm with device */
>          m->ucm_context.ucm_modifiers = pa_idxset_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func);
>  
> @@ -1346,6 +1376,7 @@ static int ucm_create_mapping(
>          pa_alsa_profile_set *ps,
>          pa_alsa_profile *p,
>          pa_alsa_ucm_device *device,
> +        pa_alsa_ucm_verb *verb,
>          const char *verb_name,
>          const char *device_name,
>          const char *sink,
> @@ -1359,9 +1390,9 @@ static int ucm_create_mapping(
>      }
>  
>      if (sink)
> -        ret = ucm_create_mapping_direction(ucm, ps, p, device, verb_name, device_name, sink, true);
> +        ret = ucm_create_mapping_direction(ucm, ps, p, device, verb, verb_name, device_name, sink, true);

Hmm, maybe we should consider having the verb_name inside the
pa_alsa_ucm_verb struct?

>      if (ret == 0 && source)
> -        ret = ucm_create_mapping_direction(ucm, ps, p, device, verb_name, device_name, source, false);
> +        ret = ucm_create_mapping_direction(ucm, ps, p, device, verb, verb_name, device_name, source, false);
>  
>      return ret;
>  }
> @@ -1444,7 +1475,7 @@ static int ucm_create_profile(
>          sink = pa_proplist_gets(dev->proplist, PA_ALSA_PROP_UCM_SINK);
>          source = pa_proplist_gets(dev->proplist, PA_ALSA_PROP_UCM_SOURCE);
>  
> -        ucm_create_mapping(ucm, ps, p, dev, verb_name, name, sink, source);
> +        ucm_create_mapping(ucm, ps, p, dev, verb, verb_name, name, sink, source);
>  
>          if (sink)
>              dev->output_jack = ucm_get_jack(ucm, name, PA_UCM_PRE_TAG_OUTPUT);
> @@ -1461,9 +1492,9 @@ static int ucm_create_profile(
>          source = pa_proplist_gets(mod->proplist, PA_ALSA_PROP_UCM_SOURCE);
>  
>          if (sink)
> -            ucm_create_mapping_for_modifier(ucm, ps, p, mod, verb_name, name, sink, true);
> +            ucm_create_mapping_for_modifier(ucm, ps, p, mod, verb, verb_name, name, sink, true);
>          else if (source)
> -            ucm_create_mapping_for_modifier(ucm, ps, p, mod, verb_name, name, source, false);
> +            ucm_create_mapping_for_modifier(ucm, ps, p, mod, verb, verb_name, name, source, false);
>      }
>  
>      pa_alsa_profile_dump(p);
> diff --git a/src/modules/alsa/alsa-ucm.h b/src/modules/alsa/alsa-ucm.h
> index bae6b72..00adc56 100644
> --- a/src/modules/alsa/alsa-ucm.h
> +++ b/src/modules/alsa/alsa-ucm.h
> @@ -83,6 +83,10 @@ typedef void snd_use_case_mgr_t;
>  /** For devices: Quality of Service */
>  #define PA_ALSA_PROP_UCM_QOS                        "alsa.ucm.qos"
>  
> +/** For verbs/devices/modifiers: Passthrough module arguments for the corresponding Playback/CapturePCM */
> +#define PA_ALSA_PROP_UCM_SINK_MODARGS                        "alsa.ucm.sink_arguments"
> +#define PA_ALSA_PROP_UCM_SOURCE_MODARGS                      "alsa.ucm.source_arguments"
> +
>  /** For devices: The modifier (if any) that this device corresponds to */
>  #define PA_ALSA_PROP_UCM_MODIFIER "alsa.ucm.modifier"
>  
> diff --git a/src/modules/alsa/module-alsa-card.c b/src/modules/alsa/module-alsa-card.c
> index 970344a..a14c94d 100644
> --- a/src/modules/alsa/module-alsa-card.c
> +++ b/src/modules/alsa/module-alsa-card.c
> @@ -253,7 +253,7 @@ static int card_set_profile(pa_card *c, pa_card_profile *new_profile) {
>          PA_IDXSET_FOREACH(am, nd->profile->output_mappings, idx) {
>  
>              if (!am->sink)
> -                am->sink = pa_alsa_sink_new(c->module, u->modargs, __FILE__, c, am);
> +                am->sink = pa_alsa_sink_new(c->module, am->sink_modargs ? am->sink_modargs : u->modargs, __FILE__, c, am);
>  
>              if (sink_inputs && am->sink) {
>                  pa_sink_move_all_finish(am->sink, sink_inputs, false);
> @@ -265,7 +265,7 @@ static int card_set_profile(pa_card *c, pa_card_profile *new_profile) {
>          PA_IDXSET_FOREACH(am, nd->profile->input_mappings, idx) {
>  
>              if (!am->source)
> -                am->source = pa_alsa_source_new(c->module, u->modargs, __FILE__, c, am);
> +                am->source = pa_alsa_source_new(c->module, am->source_modargs ? am->source_modargs : u->modargs, __FILE__, c, am);
>  
>              if (source_outputs && am->source) {
>                  pa_source_move_all_finish(am->source, source_outputs, false);
> @@ -282,11 +282,26 @@ static int card_set_profile(pa_card *c, pa_card_profile *new_profile) {
>      return 0;
>  }
>  
> +static pa_modargs* prepare_modargs(const char *module, const char *config) {

Maybe this could be a utility function inside modargs instead - make it
more generic like

pa_modargs_from_two(arg1, arg2, valid_keys);

> +    pa_modargs *ma;
> +
> +    /* It's easier to re-parse the module arguments than to do a deep modargs copy */
> +    ma = pa_modargs_new(module, valid_modargs);
> +    if (!ma)
> +        return NULL;
> +
> +    if (pa_modargs_append(ma, config, valid_modargs) < 0)
> +        pa_log_error("Error parsing modargs from configuration");
> +
> +    return ma;
> +}
> +
>  static void init_profile(struct userdata *u) {
>      uint32_t idx;
>      pa_alsa_mapping *am;
>      struct profile_data *d;
>      pa_alsa_ucm_config *ucm = &u->ucm;
> +    pa_modargs *modargs;
>  
>      pa_assert(u);
>  
> @@ -300,13 +315,31 @@ static void init_profile(struct userdata *u) {
>          }
>      }
>  
> -    if (d->profile && d->profile->output_mappings)
> -        PA_IDXSET_FOREACH(am, d->profile->output_mappings, idx)
> -            am->sink = pa_alsa_sink_new(u->module, u->modargs, __FILE__, u->card, am);
> +    modargs = u->modargs;
> +
> +    if (d->profile && d->profile->output_mappings) {
> +        PA_IDXSET_FOREACH(am, d->profile->output_mappings, idx) {
> +            if (am->sink_arguments) {
> +                am->sink_modargs = prepare_modargs(u->module->argument, am->sink_arguments);

Unless I'm missing something, this looks like the only place
am->sink_modargs is assigned. And this initializes only one of the
profiles. What about the other profiles?

> +                pa_assert(am->sink_modargs);
> +                modargs = am->sink_modargs;
> +            }
> +
> +            am->sink = pa_alsa_sink_new(u->module, modargs, __FILE__, u->card, am);
> +        }
> +    }
> +
> +    if (d->profile && d->profile->input_mappings) {
> +        PA_IDXSET_FOREACH(am, d->profile->input_mappings, idx) {
> +            if (am->source_arguments) {
> +                am->source_modargs = prepare_modargs(u->module->argument, am->source_arguments);
> +                pa_assert(am->source_modargs);
> +                modargs = am->source_modargs;
> +            }
>  
> -    if (d->profile && d->profile->input_mappings)
> -        PA_IDXSET_FOREACH(am, d->profile->input_mappings, idx)
> -            am->source = pa_alsa_source_new(u->module, u->modargs, __FILE__, u->card, am);
> +            am->source = pa_alsa_source_new(u->module, modargs, __FILE__, u->card, am);
> +        }
> +    }
>  }
>  
>  static void report_port_state(pa_device_port *p, struct userdata *u) {
> 



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux