[alsa-devel] [PATCH 1/3] speaker-test: Fix chmapped channel selection without specified chmap

[Takashi Iwai]

At Sun, 10 Nov 2013 20:29:17 +0200,
Anssi Hannula wrote:
> 
> The channel selection currently does not work properly when there is a
> driver-provided non-ALSA-traditional channel map but no manual channel
> map was explicitely requested with "-m".
> 
> For example, the CEA/HDMI 8ch map is FL,FR,RLC,RRC,FC,LFE,RL,RR. Note
> that it is otherwise the same as the traditional ALSA channel map,
> except that the traditional rear speakers are considered
> rear-center speakers and the traditional side speakers are considered
> rear speakers.
> 
> Speaker-test tries to play back channels in this following order:
>   0, /* Front Left  */
>   4, /* Center      */
>   1, /* Front Right */
>   7, /* Side Right  */
>   3, /* Rear Right  */
>   2, /* Rear Left   */
>   6, /* Side Left   */
>   5, /* LFE         */
> 
> When it is the time to play back Side Left/Right, speaker-test tries to
> look for SL/SR in the chmap, but doesn't find it, so it just plays back
> channels 6/7 (which indeed are the side speakers, or RL/RR in this
> channel map - so the correct channels are selected).
> 
> When it becomes the time to playback Rear Left/Right, speaker-test again
> tries to find RL/RR in the chmap, and this time it does find them in the
> chmap positions 6/7.
> 
> So the channels 6/7 are tested twice and 2/3 are never tested.
> 
> To fix this, define a generic playback order to be used when a channel
> map is present and assign the speaker numbers (i.e. playback order) in
> the following order:
> 1. channels in map found in map_order[] in the map_order[] order,
> 2. channels in map not found in map_order[] in map order,
> 3. channels outside the map.

You are referring to the teaching of Zen, aren't you...? ;)

I prefer something a bit more easier to understand.
For example, how about to sort the given channels once using a weight
table?


thanks,

Takashi

> When the channel mapping is specified manually, the specified order is
> used for playback as before.
> 
> Signed-off-by: Anssi Hannula <anssi.hannula at iki.fi>
> ---
>  speaker-test/speaker-test.c | 86 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 67 insertions(+), 19 deletions(-)
> 
> diff --git a/speaker-test/speaker-test.c b/speaker-test/speaker-test.c
> index d35065f..274007d 100644
> --- a/speaker-test/speaker-test.c
> +++ b/speaker-test/speaker-test.c
> @@ -88,6 +88,8 @@ enum {
>  #define BE_INT(v)		(v)
>  #endif
>  
> +#define ARRAY_SIZE(x) (int)(sizeof(x)/sizeof(x[0]))
> +
>  static char              *device      = "default";       /* playback device */
>  static snd_pcm_format_t   format      = SND_PCM_FORMAT_S16; /* sample format */
>  static unsigned int       rate        = 48000;	            /* stream rate */
> @@ -156,36 +158,82 @@ static const int	channels8[] = {
>    5, /* LFE         */
>  };
>  
> +#ifdef CONFIG_SUPPORT_CHMAP
> +/* generic version of the above channelsX[] */
> +static const int map_order[] = {
> +  SND_CHMAP_FLW,
> +  SND_CHMAP_FL,
> +  SND_CHMAP_TFL,
> +  SND_CHMAP_FLC,
> +  SND_CHMAP_TFLC,
> +  SND_CHMAP_FC,
> +  SND_CHMAP_TFC,
> +  SND_CHMAP_FRC,
> +  SND_CHMAP_TFRC,
> +  SND_CHMAP_FR,
> +  SND_CHMAP_TFR,
> +  SND_CHMAP_FRW,
> +  SND_CHMAP_SR,
> +  SND_CHMAP_TSR,
> +  SND_CHMAP_RR,
> +  SND_CHMAP_TRR,
> +  SND_CHMAP_RRC,
> +  SND_CHMAP_RC,
> +  SND_CHMAP_TRC,
> +  SND_CHMAP_RLC,
> +  SND_CHMAP_RL,
> +  SND_CHMAP_TRL,
> +  SND_CHMAP_SL,
> +  SND_CHMAP_TSL,
> +  SND_CHMAP_BC,
> +  SND_CHMAP_TC,
> +  SND_CHMAP_LLFE,
> +  SND_CHMAP_LFE,
> +  SND_CHMAP_RLFE,
> +};
> +
>  static int get_mapped_channel(int chn)
>  {
> -#ifdef CONFIG_SUPPORT_CHMAP
> -  static const int maps[MAX_CHANNELS] = {
> -    SND_CHMAP_FL,
> -    SND_CHMAP_FR,
> -    SND_CHMAP_RL,
> -    SND_CHMAP_RR,
> -    SND_CHMAP_FC,
> -    SND_CHMAP_LFE,
> -    SND_CHMAP_SL,
> -    SND_CHMAP_SR,
> -  };
> +  int found_count = 0;
> +  int i, j;
> +
> +  if (chn >= channel_map->channels)
> +    return chn; /* out of map */
>  
> -  if (channel_map && maps[chn]) {
> -    int i;
> -    for (i = 0; i < channel_map->channels; i++) {
> -      if (channel_map->pos[i] == maps[chn])
> -	return i;
> +  for (i = 0; i < ARRAY_SIZE(map_order) && found_count <= chn; i++) {
> +    for (j = 0; j < channel_map->channels; j++) {
> +      if (channel_map->pos[j] == map_order[i]) {
> +        found_count++;
> +	break;
> +      }
>      }
>    }
> -#endif
> -  return chn;
> +
> +  if (found_count == chn + 1)
> +    return j;
> +
> +  /* chn is bigger than the amount of present map_order[] channels, find
> +   * the non-map_order[] channels */
> +  for (j = 0; j < channel_map->channels && found_count <= chn; j++) {
> +    for (i = 0; i < ARRAY_SIZE(map_order); i++) {
> +      if (channel_map->pos[j] == map_order[i])
> +        break;
> +    }
> +    if (i == ARRAY_SIZE(map_order))
> +      found_count++;
> +  }
> +
> +  return j - 1;
>  }
> +#endif
>  
>  static int get_speaker_channel(int chn)
>  {
>  #ifdef CONFIG_SUPPORT_CHMAP
>    if (channel_map_set)
>      return chn;
> +  if (channel_map)
> +    return get_mapped_channel(chn);
>  #endif
>  
>    switch (channels) {
> @@ -200,7 +248,7 @@ static int get_speaker_channel(int chn)
>      break;
>    }
>  
> -  return get_mapped_channel(chn);
> +  return chn;
>  }
>  
>  static const char *get_channel_name(int chn)
> -- 
> 1.8.1.5
>