Re: [RFC PATCH v2 4/7] media: v4l2: Add audio capture and output support

On Fri, Jul 28, 2023 at 3:59 PM Tomasz Figa tfiga@chromium.org wrote:

Hi Shengjiu,

On Tue, Jul 25, 2023 at 02:12:17PM +0800, Shengjiu Wang wrote:

...

Audio signal processing has the requirement for memory to memory similar as Video.

This patch is to add this support in v4l2 framework, defined new buffer type V4L2_BUF_TYPE_AUDIO_CAPTURE and V4L2_BUF_TYPE_AUDIO_OUTPUT, defined new format v4l2_audio_format for audio case usage.

The created audio device is named "/dev/audioX".

Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com

.../media/common/videobuf2/videobuf2-v4l2.c | 4 ++ drivers/media/v4l2-core/v4l2-dev.c | 17 ++++++ drivers/media/v4l2-core/v4l2-ioctl.c | 52 +++++++++++++++++++ include/media/v4l2-dev.h | 2 + include/media/v4l2-ioctl.h | 34 ++++++++++++ include/uapi/linux/videodev2.h | 19 +++++++ 6 files changed, 128 insertions(+)

Thanks for the patch! Please check my comments inline.

Thanks for reviewing.

Sorry for sending again for using the plain text mode.

...

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index c7a54d82a55e..12f2be2773a2 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -785,6 +785,10 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) case V4L2_BUF_TYPE_META_OUTPUT: requested_sizes[0] = f->fmt.meta.buffersize; break;

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:
  

•         requested_sizes[0] = f->fmt.audio.buffersize;
  

•         break;
  default:
          return -EINVAL;
  }
  

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index f81279492682..67484f4c6eaf 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -553,6 +553,7 @@ static void determine_valid_ioctls(struct video_device *vdev) bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH; bool is_meta = vdev->vfl_type == VFL_TYPE_VIDEO && (vdev->device_caps & meta_caps);

• bool is_audio = vdev->vfl_type == VFL_TYPE_AUDIO;
  bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
  bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
  bool is_io_mc = vdev->device_caps & V4L2_CAP_IO_MC;
  

@@ -664,6 +665,19 @@ static void determine_valid_ioctls(struct video_device *vdev) SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_meta_out); SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_meta_out); }

• if (is_audio && is_rx) {
  

•         /* audio capture specific ioctls */
  

•         SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_audio_cap);
  

•         SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_audio_cap);
  

•         SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_audio_cap);
  

•         SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_audio_cap);
  

• } else if (is_audio && is_tx) {
  

•         /* audio output specific ioctls */
  

•         SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_audio_out);
  

•         SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_audio_out);
  

•         SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_audio_out);
  

•         SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_audio_out);
  

• }
  if (is_vbi) {
          /* vbi specific ioctls */
          if ((is_rx && (ops->vidioc_g_fmt_vbi_cap ||
  

@@ -927,6 +941,9 @@ int __video_register_device(struct video_device *vdev, case VFL_TYPE_TOUCH: name_base = "v4l-touch"; break;

• case VFL_TYPE_AUDIO:
  

•         name_base = "audio";
  

I think it was mentioned before that "audio" could be confusing. Wasn't there actually some other kind of /dev/audio device long ago?

Seems like for touch, "v4l-touch" was introduced. Maybe it would also make sense to call it "v4l-audio" for audio?

Ok, will change to use "v4l-audio".

...

•         break;
  default:
          pr_err("%s called with unknown type: %d\n",
                 __func__, type);
  

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 01ba27f2ef87..aa9d872bba8d 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -188,6 +188,8 @@ const char *v4l2_type_names[] = { [V4L2_BUF_TYPE_SDR_OUTPUT] = "sdr-out", [V4L2_BUF_TYPE_META_CAPTURE] = "meta-cap", [V4L2_BUF_TYPE_META_OUTPUT] = "meta-out",

• [V4L2_BUF_TYPE_AUDIO_CAPTURE]      = "audio-cap",
  

• [V4L2_BUF_TYPE_AUDIO_OUTPUT]       = "audio-out",
  

}; EXPORT_SYMBOL(v4l2_type_names);

@@ -276,6 +278,7 @@ static void v4l_print_format(const void *arg, bool write_only) const struct v4l2_sliced_vbi_format *sliced; const struct v4l2_window *win; const struct v4l2_meta_format *meta;

• const struct v4l2_audio_format *audio;
  u32 pixelformat;
  u32 planes;
  unsigned i;
  

@@ -346,6 +349,12 @@ static void v4l_print_format(const void *arg, bool write_only) pr_cont(", dataformat=%p4cc, buffersize=%u\n", &pixelformat, meta->buffersize); break;

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:
  

•         audio = &p->fmt.audio;
  

•         pr_cont(", rate=%u, format=%u, channels=%u, buffersize=%u\n",
  

•                 audio->rate, audio->format, audio->channels, audio->buffersize);
  

•         break;
  }
  

}

@@ -927,6 +936,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type) bool is_tch = vfd->vfl_type == VFL_TYPE_TOUCH; bool is_meta = vfd->vfl_type == VFL_TYPE_VIDEO && (vfd->device_caps & meta_caps);

• bool is_audio = vfd->vfl_type == VFL_TYPE_AUDIO;
  bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
  bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
  
  

@@ -992,6 +1002,14 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type) if (is_meta && is_tx && ops->vidioc_g_fmt_meta_out) return 0; break;

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:
  

•         if (is_audio && is_rx && ops->vidioc_g_fmt_audio_cap)
  

•                 return 0;
  

•         break;
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:
  

•         if (is_audio && is_tx && ops->vidioc_g_fmt_audio_out)
  

•                 return 0;
  

•         break;
  default:
          break;
  }
  

