Re: [PATCH] ALSA: pcm: comment about relation between msbits hw parameter and [S|U32] formats

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

Hi,

On Mon, Dec 13, 2021 at 08:20:38AM +0100, Takashi Iwai wrote:
> On Sun, 12 Dec 2021 12:30:30 +0100,
> Takashi Sakamoto wrote:
> > 
> > Hi,
> > 
> > On Sun, May 30, 2021, at 16:32, Takashi Iwai wrote:
> > > On Sat, 29 May 2021 05:33:53 +0200,
> > > Takashi Sakamoto wrote:
> > >> 
> > >> Regarding to handling [U|S][32|24] PCM formats, many userspace
> > >> application developers and driver developers have confusion, since they
> > >> require them to understand justification or padding. It easily
> > >> loses consistency and soundness to operate with many type of devices. In
> > >> this commit, I attempt to solve the situation by adding comment about
> > >> relation between [S|U]32 formats and 'msbits' hardware parameter.
> > >> 
> > >> The formats are used for 'left-justified' sample format, and the available
> > >> bit count in most significant bit is delivered to userspace in msbits
> > >> hardware parameter (struct snd_pcm_hw_params.msbits), which is decided by
> > >> msbits constraint added by pcm drivers (snd_pcm_hw_constraint_msbits()).
> > >> 
> > >> In driver side, the msbits constraint includes two elements; the physical
> > >> width of format and the available width of the format in most significant
> > >> bit. The former is used to match SAMPLE_BITS of format. (For my
> > >> convenience, I ignore wildcard in the usage of the constraint.)
> > >> 
> > >> As a result of interaction between ALSA pcm core and ALSA pcm application,
> > >> when the format in which SAMPLE_BITS equals to physical width of the
> > >> msbits constaint, the msbits parameter is set by referring to the
> > >> available width of the constraint. When the msbits parameter is not
> > >> changed in the above process, ALSA pcm core set it alternatively with
> > >> SAMPLE_BIT of chosen format.
> > >> 
> > >> In userspace application side, the msbits is only available after calling
> > >> ioctl(2) with SNDRV_PCM_IOCTL_HW_PARAMS request. Even if the hardware
> > >> parameter structure includes somewhat value of SAMPLE_BITS interval
> > >> parameter as width of format, all of the width is not always available
> > >> since msbits can be less than the width.
> > >> 
> > >> I note that [S|U24] formats are used for 'right-justified' 24 bit sample
> > >> formats within 32 bit frame. The first byte in most significant bit
> > >> should be invalidated. Although the msbits exposed to userspace should be
> > >> zero as invalid value, actually it is 32 from physical width of format.
> > >> 
> > >> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > >
> > > Thanks, applied.
> > >
> > >
> > > Takashi
> > 
> > I can not find the patch in your tree. Would I ask you to review again?
> 
> Hrm, it seems that the commit was lost by some reason.  Sorry!
> 
> > If it should be going to be applied, I'd like you to fix my typo in the subject line;
> > 
> >  * "[S|U32]" -> "[S|U]32"
> 
> There was another similar typo in the patch description and I
> corrected both.  Now applied to for-next, commit
> fb6723daf89083a0d2290f3a0abc777e40766c84.

Thank you, and I overlooked that the patch still includes C99 style
comment in the part for UAPI header...  I'm preparing to fix it.


Regards

Takashi Sakamoto