linux-kernel - Re: [RFC PATCH v8 13/13] media: vim2m_audio: add virtual driver for audio memory to memory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0db3d822-9bfa-4efc-bf9d-3ae218b6815d@xs4all.nl>
Date:   Tue, 7 Nov 2023 10:41:04 +0100
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Shengjiu Wang <shengjiu.wang@....com>, sakari.ailus@....fi,
        tfiga@...omium.org, m.szyprowski@...sung.com, mchehab@...nel.org,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        shengjiu.wang@...il.com, Xiubo.Lee@...il.com, festevam@...il.com,
        nicoleotsuka@...il.com, lgirdwood@...il.com, broonie@...nel.org,
        perex@...ex.cz, tiwai@...e.com, alsa-devel@...a-project.org,
        linuxppc-dev@...ts.ozlabs.org
Subject: Re: [RFC PATCH v8 13/13] media: vim2m_audio: add virtual driver for
 audio memory to memory

On 06/11/2023 14:58, Hans Verkuil wrote:
> On 27/10/2023 12:35, Shengjiu Wang wrote:
>> Audio memory to memory virtual driver use video memory to memory
>> virtual driver vim2m.c as example. The main difference is
>> device type is VFL_TYPE_AUDIO and device cap type is V4L2_CAP_AUDIO_M2M.
>>
>> The device_run function is a dummy function, which is simply
>> copy the data from input buffer to output buffer.
> 
> I started work on the v4l-utils part of this, using this driver.
> 
> I noticed that this driver doesn't expose the V4L2_CID_M2M_AUDIO_SOURCE/SINK_RATE
> controls, and it really should, otherwise it is not representative of this
> type of device.
> 
> It is enough to start with just a single fixed rate listed for each control.
> 
> It would be even nicer if you can have two rates such as 24000 and 48000 and
> do the actual rate conversion, i.e. dropping every other sample or duplicating
> each sample depending on whether you're halving or doubling the rate. That
> should be easy to implement, and it makes this driver much more realistic.

Update: I have finished the v4l-utils update (I'll post a patch for that later).

But while testing I noticed that this driver does not set up the sequence number
and it doesn't copy the timestamp. So the patch below needs to be applied.

Just squash it together with your patch. Note that you need to do the same for
your alsa driver.

Also, please rename the source name from vim2m_audio.c to vim2m-audio.c. That is
consistent with the naming elsewhere in test-drivers.

I also want to have support for the MEDIA_CONTROLLER here. See vim2m, search for
CONFIG_MEDIA_CONTROLLER. Both in this test driver and also in your audio driver.

This will require adding a new media entity (MEDIA_ENT_F_PROC_AUDIO_RESAMPLER?).
And you also need to add a new MEDIA_INTF_T_V4L_AUDIO interface type that will be
used by v4l2_m2m_register_media_controller(). That function can check vdev->vfl_type
to see if it needs to use MEDIA_INTF_T_V4L_VIDEO or MEDIA_INTF_T_V4L_AUDIO.
Remember to update the documentation as well!

The reason for using the media controller here is that it turns out to be very useful
for application to detect what sort of m2m device it is dealing with: it has proven
it worth for video codecs, and I think it should be standard for new m2m devices, and
especially for a completely new type of m2m device.

Regards,

	Hans

Signed-off-by: Hans Verkuil <hverkuil-cisco@...all.nl>
---
diff --git a/drivers/media/test-drivers/vim2m_audio.c b/drivers/media/test-drivers/vim2m_audio.c
index 2134e8338417..e8aa2bb0aa77 100644
--- a/drivers/media/test-drivers/vim2m_audio.c
+++ b/drivers/media/test-drivers/vim2m_audio.c
@@ -62,6 +62,7 @@ struct audm2m_q_data {
 	unsigned int		channels;
 	unsigned int		buffersize;
 	u32			fourcc;
+	unsigned int		sequence;
 };

 enum {
@@ -170,6 +171,9 @@ static void device_run(void *priv)

 	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
 	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+	src_buf->sequence = q_data_src->sequence++;
+	dst_buf->sequence = q_data_dst->sequence++;
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);

 	v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE);
 	v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE);
@@ -423,6 +427,15 @@ static void audm2m_buf_queue(struct vb2_buffer *vb)
 	v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
 }

+static int audm2m_start_streaming(struct vb2_queue *q, unsigned int count)
+{
+	struct audm2m_ctx *ctx = vb2_get_drv_priv(q);
+	struct audm2m_q_data *q_data = get_q_data(ctx, q->type);
+
+	q_data->sequence = 0;
+	return 0;
+}
+
 static void audm2m_stop_streaming(struct vb2_queue *q)
 {
 	struct audm2m_ctx *ctx = vb2_get_drv_priv(q);
@@ -442,6 +455,7 @@ static void audm2m_stop_streaming(struct vb2_queue *q)
 static const struct vb2_ops audm2m_qops = {
 	.queue_setup	 = audm2m_queue_setup,
 	.buf_queue	 = audm2m_buf_queue,
+	.start_streaming  = audm2m_start_streaming,
 	.stop_streaming  = audm2m_stop_streaming,
 	.wait_prepare	 = vb2_ops_wait_prepare,
 	.wait_finish	 = vb2_ops_wait_finish,