@@ -1594,6 +1612,16 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops, break; ret = ops->vidioc_enum_fmt_meta_out(file, fh, arg); break;

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:
  

•         if (unlikely(!ops->vidioc_enum_fmt_audio_cap))
  

•                 break;
  

•         ret = ops->vidioc_enum_fmt_audio_cap(file, fh, arg);
  

•         break;
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:
  

•         if (unlikely(!ops->vidioc_enum_fmt_audio_out))
  

•                 break;
  

•         ret = ops->vidioc_enum_fmt_audio_out(file, fh, arg);
  

•         break;
  }
  if (ret == 0)
          v4l_fill_fmtdesc(p);
  

@@ -1670,6 +1698,10 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops, return ops->vidioc_g_fmt_meta_cap(file, fh, arg); case V4L2_BUF_TYPE_META_OUTPUT: return ops->vidioc_g_fmt_meta_out(file, fh, arg);

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:
  

•         return ops->vidioc_g_fmt_audio_cap(file, fh, arg);
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:
  

•         return ops->vidioc_g_fmt_audio_out(file, fh, arg);
  }
  return -EINVAL;
  

} @@ -1781,6 +1813,16 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops, break; memset_after(p, 0, fmt.meta); return ops->vidioc_s_fmt_meta_out(file, fh, arg);

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:
  

•         if (unlikely(!ops->vidioc_s_fmt_audio_cap))
  

•                 break;
  

•         memset_after(p, 0, fmt.audio);
  

•         return ops->vidioc_s_fmt_audio_cap(file, fh, arg);
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:
  

•         if (unlikely(!ops->vidioc_s_fmt_audio_out))
  

•                 break;
  

•         memset_after(p, 0, fmt.audio);
  

•         return ops->vidioc_s_fmt_audio_out(file, fh, arg);
  }
  return -EINVAL;
  

} @@ -1889,6 +1931,16 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops, break; memset_after(p, 0, fmt.meta); return ops->vidioc_try_fmt_meta_out(file, fh, arg);

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:
  

•         if (unlikely(!ops->vidioc_try_fmt_audio_cap))
  

•                 break;
  

•         memset_after(p, 0, fmt.audio);
  

•         return ops->vidioc_try_fmt_audio_cap(file, fh, arg);
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:
  

•         if (unlikely(!ops->vidioc_try_fmt_audio_out))
  

•                 break;
  

•         memset_after(p, 0, fmt.audio);
  

•         return ops->vidioc_try_fmt_audio_out(file, fh, arg);
  }
  return -EINVAL;
  

} diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h index e0a13505f88d..0924e6d1dab1 100644 --- a/include/media/v4l2-dev.h +++ b/include/media/v4l2-dev.h @@ -30,6 +30,7 @@

• @VFL_TYPE_SUBDEV: for V4L2 subdevices
• @VFL_TYPE_SDR: for Software Defined Radio tuners
• @VFL_TYPE_TOUCH: for touch sensors

• • @VFL_TYPE_AUDIO: for audio input/output devices
  • @VFL_TYPE_MAX: number of VFL types, must always be last in the enum
  */

