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

[Anssi Hannula <anssi.hannula@iki.fi>]

11.11.2013 17:56, Takashi Iwai kirjoitti:
> 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...? ;)

Heh, I only realized afterwards how confusing that map order stuff sounds :p

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

OK, I'll try something like that.

> 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@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
>>


-- 
Anssi Hannula