Em Mon, 19 Feb 2024 12:05:02 +0800 Shengjiu Wang shengjiu.wang@gmail.com escreveu:
Hi Mauro
On Sat, Feb 17, 2024 at 5:19 PM Mauro Carvalho Chehab mchehab@kernel.org wrote:
Em Thu, 18 Jan 2024 20:32:01 +0800 Shengjiu Wang shengjiu.wang@nxp.com escreveu:
The audio sample format definition is from alsa, the header file is include/uapi/sound/asound.h, but don't include this header file directly, because in user space, there is another copy in alsa-lib. There will be conflict in userspace for include videodev2.h & asound.h and asoundlib.h
Here still use the fourcc format.
Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com
.../userspace-api/media/v4l/pixfmt-audio.rst | 87 +++++++++++++++++++ .../userspace-api/media/v4l/pixfmt.rst | 1 + drivers/media/v4l2-core/v4l2-ioctl.c | 13 +++ include/uapi/linux/videodev2.h | 23 +++++ 4 files changed, 124 insertions(+) create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-audio.rst
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-audio.rst b/Documentation/userspace-api/media/v4l/pixfmt-audio.rst new file mode 100644 index 000000000000..04b4a7fbd8f4 --- /dev/null +++ b/Documentation/userspace-api/media/v4l/pixfmt-audio.rst @@ -0,0 +1,87 @@ +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
+.. _pixfmt-audio:
+************* +Audio Formats +*************
+These formats are used for :ref:`audiomem2mem` interface only.
+.. tabularcolumns:: |p{5.8cm}|p{1.2cm}|p{10.3cm}|
+.. cssclass:: longtable
+.. flat-table:: Audio Format
- :header-rows: 1
- :stub-columns: 0
- :widths: 3 1 4
- Identifier
- Code- Details
- .. _V4L2-AUDIO-FMT-S8:
- ``V4L2_AUDIO_FMT_S8``- 'S8'- Corresponds to SNDRV_PCM_FORMAT_S8 in ALSA
- .. _V4L2-AUDIO-FMT-S16-LE:
Hmm... why can't we just use SNDRV_*_FORMAT_*? Those are already part of an uAPI header. No need to add any abstraction here and/or redefine what is there already at include/uapi/sound/asound.h.
Actually I try to avoid including the include/uapi/sound/asound.h. Because in user space, there is another copy in alsa-lib (asoundlib.h). There will be conflict in userspace when including videodev2.h and asoundlib.h.
Well, alsasoundlib.h seems to be using the same definitions: https://github.com/michaelwu/alsa-lib/blob/master/include/pcm.h
So, I can't see what would be the actual issue, as both userspace library and ALSA internal headers use the same magic numbers.
You can still do things like:
#ifdef __KERNEL__ # include <sound/asound.h> #else # include <asoundlib.h> #endif
To avoid such kind of conflicts, if you need to have it included on some header file. Yet, I can't see why you would need that.
IMO, at uAPI headers, you just need to declare the uAPI audiofmt field to be either __u32 or __u64, pointing it to where this value comes from (on both userspace and Kernelspace. E. g.:
/** * struct v4l2_audio_format - audio data format definition * @audioformat: * an integer number matching the fields inside * enum snd_pcm_format_t (e. g. `SNDRV_PCM_FORMAT_*`), as defined * in include/uapi/sound/asound.h and * https://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html#gaa14b7f26.... * @channels: channel numbers * @buffersize: maximum size in bytes required for data */ struct v4l2_audio_format { __u32 audioformat; __u32 channels; __u32 buffersize; } __attribute__ ((packed));
Then, at documentation you just need to point to where the possible values for SNDRV_PCM_FORMAT_ are defined. No need to document them one by one.
With such definition, you'll only need to include sound/asound.h within the kAPI scope.
And in the V4l framework, the fourcc type is commonly used in other cases (video, radio, touch, meta....), to avoid changing common code a lot, so I think using fourcc definition for audio may be simpler.
Those are real video streams (or a video-related streams, in the case of metadata) where fourcc is widely used. There, it makes sense. However, ALSA format definitions are already being used for a long time. There's no sense on trying to reinvent it - or having an abstract layer to convert from/to fourcc <==> enum snd_pcm_format_t. Just use what is there already.
Thanks, Mauro