enum vfl_devnode_type { @@ -39,6 +40,7 @@ enum vfl_devnode_type { VFL_TYPE_SUBDEV, VFL_TYPE_SDR, VFL_TYPE_TOUCH,

• VFL_TYPE_AUDIO,
  VFL_TYPE_MAX /* Shall be the last one */
  

};

diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h index edb733f21604..f840cf740ce1 100644 --- a/include/media/v4l2-ioctl.h +++ b/include/media/v4l2-ioctl.h @@ -45,6 +45,12 @@ struct v4l2_fh;

• @vidioc_enum_fmt_meta_out: pointer to the function that implements
• :ref:`VIDIOC_ENUM_FMT <vidioc_enum_fmt>` ioctl logic
• for metadata output

• • @vidioc_enum_fmt_audio_cap: pointer to the function that implements
• • :ref:`VIDIOC_ENUM_FMT <vidioc_enum_fmt>` ioctl logic
• • for audio capture
• • @vidioc_enum_fmt_audio_out: pointer to the function that implements
• • :ref:`VIDIOC_ENUM_FMT <vidioc_enum_fmt>` ioctl logic
• • for audio output
  • @vidioc_g_fmt_vid_cap: pointer to the function that implements
  • :ref:`VIDIOC_G_FMT <vidioc_g_fmt>` ioctl logic for video capture
  • in single plane mode

@@ -79,6 +85,10 @@ struct v4l2_fh;

• :ref:`VIDIOC_G_FMT <vidioc_g_fmt>` ioctl logic for metadata capture
• @vidioc_g_fmt_meta_out: pointer to the function that implements
• :ref:`VIDIOC_G_FMT <vidioc_g_fmt>` ioctl logic for metadata output

• • @vidioc_g_fmt_audio_cap: pointer to the function that implements
• • :ref:`VIDIOC_G_FMT <vidioc_g_fmt>` ioctl logic for audio capture
• • @vidioc_g_fmt_audio_out: pointer to the function that implements
• • :ref:`VIDIOC_G_FMT <vidioc_g_fmt>` ioctl logic for audio output
  • @vidioc_s_fmt_vid_cap: pointer to the function that implements
  • :ref:`VIDIOC_S_FMT <vidioc_g_fmt>` ioctl logic for video capture
  • in single plane mode

@@ -113,6 +123,10 @@ struct v4l2_fh;

• :ref:`VIDIOC_S_FMT <vidioc_g_fmt>` ioctl logic for metadata capture
• @vidioc_s_fmt_meta_out: pointer to the function that implements
• :ref:`VIDIOC_S_FMT <vidioc_g_fmt>` ioctl logic for metadata output

• • @vidioc_s_fmt_audio_cap: pointer to the function that implements
• • :ref:`VIDIOC_S_FMT <vidioc_g_fmt>` ioctl logic for audio capture
• • @vidioc_s_fmt_audio_out: pointer to the function that implements
• • :ref:`VIDIOC_S_FMT <vidioc_g_fmt>` ioctl logic for audio output
  • @vidioc_try_fmt_vid_cap: pointer to the function that implements
  • :ref:`VIDIOC_TRY_FMT <vidioc_g_fmt>` ioctl logic for video capture
  • in single plane mode

@@ -149,6 +163,10 @@ struct v4l2_fh;

• :ref:`VIDIOC_TRY_FMT <vidioc_g_fmt>` ioctl logic for metadata capture
• @vidioc_try_fmt_meta_out: pointer to the function that implements
• :ref:`VIDIOC_TRY_FMT <vidioc_g_fmt>` ioctl logic for metadata output

• • @vidioc_try_fmt_audio_cap: pointer to the function that implements
• • :ref:`VIDIOC_TRY_FMT <vidioc_g_fmt>` ioctl logic for audio capture
• • @vidioc_try_fmt_audio_out: pointer to the function that implements
• • :ref:`VIDIOC_TRY_FMT <vidioc_g_fmt>` ioctl logic for audio output
  • @vidioc_reqbufs: pointer to the function that implements
  • :ref:`VIDIOC_REQBUFS <vidioc_reqbufs>` ioctl
  • @vidioc_querybuf: pointer to the function that implements

@@ -315,6 +333,10 @@ struct v4l2_ioctl_ops { struct v4l2_fmtdesc *f); int (*vidioc_enum_fmt_meta_out)(struct file *file, void *fh, struct v4l2_fmtdesc *f);

• int (*vidioc_enum_fmt_audio_cap)(struct file *file, void *fh,
  

•                                  struct v4l2_fmtdesc *f);
  

• int (*vidioc_enum_fmt_audio_out)(struct file *file, void *fh,
  

•                                  struct v4l2_fmtdesc *f);
  
