Re: [PATCH] ALSA: pcm: clarify and fix default msbits value for all formats

[Jaroslav Kysela <perex@xxxxxxxx>]

On 22. 02. 24 19:33, Takashi Iwai wrote:
> On Thu, 22 Feb 2024 18:36:49 +0100,
> Jaroslav Kysela wrote:
> > Return used most significant bits from sample bit-width rather than the whole
> > physical sample word size. The starting bit offset is defined in the format
> > itself.
> >
> > The behaviour is not changed for 32-bit formats like S32_LE. But with this
> > change - msbits value 24 instead 32 is returned for 24-bit formats like S24_LE
> > etc.
> >
> > Also, commit 2112aa034907 ("ALSA: pcm: Introduce MSBITS subformat interface")
> > compares sample bit-width not physical sample bit-width to reset MSBITS_MAX bit
> > from the subformat bitmask.
> >
> > Probably no applications are using msbits value for other than S32_LE/U32_LE
> > formats, because no drivers are reducing msbits value for other formats (with
> > the msb offset) at the moment.
> >
> > For sanity, increase PCM protocol version, letting the user space to detect
> > the changed behaviour.
> >
> > Signed-off-by: Jaroslav Kysela <perex@xxxxxxxx>
>
> Hmm, the idea is nice, but I hesitate to take this as is.
>
> Basically the kernel should keep the backward compatibility, and this
> won't (although the feature is very minor).
>
> IMO, it'd be safer to check user_pversion and applies the new behavior
> only with it being >= 2.0.17, for example.

It's not so easy. The user_pversion is detached from substream (pcm_file 
structure) and propagating this value to all related functions creates really 
ugly code.

If you won't accept this ASIS, I propose to add new flags to the hw_params 
structure to alter the msbits behaviour. But I can hardly see any impact to 
the user space. The affected format/msbits combination was never used in the 
kernel drivers. It's more like a clarification than a fix.

					Jaroslav

--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.