Message ID | 369cc35b-9625-4512-bb7a-7d5ccfe28e5c@xs4all.nl (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [GIT,PULL,FOR,v6.10] Add imx-asrc m2m audio rate control driver | expand |
Em Tue, 19 Mar 2024 16:15:35 +0100 Hans Verkuil <hverkuil@xs4all.nl> escreveu: > Hi Mauro, > > This adds support for the imx-asrc m2m audio rate control driver. Most of these > patches add support for the new v4l-audioX device nodes and related types, > and adding support for fixed point control types. > > The ASoC patches have been acked by Mark Brown. > > It also adds a vim2m-audio test driver so we can test the v4l-audio infrastructure. > > Since V4L2 is fourcc-based all the way, directly using snd_pcm_format_t values > is not an option: fourcc's are expected to be printable characters, and it is > pretty much certain that there are applications that show it like that to the > end-user. The utilities in v4l-utils certainly do, and it is in fact a > perfectly reasonable thing to do. So instead we map the snd_pcm_format_t value > to a fourcc with v4l2_audfmt_to_fourcc and map it back with v4l2_fourcc_to_audfmt. I still think that this can cause maintenance burden to sync up changes from fourcc to snd_pcm_format_t for no good reason, as any apps that will be working with this will require changes anyway to support the new devnodes and ioctls. With regards to printable fourcc values, it doesn't seem the best for audio. I mean, for V4L2, when we had fourcc introduced, the formats were: RGB3 BGR3 YUYV ... so, just printing them as ASCII would make sense, but those days were gone a very long time ago, as now we have fourccs like: pBCC Which you can't really know what it means, except by looking on its definition: #define V4L2_PIX_FMT_SBGGR12P v4l2_fourcc('p', 'B', 'C', 'C') For audio, even today's proposal makes no sense to be printed as ASCII. See, it is a lot clearer if userspace would print: "PCM A-LAW" # for ITU-T G711 A-law 8-bit encoding "PCM μ-LAW" # for ITU-T G711 μ-law 8-bit encoding "PCM S8" # for low-quality s8 encoding ... "PCM S32_LE" ... than any obfuscated fourccs: "AUAL" # currently missing at V4L2 proposal patch "AUML" # currently missing at V4L2 proposal patch "AU00" ... "AU10" ... Besides that, there were never any warranty that fourccs are printable at the media subsystem. I might be wrong but I guess we even used some unprintable ones in the past. So, the argument that new apps that will support audio will need to receive a fourcc instead of snd_pcm_format_t sounds bogus to me. Also, the current proposal misses lots of already-existing codes at snd_pcm_format_t. > > This PR is using v15 of the patch series: > > https://patchwork.linuxtv.org/project/linux-media/list/?series=12460 > > And the corresponding v4l-utils patch series is here: > > https://patchwork.linuxtv.org/user/todo/linux-media/?series=12074 > > Regards, > > Hans > > The following changes since commit b14257abe7057def6127f6fb2f14f9adc8acabdb: > > media: rcar-isp: Disallow unbind of devices (2024-03-07 16:35:13 +0100) > > are available in the Git repository at: > > git://linuxtv.org/hverkuil/media_tree.git tags/br-audio > > for you to fetch changes up to af06c8792653c42d1f126505ec9180255091d94e: > > media: vim2m-audio: add virtual driver for audio memory to memory (2024-03-19 15:55:38 +0100) > > ---------------------------------------------------------------- > Tag branch > > ---------------------------------------------------------------- > Hans Verkuil (1): > media: v4l2-ctrls: add support for fraction_bits > > Shengjiu Wang (15): > ASoC: fsl_asrc: define functions for memory to memory usage > ASoC: fsl_easrc: define functions for memory to memory usage > ASoC: fsl_asrc: move fsl_asrc_common.h to include/sound > ASoC: fsl_asrc: register m2m platform device > ASoC: fsl_easrc: register m2m platform device > media: uapi: Add V4L2_CAP_AUDIO_M2M capability flag > media: v4l2: Add audio capture and output support > media: uapi: Define audio sample format fourcc type > media: uapi: Add V4L2_CTRL_CLASS_M2M_AUDIO > media: uapi: Add audio rate controls support > media: uapi: Declare interface types for Audio > media: uapi: Add an entity type for audio resampler > media: vivid: add fixed point test controls > media: imx-asrc: Add memory to memory driver > media: vim2m-audio: add virtual driver for audio memory to memory > > Documentation/userspace-api/media/mediactl/media-types.rst | 11 + > Documentation/userspace-api/media/v4l/buffer.rst | 6 + > Documentation/userspace-api/media/v4l/common.rst | 1 + > Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst | 71 +++ > Documentation/userspace-api/media/v4l/devices.rst | 1 + > Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst | 59 +++ > Documentation/userspace-api/media/v4l/pixfmt-audio.rst | 100 ++++ > Documentation/userspace-api/media/v4l/pixfmt.rst | 1 + > Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst | 2 + > Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 4 + > Documentation/userspace-api/media/v4l/vidioc-g-fmt.rst | 4 + > Documentation/userspace-api/media/v4l/vidioc-querycap.rst | 3 + > Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst | 11 +- > Documentation/userspace-api/media/videodev2.h.rst.exceptions | 3 + > MAINTAINERS | 17 + > drivers/media/common/videobuf2/videobuf2-v4l2.c | 4 + > drivers/media/platform/nxp/Kconfig | 13 + > drivers/media/platform/nxp/Makefile | 1 + > drivers/media/platform/nxp/imx-asrc.c | 1256 +++++++++++++++++++++++++++++++++++++++++++++++++++ > drivers/media/test-drivers/Kconfig | 10 + > drivers/media/test-drivers/Makefile | 1 + > drivers/media/test-drivers/vim2m-audio.c | 793 ++++++++++++++++++++++++++++++++ > drivers/media/test-drivers/vivid/vivid-core.h | 2 + > drivers/media/test-drivers/vivid/vivid-ctrls.c | 26 ++ > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 + > drivers/media/v4l2-core/v4l2-ctrls-api.c | 1 + > drivers/media/v4l2-core/v4l2-ctrls-core.c | 93 +++- > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 10 + > drivers/media/v4l2-core/v4l2-dev.c | 21 + > drivers/media/v4l2-core/v4l2-ioctl.c | 66 +++ > drivers/media/v4l2-core/v4l2-mem2mem.c | 13 +- > include/media/v4l2-ctrls.h | 13 +- > include/media/v4l2-dev.h | 2 + > include/media/v4l2-ioctl.h | 34 ++ > {sound/soc/fsl => include/sound}/fsl_asrc_common.h | 60 +++ > include/uapi/linux/media.h | 2 + > include/uapi/linux/v4l2-controls.h | 9 + > include/uapi/linux/videodev2.h | 50 +- > sound/soc/fsl/fsl_asrc.c | 144 ++++++ > sound/soc/fsl/fsl_asrc.h | 4 +- > sound/soc/fsl/fsl_asrc_dma.c | 2 +- > sound/soc/fsl/fsl_easrc.c | 233 ++++++++++ > sound/soc/fsl/fsl_easrc.h | 6 +- > 43 files changed, 3145 insertions(+), 27 deletions(-) > create mode 100644 Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst > create mode 100644 Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst > create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-audio.rst > create mode 100644 drivers/media/platform/nxp/imx-asrc.c > create mode 100644 drivers/media/test-drivers/vim2m-audio.c > rename {sound/soc/fsl => include/sound}/fsl_asrc_common.h (60%) >
Hi Shengjiu, On 21/03/2024 15:57, Mauro Carvalho Chehab wrote: > Em Tue, 19 Mar 2024 16:15:35 +0100 > Hans Verkuil <hverkuil@xs4all.nl> escreveu: > >> Hi Mauro, >> >> This adds support for the imx-asrc m2m audio rate control driver. Most of these >> patches add support for the new v4l-audioX device nodes and related types, >> and adding support for fixed point control types. >> >> The ASoC patches have been acked by Mark Brown. >> >> It also adds a vim2m-audio test driver so we can test the v4l-audio infrastructure. >> >> Since V4L2 is fourcc-based all the way, directly using snd_pcm_format_t values >> is not an option: fourcc's are expected to be printable characters, and it is >> pretty much certain that there are applications that show it like that to the >> end-user. The utilities in v4l-utils certainly do, and it is in fact a >> perfectly reasonable thing to do. So instead we map the snd_pcm_format_t value >> to a fourcc with v4l2_audfmt_to_fourcc and map it back with v4l2_fourcc_to_audfmt. > > I still think that this can cause maintenance burden to sync up changes > from fourcc to snd_pcm_format_t for no good reason, as any apps that will > be working with this will require changes anyway to support the new > devnodes and ioctls. I will need more time to dig into this. I was on vacation for over a week, and after that I was very busy. This needs more time from me. But I haven't forgotten this, and it is on my todo list! Regards, Hans > > With regards to printable fourcc values, it doesn't seem the best for audio. > I mean, for V4L2, when we had fourcc introduced, the formats were: > > RGB3 > BGR3 > YUYV > ... > > so, just printing them as ASCII would make sense, but those days were > gone a very long time ago, as now we have fourccs like: > > pBCC > > Which you can't really know what it means, except by looking on its > definition: > > #define V4L2_PIX_FMT_SBGGR12P v4l2_fourcc('p', 'B', 'C', 'C') > > For audio, even today's proposal makes no sense to be printed as > ASCII. See, it is a lot clearer if userspace would print: > > "PCM A-LAW" # for ITU-T G711 A-law 8-bit encoding > "PCM μ-LAW" # for ITU-T G711 μ-law 8-bit encoding > "PCM S8" # for low-quality s8 encoding > ... > "PCM S32_LE" > ... > > than any obfuscated fourccs: > > "AUAL" # currently missing at V4L2 proposal patch > "AUML" # currently missing at V4L2 proposal patch > "AU00" > ... > "AU10" > ... > > Besides that, there were never any warranty that fourccs are printable > at the media subsystem. I might be wrong but I guess we even used some > unprintable ones in the past. > > > So, the argument that new apps that will support audio will need to > receive a fourcc instead of snd_pcm_format_t sounds bogus to me. > > Also, the current proposal misses lots of already-existing codes at > snd_pcm_format_t. > > >> >> This PR is using v15 of the patch series: >> >> https://patchwork.linuxtv.org/project/linux-media/list/?series=12460 >> >> And the corresponding v4l-utils patch series is here: >> >> https://patchwork.linuxtv.org/user/todo/linux-media/?series=12074 >> >> Regards, >> >> Hans >> >> The following changes since commit b14257abe7057def6127f6fb2f14f9adc8acabdb: >> >> media: rcar-isp: Disallow unbind of devices (2024-03-07 16:35:13 +0100) >> >> are available in the Git repository at: >> >> git://linuxtv.org/hverkuil/media_tree.git tags/br-audio >> >> for you to fetch changes up to af06c8792653c42d1f126505ec9180255091d94e: >> >> media: vim2m-audio: add virtual driver for audio memory to memory (2024-03-19 15:55:38 +0100) >> >> ---------------------------------------------------------------- >> Tag branch >> >> ---------------------------------------------------------------- >> Hans Verkuil (1): >> media: v4l2-ctrls: add support for fraction_bits >> >> Shengjiu Wang (15): >> ASoC: fsl_asrc: define functions for memory to memory usage >> ASoC: fsl_easrc: define functions for memory to memory usage >> ASoC: fsl_asrc: move fsl_asrc_common.h to include/sound >> ASoC: fsl_asrc: register m2m platform device >> ASoC: fsl_easrc: register m2m platform device >> media: uapi: Add V4L2_CAP_AUDIO_M2M capability flag >> media: v4l2: Add audio capture and output support >> media: uapi: Define audio sample format fourcc type >> media: uapi: Add V4L2_CTRL_CLASS_M2M_AUDIO >> media: uapi: Add audio rate controls support >> media: uapi: Declare interface types for Audio >> media: uapi: Add an entity type for audio resampler >> media: vivid: add fixed point test controls >> media: imx-asrc: Add memory to memory driver >> media: vim2m-audio: add virtual driver for audio memory to memory >> >> Documentation/userspace-api/media/mediactl/media-types.rst | 11 + >> Documentation/userspace-api/media/v4l/buffer.rst | 6 + >> Documentation/userspace-api/media/v4l/common.rst | 1 + >> Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst | 71 +++ >> Documentation/userspace-api/media/v4l/devices.rst | 1 + >> Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst | 59 +++ >> Documentation/userspace-api/media/v4l/pixfmt-audio.rst | 100 ++++ >> Documentation/userspace-api/media/v4l/pixfmt.rst | 1 + >> Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst | 2 + >> Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 4 + >> Documentation/userspace-api/media/v4l/vidioc-g-fmt.rst | 4 + >> Documentation/userspace-api/media/v4l/vidioc-querycap.rst | 3 + >> Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst | 11 +- >> Documentation/userspace-api/media/videodev2.h.rst.exceptions | 3 + >> MAINTAINERS | 17 + >> drivers/media/common/videobuf2/videobuf2-v4l2.c | 4 + >> drivers/media/platform/nxp/Kconfig | 13 + >> drivers/media/platform/nxp/Makefile | 1 + >> drivers/media/platform/nxp/imx-asrc.c | 1256 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> drivers/media/test-drivers/Kconfig | 10 + >> drivers/media/test-drivers/Makefile | 1 + >> drivers/media/test-drivers/vim2m-audio.c | 793 ++++++++++++++++++++++++++++++++ >> drivers/media/test-drivers/vivid/vivid-core.h | 2 + >> drivers/media/test-drivers/vivid/vivid-ctrls.c | 26 ++ >> drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 + >> drivers/media/v4l2-core/v4l2-ctrls-api.c | 1 + >> drivers/media/v4l2-core/v4l2-ctrls-core.c | 93 +++- >> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 10 + >> drivers/media/v4l2-core/v4l2-dev.c | 21 + >> drivers/media/v4l2-core/v4l2-ioctl.c | 66 +++ >> drivers/media/v4l2-core/v4l2-mem2mem.c | 13 +- >> include/media/v4l2-ctrls.h | 13 +- >> include/media/v4l2-dev.h | 2 + >> include/media/v4l2-ioctl.h | 34 ++ >> {sound/soc/fsl => include/sound}/fsl_asrc_common.h | 60 +++ >> include/uapi/linux/media.h | 2 + >> include/uapi/linux/v4l2-controls.h | 9 + >> include/uapi/linux/videodev2.h | 50 +- >> sound/soc/fsl/fsl_asrc.c | 144 ++++++ >> sound/soc/fsl/fsl_asrc.h | 4 +- >> sound/soc/fsl/fsl_asrc_dma.c | 2 +- >> sound/soc/fsl/fsl_easrc.c | 233 ++++++++++ >> sound/soc/fsl/fsl_easrc.h | 6 +- >> 43 files changed, 3145 insertions(+), 27 deletions(-) >> create mode 100644 Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst >> create mode 100644 Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst >> create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-audio.rst >> create mode 100644 drivers/media/platform/nxp/imx-asrc.c >> create mode 100644 drivers/media/test-drivers/vim2m-audio.c >> rename {sound/soc/fsl => include/sound}/fsl_asrc_common.h (60%) >> >