[PATCH 1/2] alsa: Use alsa-lib's channel mapping API to select the best channel map

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

 



> Recently ALSA added the channel mapping API. Here we use it to set
> the right channel map if ALSA gives us several maps to choose from,
> and skip the PCM completely if no map fit.

nitpicking below; what blocks this patch?
 
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
>  src/modules/alsa/alsa-sink.c   |   6 ++
>  src/modules/alsa/alsa-source.c |   6 ++
>  src/modules/alsa/alsa-util.c   | 142 +++++++++++++++++++++++++++++++++++++++++
>  src/modules/alsa/alsa-util.h   |   2 +
>  4 files changed, 156 insertions(+)
> 
> diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
> index e10e14e..e101d48 100644
> --- a/src/modules/alsa/alsa-sink.c
> +++ b/src/modules/alsa/alsa-sink.c
> @@ -1111,6 +1111,9 @@ static int unsuspend(struct userdata *u) {
>          goto fail;
>      }
>  
> +    if (pa_alsa_set_channel_map(u->pcm_handle, &u->sink->channel_map) < 0)
> +        goto fail;
> +
>      if (update_sw_params(u) < 0)
>          goto fail;
>  
> @@ -2243,6 +2246,9 @@ pa_sink *pa_alsa_sink_new(pa_module *m, pa_modargs *ma, const char*driver, pa_ca
>          goto fail;
>      }
>  
> +    if (pa_alsa_set_channel_map(u->pcm_handle, &map) < 0)
> +        goto fail;
> +
>      /* ALSA might tweak the sample spec, so recalculate the frame size */
>      frame_size = pa_frame_size(&ss);
>  
> diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
> index 2e93e0f..41e6601 100644
> --- a/src/modules/alsa/alsa-source.c
> +++ b/src/modules/alsa/alsa-source.c
> @@ -995,6 +995,9 @@ static int unsuspend(struct userdata *u) {
>          goto fail;
>      }
>  
> +    if (pa_alsa_set_channel_map(u->pcm_handle, &u->source->channel_map) < 0)
> +        goto fail;
> +
>      if (update_sw_params(u) < 0)
>          goto fail;
>  
> @@ -1942,6 +1945,9 @@ pa_source *pa_alsa_source_new(pa_module *m, pa_modargs *ma, const char*driver, p
>          goto fail;
>      }
>  
> +    if (pa_alsa_set_channel_map(u->pcm_handle, &map) < 0)
> +        goto fail;
> +
>      /* ALSA might tweak the sample spec, so recalculate the frame size */
>      frame_size = pa_frame_size(&ss);
>  
> diff --git a/src/modules/alsa/alsa-util.c b/src/modules/alsa/alsa-util.c
> index 75f5858..0baf54d 100644
> --- a/src/modules/alsa/alsa-util.c
> +++ b/src/modules/alsa/alsa-util.c
> @@ -478,6 +478,143 @@ int pa_alsa_set_sw_params(snd_pcm_t *pcm, snd_pcm_uframes_t avail_min, bool peri
>      return 0;
>  }
>  
> +#if (SND_LIB_VERSION >= ((1<<16)|(0<<8)|27)) /* API additions in 1.0.27 */
> +
> +static const int chmap_channel_ids[PA_CHANNEL_POSITION_MAX] = {
> +    [PA_CHANNEL_POSITION_MONO] = SND_CHMAP_MONO,
> +
> +    [PA_CHANNEL_POSITION_FRONT_CENTER] = SND_CHMAP_FC,
> +    [PA_CHANNEL_POSITION_FRONT_LEFT] = SND_CHMAP_FL,
> +    [PA_CHANNEL_POSITION_FRONT_RIGHT] = SND_CHMAP_FR,
> +
> +    [PA_CHANNEL_POSITION_REAR_CENTER] = SND_CHMAP_RC,
> +    [PA_CHANNEL_POSITION_REAR_LEFT] = SND_CHMAP_RL,
> +    [PA_CHANNEL_POSITION_REAR_RIGHT] = SND_CHMAP_RR,
> +
> +    [PA_CHANNEL_POSITION_LFE] = SND_CHMAP_LFE,
> +
> +    [PA_CHANNEL_POSITION_FRONT_LEFT_OF_CENTER] = SND_CHMAP_FLC,
> +    [PA_CHANNEL_POSITION_FRONT_RIGHT_OF_CENTER] = SND_CHMAP_FRC,
> +
> +    [PA_CHANNEL_POSITION_SIDE_LEFT] = SND_CHMAP_SL,
> +    [PA_CHANNEL_POSITION_SIDE_RIGHT] = SND_CHMAP_SR,
> +
> +    [PA_CHANNEL_POSITION_TOP_CENTER] = SND_CHMAP_TC,
> +
> +    [PA_CHANNEL_POSITION_TOP_FRONT_CENTER] = SND_CHMAP_TFC,
> +    [PA_CHANNEL_POSITION_TOP_FRONT_LEFT] = SND_CHMAP_TFL,
> +    [PA_CHANNEL_POSITION_TOP_FRONT_RIGHT] = SND_CHMAP_TFR,
> +
> +    [PA_CHANNEL_POSITION_TOP_REAR_CENTER] = SND_CHMAP_TRC,
> +    [PA_CHANNEL_POSITION_TOP_REAR_LEFT] = SND_CHMAP_TRL,
> +    [PA_CHANNEL_POSITION_TOP_REAR_RIGHT] = SND_CHMAP_TRR
> +};
> +
> +int pa_alsa_set_channel_map(snd_pcm_t *pcm, pa_channel_map *map) {

map could be const

> +    snd_pcm_chmap_t *chmap;
> +    unsigned int i;
> +    int err;
> +    char buf[512] = "";

there are no asserts; the internal functions proposed do have asserts... 
it should be the other way around :)