  /* VIDIOC_G_FMT handlers */
  int (*vidioc_g_fmt_vid_cap)(struct file *file, void *fh,
  

@@ -345,6 +367,10 @@ struct v4l2_ioctl_ops { struct v4l2_format *f); int (*vidioc_g_fmt_meta_out)(struct file *file, void *fh, struct v4l2_format *f);

• int (*vidioc_g_fmt_audio_cap)(struct file *file, void *fh,
  

•                               struct v4l2_format *f);
  

• int (*vidioc_g_fmt_audio_out)(struct file *file, void *fh,
  

•                               struct v4l2_format *f);
  
  /* VIDIOC_S_FMT handlers */
  int (*vidioc_s_fmt_vid_cap)(struct file *file, void *fh,
  

@@ -375,6 +401,10 @@ struct v4l2_ioctl_ops { struct v4l2_format *f); int (*vidioc_s_fmt_meta_out)(struct file *file, void *fh, struct v4l2_format *f);

• int (*vidioc_s_fmt_audio_cap)(struct file *file, void *fh,
  

•                               struct v4l2_format *f);
  

• int (*vidioc_s_fmt_audio_out)(struct file *file, void *fh,
  

•                               struct v4l2_format *f);
  
  /* VIDIOC_TRY_FMT handlers */
  int (*vidioc_try_fmt_vid_cap)(struct file *file, void *fh,
  

@@ -405,6 +435,10 @@ struct v4l2_ioctl_ops { struct v4l2_format *f); int (*vidioc_try_fmt_meta_out)(struct file *file, void *fh, struct v4l2_format *f);

• int (*vidioc_try_fmt_audio_cap)(struct file *file, void *fh,
  

•                                 struct v4l2_format *f);
  

• int (*vidioc_try_fmt_audio_out)(struct file *file, void *fh,
  

•                                 struct v4l2_format *f);
  
  /* Buffer handlers */
  int (*vidioc_reqbufs)(struct file *file, void *fh,
  

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 3af6a82d0cad..e5051410928a 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -153,6 +153,8 @@ enum v4l2_buf_type { V4L2_BUF_TYPE_SDR_OUTPUT = 12, V4L2_BUF_TYPE_META_CAPTURE = 13, V4L2_BUF_TYPE_META_OUTPUT = 14,

• V4L2_BUF_TYPE_AUDIO_CAPTURE        = 15,
  

• V4L2_BUF_TYPE_AUDIO_OUTPUT         = 16,
  /* Deprecated, do not use */
  V4L2_BUF_TYPE_PRIVATE              = 0x80,
  

}; @@ -169,6 +171,7 @@ enum v4l2_buf_type { || (type) == V4L2_BUF_TYPE_VBI_OUTPUT \ || (type) == V4L2_BUF_TYPE_SLICED_VBI_OUTPUT \ || (type) == V4L2_BUF_TYPE_SDR_OUTPUT \

•  || (type) == V4L2_BUF_TYPE_AUDIO_OUTPUT                \
   || (type) == V4L2_BUF_TYPE_META_OUTPUT)
  
  

#define V4L2_TYPE_IS_CAPTURE(type) (!V4L2_TYPE_IS_OUTPUT(type)) @@ -2415,6 +2418,20 @@ struct v4l2_meta_format { __u32 buffersize; } __attribute__ ((packed));

+/**

• • struct v4l2_audio_format - audio data format definition
• • @rate: sample rate
• • @format: sample format
• • @channels: channel numbers
• • @buffersize: maximum size in bytes required for data
• */

+struct v4l2_audio_format {

• __u32                           rate;
  

• __u32                           format;
  

What are the values for the rate and format fields? Since they are part of the UAPI, they need to be defined.

The range for sample rate is [5512, 768000]. The format is defined in include/uapi/sound/asound.h, they are SNDRV_PCM_FORMAT_S8, SNDRV_PCM_FORMAT_U8, ...

Where should I put these info?

Best regards Wang shengjiu

Best regards, Tomasz

...

• __u32                           channels;
  

• __u32                           buffersize;
  

+} __attribute__ ((packed));

/**

• struct v4l2_format - stream data format
• @type: enum v4l2_buf_type; type of the data stream

@@ -2423,6 +2440,7 @@ struct v4l2_meta_format {

• @win: definition of an overlaid image
• @vbi: raw VBI capture or output parameters
• @sliced: sliced VBI capture or output parameters

• • @audio: definition of an audio format
  • @raw_data: placeholder for future extensions and custom formats
  • @fmt: union of @pix, @pix_mp, @win, @vbi, @sliced, @sdr, @meta

  •       and @raw_data
    

@@ -2437,6 +2455,7 @@ struct v4l2_format { struct v4l2_sliced_vbi_format sliced; /* V4L2_BUF_TYPE_SLICED_VBI_CAPTURE */ struct v4l2_sdr_format sdr; /* V4L2_BUF_TYPE_SDR_CAPTURE */ struct v4l2_meta_format meta; /* V4L2_BUF_TYPE_META_CAPTURE */

•         struct v4l2_audio_format        audio;   /* V4L2_BUF_TYPE_AUDIO_CAPTURE */
          __u8    raw_data[200];                   /* user-defined */
  } fmt;
  

};

2.34.1