diff mbox series

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

Message ID 20210529033353.21641-1-o-takashi@sakamocchi.jp (mailing list archive)
State New, archived
Headers show
Series ALSA: pcm: comment about relation between msbits hw parameter and [S|U32] formats | expand

Commit Message

Takashi Sakamoto May 29, 2021, 3:33 a.m. UTC
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>
---
 include/sound/pcm.h         | 3 +++
 include/uapi/sound/asound.h | 3 +++
 2 files changed, 6 insertions(+)

Comments

Takashi Iwai May 30, 2021, 7:32 a.m. UTC | #1
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
Takashi Sakamoto Dec. 12, 2021, 11:30 a.m. UTC | #2
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?

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"

Regards

Takashi Sakamoto
Takashi Iwai Dec. 13, 2021, 7:20 a.m. UTC | #3
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.


thanks,

Takashi
Takashi Sakamoto Dec. 13, 2021, 8:02 a.m. UTC | #4
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
Takashi Iwai Dec. 13, 2021, 8:18 a.m. UTC | #5
On Mon, 13 Dec 2021 09:02:46 +0100,
Takashi Sakamoto wrote:
> 
> 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.

Ahh, yeah, that was the reason I dropped your patch later.


Takashi
diff mbox series

Patch

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 2e1200d17d0c..b84ff6e4ee53 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -147,6 +147,9 @@  struct snd_pcm_ops {
 #define SNDRV_PCM_FMTBIT_S24_BE		_SNDRV_PCM_FMTBIT(S24_BE)
 #define SNDRV_PCM_FMTBIT_U24_LE		_SNDRV_PCM_FMTBIT(U24_LE)
 #define SNDRV_PCM_FMTBIT_U24_BE		_SNDRV_PCM_FMTBIT(U24_BE)
+// For S32/U32 formats, 'msbits' hardware parameter is often used to deliver information about the
+// available bit count in most significant bit. It's for the case of so-called 'left-justified' or
+// `right-padding` sample which has less width than 32 bit.
 #define SNDRV_PCM_FMTBIT_S32_LE		_SNDRV_PCM_FMTBIT(S32_LE)
 #define SNDRV_PCM_FMTBIT_S32_BE		_SNDRV_PCM_FMTBIT(S32_BE)
 #define SNDRV_PCM_FMTBIT_U32_LE		_SNDRV_PCM_FMTBIT(U32_LE)
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index d17c061950df..b43ea0067e1f 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -202,6 +202,9 @@  typedef int __bitwise snd_pcm_format_t;
 #define	SNDRV_PCM_FORMAT_S24_BE	((__force snd_pcm_format_t) 7) /* low three bytes */
 #define	SNDRV_PCM_FORMAT_U24_LE	((__force snd_pcm_format_t) 8) /* low three bytes */
 #define	SNDRV_PCM_FORMAT_U24_BE	((__force snd_pcm_format_t) 9) /* low three bytes */
+// For S32/U32 formats, 'msbits' hardware parameter is often used to deliver information about the
+// available bit count in most significant bit. It's for the case of so-called 'left-justified' or
+// `right-padding` sample which has less width than 32 bit.
 #define	SNDRV_PCM_FORMAT_S32_LE	((__force snd_pcm_format_t) 10)
 #define	SNDRV_PCM_FORMAT_S32_BE	((__force snd_pcm_format_t) 11)
 #define	SNDRV_PCM_FORMAT_U32_LE	((__force snd_pcm_format_t) 12)