init of buf unnecessary I think

> +
> +    chmap = alloca(sizeof(unsigned int) * (map->channels + 1));

this seems dangerous; I'd rather have a sizeof(snd_pcm_chmap_t) in there, 
and make sure it is zero'd

> +    chmap->channels = map->channels;
> +    for (i = 0; i < map->channels; i++)
> +        chmap->pos[i] = chmap_channel_ids[map->map[i]];
> +
> +    if (snd_pcm_chmap_print(chmap, sizeof(buf), buf) < 0)
> +        return -1;
> +    pa_log_debug("Setting channel map to %s", buf);
> +
> +    if ((err = snd_pcm_set_chmap(pcm, chmap)) < 0) {
> +        if (err == -ENXIO)
> +            return 0; /* snd_pcm_set_chmap returns -ENXIO if device does not support channel maps at all */
> +        pa_log_warn("Unable to set channel map: %s\n", pa_alsa_strerror(err));
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +
> +static int is_chmap_compatible(snd_pcm_chmap_query_t *c, pa_channel_map *map) {

this function does a lot more that checking for compatibility, find better 
name?
map const?

> +    unsigned int i;
> +    bool needsmove = false;
> +    char cm[PA_CHANNEL_MAP_SNPRINT_MAX];
> +    char buf[512] = "";
> +
> +    pa_assert(c);
> +    pa_assert(map);
> +
> +    pa_channel_map_snprint(cm, sizeof(cm), map);
> +    if (snd_pcm_chmap_print(&c->map, sizeof(buf), buf) < 0)
> +        return -1;
> +
> +    pa_log_debug("Checking ALSA channel map type %s, %s against PA map %s",
> +        snd_pcm_chmap_type_name(c->type), buf, cm);
> +
> +    if (c->map.channels != map->channels)
> +        needsmove = true;
> +
> +    for (i = 0; i < map->channels; i++) {
> +        unsigned int j;
> +        bool found = false;
> +        unsigned int m = chmap_channel_ids[map->map[i]];
> +        if (c->map.channels > i && m == c->map.pos[i])
> +            continue; /* Great, it's where we want it! */
> +        for (j = 0; j < c->map.channels; j++)
> +            if (m == c->map.pos[j]) {
> +                found = true;
> +                if (c->type != SND_CHMAP_TYPE_VAR)
> +                    needsmove = true;
> +                break;
> +            }
> +        if (!found)
> +            return -1;
> +    }
> +
> +    return needsmove ? 1 : 2;
> +}
> +
> +static int query_chmap_for_pcm(snd_pcm_t *pcm, pa_channel_map *map) {
> +
> +    snd_pcm_chmap_query_t **i, **avail;
> +    int bestscore = 0;
> +
> +    avail = snd_pcm_query_chmaps(pcm);
> +    if (avail == NULL) {
> +        pa_log_debug("This device does not support ALSA channel mapping API.");
> +        return 0;
> +    }
> +
> +    for (i = avail; *i != NULL; i++) {
> +         int score = is_chmap_compatible(*i, map);
> +         if (score > bestscore)
> +            bestscore = score;
> +    }
> +
> +    snd_pcm_free_chmaps(avail);
> +
> +    if (bestscore < 2) {
> +        /* TODO: If we find all channels but not in the right order, we can update
> +           our own channel map to match. We haven't implemented that yet. */
> +        pa_log_debug("Did not find a compatible channel map, skipping.");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +#else
> +
> +static int query_chmap_for_pcm(snd_pcm_t *pcm, pa_channel_map *map) {
> +    return 0;
> +}
> +
> +int pa_alsa_set_chmap(snd_pcm_t *pcm, pa_channel_map *map) {
> +    return 0;
> +}
> +
> +#endif
> +
>  snd_pcm_t *pa_alsa_open_by_device_id_auto(
>          const char *dev_id,
>          char **dev,
> @@ -714,6 +851,11 @@ snd_pcm_t *pa_alsa_open_by_device_string(
>              goto fail;
>          }
>  
> +        if (query_chmap_for_pcm(pcm_handle, map) < 0) {
> +            snd_pcm_close(pcm_handle);
> +            goto fail;
> +        }
> +
>          if (dev)
>              *dev = d;
>          else
> diff --git a/src/modules/alsa/alsa-util.h b/src/modules/alsa/alsa-util.h
> index 0e3ae69..2175985 100644
> --- a/src/modules/alsa/alsa-util.h
> +++ b/src/modules/alsa/alsa-util.h
> @@ -108,6 +108,8 @@ snd_pcm_t *pa_alsa_open_by_template(
>          bool *use_tsched,                 /* modified at return */
>          bool require_exact_channel_number);
>  
> +int pa_alsa_set_channel_map(snd_pcm_t *pcm, pa_channel_map *map);
> +
>  void pa_alsa_dump(pa_log_level_t level, snd_pcm_t *pcm);
>  void pa_alsa_dump_status(snd_pcm_t *pcm);
>  
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)


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

  Powered by Linux