<div dir="ltr"><br><br>On Mon, Feb 26, 2024 at 9:55 PM Nicolas Dufresne <<a href="mailto:nicolas@ndufresne.ca">nicolas@ndufresne.ca</a>> wrote:<br>><br>> Le lundi 26 février 2024 à 16:28 +0800, Shengjiu Wang a écrit :<br>> > The audio sample format definition is from alsa,<br>> > the header file is include/uapi/sound/asound.h, but<br>> > don't include this header file directly, because in<br>> > user space, there is another copy in alsa-lib.<br>> > There will be conflict in userspace for include<br>> > videodev2.h & asound.h and asoundlib.h<br>> ><br>> > Here still use the fourcc format.<br>><br>> I'd like to join Mauro's voice that duplicating the audio formats is a bad idea.<br>> We have the same issues with video formats when you look at V4L2 vs DRM. You're<br>> rationale is that videodev2.h will be ambiguous if it includes asound.h, but<br>> looking at this change, there is no reason why you need to include asound.h in<br>> videodev2.h at all. The format type can be abstracted out with a uint32 in the<br>> API, and then it would be up to the users to include and use the appropriate<br>> formats IDs.<br>><div><br></div><div>Thanks.</div><div><br>There is another reason mentioned by Hans:<br>"<br><i>The problem is that within V4L2 we use fourcc consistently to describe a<br>format, including in VIDIOC_ENUM_FMT. And the expectation is that the fourcc<br>can be printed to a human readable string (there is even a printk format for<br>that these days).<br><br>But the pcm values are all small integers (and can even be 0!), and<br>printing the fourcc will give garbage. It doesn't work well at all<br>with the V4L2 API. But by having a straightforward conversion between the<br>pcm identifier and a fourcc it was really easy to deal with this.<br><br>There might even be applications today that call VIDIOC_ENUM_FMT to see<br>what is supported and fail if it is not a proper fourcc is returned.<br><br>It will certainly report nonsense in v4l_print_fmtdesc() (v4l2-ioctl.c).<br><br>One of the early versions of this patch series did precisely what you request,<br>but it just doesn't work well within the V4L2 uAPI.</i><div><i>"</i><br><br>Best regards</div><div>Shengjiu Wang<br><br>> Nicolas<br>><br>> ><br>> > Signed-off-by: Shengjiu Wang <<a href="mailto:shengjiu.wang@nxp.com">shengjiu.wang@nxp.com</a>><br>> > ---<br>> >  .../userspace-api/media/v4l/pixfmt-audio.rst  | 87 +++++++++++++++++++<br>> >  .../userspace-api/media/v4l/pixfmt.rst        |  1 +<br>> >  drivers/media/v4l2-core/v4l2-ioctl.c          | 13 +++<br>> >  include/uapi/linux/videodev2.h                | 23 +++++<br>> >  4 files changed, 124 insertions(+)<br>> >  create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-audio.rst<br>> ><br>> > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-audio.rst b/Documentation/userspace-api/media/v4l/pixfmt-audio.rst<br>> > new file mode 100644<br>> > index 000000000000..04b4a7fbd8f4<br>> > --- /dev/null<br>> > +++ b/Documentation/userspace-api/media/v4l/pixfmt-audio.rst<br>> > @@ -0,0 +1,87 @@<br>> > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later<br>> > +<br>> > +.. _pixfmt-audio:<br>> > +<br>> > +*************<br>> > +Audio Formats<br>> > +*************<br>> > +<br>> > +These formats are used for :ref:`audiomem2mem` interface only.<br>> > +<br>> > +.. tabularcolumns:: |p{5.8cm}|p{1.2cm}|p{10.3cm}|<br>> > +<br>> > +.. cssclass:: longtable<br>> > +<br>> > +.. flat-table:: Audio Format<br>> > +    :header-rows:  1<br>> > +    :stub-columns: 0<br>> > +    :widths:       3 1 4<br>> > +<br>> > +    * - Identifier<br>> > +      - Code<br>> > +      - Details<br>> > +    * .. _V4L2-AUDIO-FMT-S8:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_S8``<br>> > +      - 'S8'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_S8 in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-S16-LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_S16_LE``<br>> > +      - 'S16_LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_S16_LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-U16-LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_U16_LE``<br>> > +      - 'U16_LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_U16_LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-S24-LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_S24_LE``<br>> > +      - 'S24_LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_S24_LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-U24-LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_U24_LE``<br>> > +      - 'U24_LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_U24_LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-S32-LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_S32_LE``<br>> > +      - 'S32_LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_S32_LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-U32-LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_U32_LE``<br>> > +      - 'U32_LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_U32_LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-FLOAT-LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_FLOAT_LE``<br>> > +      - 'FLOAT_LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_FLOAT_LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-IEC958-SUBFRAME-LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_IEC958_SUBFRAME_LE``<br>> > +      - 'IEC958_SUBFRAME_LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_IEC958_SUBFRAME_LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-S24-3LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_S24_3LE``<br>> > +      - 'S24_3LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_S24_3LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-U24-3LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_U24_3LE``<br>> > +      - 'U24_3LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_U24_3LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-S20-3LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_S20_3LE``<br>> > +      - 'S20_3LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_S24_3LE in ALSA<br>> > +    * .. _V4L2-AUDIO-FMT-U20-3LE:<br>> > +<br>> > +      - ``V4L2_AUDIO_FMT_U20_3LE``<br>> > +      - 'U20_3LE'<br>> > +      - Corresponds to SNDRV_PCM_FORMAT_U20_3LE in ALSA<br>> > diff --git a/Documentation/userspace-api/media/v4l/pixfmt.rst b/Documentation/userspace-api/media/v4l/pixfmt.rst<br>> > index 11dab4a90630..2eb6fdd3b43d 100644<br>> > --- a/Documentation/userspace-api/media/v4l/pixfmt.rst<br>> > +++ b/Documentation/userspace-api/media/v4l/pixfmt.rst<br>> > @@ -36,3 +36,4 @@ see also :ref:`VIDIOC_G_FBUF <VIDIOC_G_FBUF>`.)<br>> >      colorspaces<br>> >      colorspaces-defs<br>> >      colorspaces-details<br>> > +    pixfmt-audio<br>> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c<br>> > index 961abcdf7290..be229c69e991 100644<br>> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c<br>> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c<br>> > @@ -1471,6 +1471,19 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)<br>> >       case V4L2_PIX_FMT_Y210:         descr = "10-bit YUYV Packed"; break;<br>> >       case V4L2_PIX_FMT_Y212:         descr = "12-bit YUYV Packed"; break;<br>> >       case V4L2_PIX_FMT_Y216:         descr = "16-bit YUYV Packed"; break;<br>> > +     case V4L2_AUDIO_FMT_S8:         descr = "8-bit Signed"; break;<br>> > +     case V4L2_AUDIO_FMT_S16_LE:     descr = "16-bit Signed LE"; break;<br>> > +     case V4L2_AUDIO_FMT_U16_LE:             descr = "16-bit Unsigned LE"; break;<br>> > +     case V4L2_AUDIO_FMT_S24_LE:             descr = "24(32)-bit Signed LE"; break;<br>> > +     case V4L2_AUDIO_FMT_U24_LE:             descr = "24(32)-bit Unsigned LE"; break;<br>> > +     case V4L2_AUDIO_FMT_S32_LE:             descr = "32-bit Signed LE"; break;<br>> > +     case V4L2_AUDIO_FMT_U32_LE:             descr = "32-bit Unsigned LE"; break;<br>> > +     case V4L2_AUDIO_FMT_FLOAT_LE:           descr = "32-bit Float LE"; break;<br>> > +     case V4L2_AUDIO_FMT_IEC958_SUBFRAME_LE: descr = "32-bit IEC958 LE"; break;<br>> > +     case V4L2_AUDIO_FMT_S24_3LE:            descr = "24(24)-bit Signed LE"; break;<br>> > +     case V4L2_AUDIO_FMT_U24_3LE:            descr = "24(24)-bit Unsigned LE"; break;<br>> > +     case V4L2_AUDIO_FMT_S20_3LE:            descr = "20(24)-bit Signed LE"; break;<br>> > +     case V4L2_AUDIO_FMT_U20_3LE:            descr = "20(24)-bit Unsigned LE"; break;<br>> ><br>> >       default:<br>> >               /* Compressed formats */<br>> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h<br>> > index 2c03d2dfadbe..673a6235a029 100644<br>> > --- a/include/uapi/linux/videodev2.h<br>> > +++ b/include/uapi/linux/videodev2.h<br>> > @@ -843,6 +843,29 @@ struct v4l2_pix_format {<br>> >  #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */<br>> >  #define V4L2_META_FMT_RK_ISP1_STAT_3A        v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */<br>> ><br>> > +/*<br>> > + * Audio-data formats<br>> > + * All these audio formats use a fourcc starting with 'AU'<br>> > + * followed by the SNDRV_PCM_FORMAT_ value from asound.h.<br>> > + */<br>> > +#define V4L2_AUDIO_FMT_S8                    v4l2_fourcc('A', 'U', '0', '0')<br>> > +#define V4L2_AUDIO_FMT_S16_LE                        v4l2_fourcc('A', 'U', '0', '2')<br>> > +#define V4L2_AUDIO_FMT_U16_LE                        v4l2_fourcc('A', 'U', '0', '4')<br>> > +#define V4L2_AUDIO_FMT_S24_LE                        v4l2_fourcc('A', 'U', '0', '6')<br>> > +#define V4L2_AUDIO_FMT_U24_LE                        v4l2_fourcc('A', 'U', '0', '8')<br>> > +#define V4L2_AUDIO_FMT_S32_LE                        v4l2_fourcc('A', 'U', '1', '0')<br>> > +#define V4L2_AUDIO_FMT_U32_LE                        v4l2_fourcc('A', 'U', '1', '2')<br>> > +#define V4L2_AUDIO_FMT_FLOAT_LE                      v4l2_fourcc('A', 'U', '1', '4')<br>> > +#define V4L2_AUDIO_FMT_IEC958_SUBFRAME_LE    v4l2_fourcc('A', 'U', '1', '8')<br>> > +#define V4L2_AUDIO_FMT_S24_3LE                       v4l2_fourcc('A', 'U', '3', '2')<br>> > +#define V4L2_AUDIO_FMT_U24_3LE                       v4l2_fourcc('A', 'U', '3', '4')<br>> > +#define V4L2_AUDIO_FMT_S20_3LE                       v4l2_fourcc('A', 'U', '3', '6')<br>> > +#define V4L2_AUDIO_FMT_U20_3LE                       v4l2_fourcc('A', 'U', '3', '8')<br>> > +<br>> > +#define v4l2_fourcc_to_audfmt(fourcc)        \<br>> > +     (__force snd_pcm_format_t)(((((fourcc) >> 16) & 0xff) - '0') * 10  \<br>> > +                                + ((((fourcc) >> 24) & 0xff) - '0'))<br>> > +<br>> >  /* priv field value to indicates that subsequent fields are valid. */<br>> >  #define V4L2_PIX_FMT_PRIV_MAGIC              0xfeedcafe<br>> ><br>></div></div></div>