On Tue, 29 Mar 2016 19:23:12 +0200, Russell King - ARM Linux wrote: > > On Tue, Mar 29, 2016 at 10:54:08AM +0200, Takashi Iwai wrote: > > On Thu, 17 Mar 2016 13:22:29 +0100, > > Jyri Sarha wrote: > > > > > > Add IEC958 channel status helper that gets the audio properties from > > > snd_pcm_hw_params instead of snd_pcm_runtime. This is needed to > > > produce the channel status bits already in audio stream configuration > > > phase. > > > > > > Signed-off-by: Jyri Sarha <jsarha@xxxxxx> > > > > This patch looks almost good to me, but... > > > > > @@ -71,6 +59,7 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, > > > IEC958_AES4_CON_MAX_WORDLEN_24; > > > break; > > > case 24: > > > + case 32: /* Assume 24-bit width for 32-bit samples. */ > > > ws = IEC958_AES4_CON_WORDLEN_24_20 | > > > IEC958_AES4_CON_MAX_WORDLEN_24; > > > break; > > > > ... this change is silently slipped in. It should be mentioned in the > > changelog, or split to another patch, as this is basically an > > orthogonal change. > > Does it even make sense - AES doesn't have support for 32-bit samples, > it can only ever truncate them down to 24-bit. Well, using SNDRV_PCM_FORMAT_S32_* for 24 bit samples is seen often on real devices, mostly on PCI ones. So, adding the value 32 check there itself is valid, I suppose. It's rather due to a bad design in the current API. The actual bits should be specified hw_params.msbits field, instead. But, not all drivers set this properly, unfortunately. Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel