linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Shengjiu Wang <shengjiu.wang@nxp.com>,
	sakari.ailus@iki.fi, tfiga@chromium.org,
	m.szyprowski@samsung.com, mchehab@kernel.org,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	shengjiu.wang@gmail.com, Xiubo.Lee@gmail.com, festevam@gmail.com,
	nicoleotsuka@gmail.com, lgirdwood@gmail.com, broonie@kernel.org,
	perex@perex.cz, tiwai@suse.com, alsa-devel@alsa-project.org,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [RFC PATCH v8 13/13] media: vim2m_audio: add virtual driver for audio memory to memory
Date: Tue, 7 Nov 2023 10:41:04 +0100	[thread overview]
Message-ID: <0db3d822-9bfa-4efc-bf9d-3ae218b6815d@xs4all.nl> (raw)
In-Reply-To: <c7daf33d-9d6d-499e-b477-35176dbaca38@xs4all.nl>

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@xs4all.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,


  reply	other threads:[~2023-11-07  9:41 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-27 10:35 [RFC PATCH v8 00/13] Add audio support in v4l2 framework Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 01/13] ASoC: fsl_asrc: define functions for memory to memory usage Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 02/13] ASoC: fsl_easrc: " Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 03/13] ASoC: fsl_asrc: move fsl_asrc_common.h to include/sound Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 04/13] ASoC: fsl_asrc: register m2m platform device Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 05/13] ASoC: fsl_easrc: " Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 06/13] media: uapi: Add V4L2_CAP_AUDIO_M2M capability flag Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 07/13] media: v4l2: Add audio capture and output support Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 08/13] media: uapi: Define audio sample format fourcc type Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 09/13] media: uapi: Add V4L2_CTRL_CLASS_M2M_AUDIO Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 10/13] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 11/13] media: uapi: Add audio rate controls support Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 12/13] media: imx-asrc: Add memory to memory driver Shengjiu Wang
2023-10-27 10:35 ` [RFC PATCH v8 13/13] media: vim2m_audio: add virtual driver for audio memory to memory Shengjiu Wang
2023-11-06 13:58   ` Hans Verkuil
2023-11-07  9:41     ` Hans Verkuil [this message]
2023-11-07  9:46       ` [RFC PATCH] v4l-utils: add support for v4l-audioX devices Hans Verkuil
2023-10-27 11:18 ` [RFC PATCH v8 00/13] Add audio support in v4l2 framework Hans Verkuil
2023-10-30  1:56   ` Shengjiu Wang
2023-10-30 11:35     ` Daniel Baluta
2023-11-01 11:05     ` Shengjiu Wang
2023-11-01 11:45       ` Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0db3d822-9bfa-4efc-bf9d-3ae218b6815d@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=Xiubo.Lee@gmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=festevam@gmail.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mchehab@kernel.org \
    --cc=nicoleotsuka@gmail.com \
    --cc=perex@perex.cz \
    --cc=sakari.ailus@iki.fi \
    --cc=shengjiu.wang@gmail.com \
    --cc=shengjiu.wang@nxp.com \
    --cc=tfiga@chromium.org \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).