On Tue, 2017-09-12 at 09:12 +0200, Takashi Iwai wrote:
> On Tue, 12 Sep 2017 09:02:47 +0200,
> Clemens Ladisch wrote:
> >
> > Jörg Krause wrote:
> > > The i.MX6UL has a Synchronous Audio Interface (SAI), where the data in
> > > the FIFO can be aligned anywhere within the 32-bit wide register
> > > through the use of the First Bit Shifted configuration field. In the
> > > corresponding Linux SAI driver the FBS field is set the way that data
> > > alignment for 24-bit data is:
> > >
> > > 31 30 29 28 | 27 26 25 24 | 23 22 21 20 | .. | 3 2 1 0
> > > ## ## ## ## ## ## ## ## [ DATA[23:0] ]
> >
> > This indeed is S24_LE.
> >
> > Which is an extremely uncommon format. If possible, the driver should
> > be changed to align to the sample's MSB to the memory word's MSB, and
> > then label it as S32_LE.
> >
> > > > S24_LE cannot be handled with the same algorithm as S32_LE.
> >
> > To be precise: the sign in bit 23 must be preserved.
>
> I guess the simple calculation with S24_LE using the current code
> would work as long as the upper 8 bits are ignored by hardware.
> The upper 8 bits would hold bogus bits, and clearing this would be an
> extra action we'd need in addition.
Thanks! I've copied the area convertion macro for S24_3LE and
explicitly set the upper 8 bits to 0:
"""
#define CONVERT_AREA_S24_LE() do { \
unsigned int ch, fr; \
unsigned char *src, *dst; \
int tmp; \
for (ch = 0; ch < channels; ch++) { \
src_area = &src_areas[ch]; \
dst_area = &dst_areas[ch]; \
src = snd_pcm_channel_area_addr(src_area, src_offset); \
dst = snd_pcm_channel_area_addr(dst_area, dst_offset); \
src_step = snd_pcm_channel_area_step(src_area); \
dst_step = snd_pcm_channel_area_step(dst_area); \
GET_VOL_SCALE; \
fr = frames; \
if (! vol_scale) { \
while (fr--) { \
dst[0] = dst[1] = dst[2] = dst[3] = 0; \
dst += dst_step; \
} \
} else if (vol_scale == 0xffff) { \
while (fr--) { \
dst[0] = src[0]; \
dst[1] = src[1]; \
dst[2] = src[2]; \
dst[3] = 0; \
src += dst_step; \
dst += src_step; \
} \
} else { \
while (fr--) { \
tmp = src[0] | \
(src[1] << 8) | \
(((signed char *) src)[2] << 16); \
tmp = MULTI_DIV_24(tmp, vol_scale); \
dst[0] = tmp; \
dst[1] = tmp >> 8; \
dst[2] = tmp >> 16; \
dst[3] = 0; \
src += dst_step; \
dst += src_step; \
} \
} \
} \
} while (0)
"""
Is that okay?
Jörg Krause
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
