[alsa-devel] [RFC] ALSA: usb: supply channel maps even when wChannelConfig is unspecified

[Takashi Iwai]

At Fri,  1 Nov 2013 16:38:59 +0100,
David Henningsson wrote:
> 
> If wChannelconfig is given for some formats but not others, userspace
> might not be able to set the channel map.
> 
> This is RFC because I'm not sure what the best behaviour is - to guess
> the channel map from the given number of channels (it's quite likely
> that one channel is MONO and two channels is FL FR), or just to supply
> UNKNOWN for all channels.
> 

In the case of USB-audio, I guess this is OK to fill the guessed
chmaps, especially for the mono channel.  So I can take this patch if
you already tested on your device.

> But the complete lack of channel map for a format leads userspace to
> believe that the format is not available at all. Or am I
> misunderstanding how this should be used?

The chmap is just an optional information, thus excluding the format
due to the lack of chmap doesn't sound right.  The lack of chmap means
merely that the hardware doesn't give the chmap information, but the
format itself must be available.

So, in the case of PA, I'd expect it handles such a format as is of
now, just guessing / using the pre-defined chmaps.


thanks,

Takashi

> 
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
>  sound/usb/stream.c |   18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> index c4339f9..b43b6ee 100644
> --- a/sound/usb/stream.c
> +++ b/sound/usb/stream.c
> @@ -281,8 +281,6 @@ static struct snd_pcm_chmap_elem *convert_chmap(int channels, unsigned int bits,
>  	const unsigned int *maps;
>  	int c;
>  
> -	if (!bits)
> -		return NULL;
>  	if (channels > ARRAY_SIZE(chmap->map))
>  		return NULL;
>  
> @@ -293,9 +291,19 @@ static struct snd_pcm_chmap_elem *convert_chmap(int channels, unsigned int bits,
>  	maps = protocol == UAC_VERSION_2 ? uac2_maps : uac1_maps;
>  	chmap->channels = channels;
>  	c = 0;
> -	for (; bits && *maps; maps++, bits >>= 1) {
> -		if (bits & 1)
> -			chmap->map[c++] = *maps;
> +
> +	if (bits) {
> +		for (; bits && *maps; maps++, bits >>= 1)
> +			if (bits & 1)
> +				chmap->map[c++] = *maps;
> +	} else {
> +		/* If we're missing wChannelConfig, then guess something
> +		    to make sure the channel map is not skipped entirely */
> +		if (channels == 1)
> +			chmap->map[c++] = SNDRV_CHMAP_MONO;
> +		else
> +			for (; c < channels && *maps; maps++)
> +				chmap->map[c++] = *maps;
>  	}
>  
>  	for (; c < channels; c++)
> -- 
> 1.7.9.5
>