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

[Takashi Iwai <tiwai@xxxxxxx>]

On Fri, 23 Feb 2024 09:45:40 +0100,
Takashi Iwai wrote:
> 
> On Thu, 22 Feb 2024 19:59:14 +0100,
> Jaroslav Kysela wrote:
> > 
> > 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.
> 
> OK, then let's take this.  I checked the major sound backend apps, and
> they don't care about msbits, so the influence must be pretty low.

To be more exact, both PA and PW have the same code to check the
msbits but they merely set "alsa.resolution_bits" property, and it's
not referred in the standard configs.


Takashi