[PATCH v8 1/6] ALSA: pcm: add IEC958 channel status helper for hw_params
Takashi Iwai
tiwai at suse.de
Wed Mar 30 06:21:18 UTC 2016
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 at ti.com>
> >
> > 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
More information about the dri-devel
mailing list