Re: vb2_queue type question

On 30/03/2021 11:56, Hans Verkuil wrote:

On 3/30/21 10:18 AM, Tomi Valkeinen wrote:

quoted

Hi Hans,

On 26/03/2021 12:18, Hans Verkuil wrote:

quoted

The only thing that you do is update the queue type when you set the
video or metadata format. When you start allocating buffers the queue type
of the last set format is used. At that moment any attempt to set the format
to another type will fail since vb2_is_busy(queue) will be true.

So only the s_fmt ioctl will change the type. The g/try_fmt ioctls just must
keep working as-is.

I noticed that v4l2-compliance complains about this. It first tests the
format ioctls for both video and metadata buffers, and the last s_fmt is
for metadata. Then it tests buffer ioctls, and reqbufs for video buffers
fails as the queue is in metadata mode, not video mode.

I added a custom .vidioc_reqbufs function to the driver which sets the
queue type and then calls vb2_ioctl_reqbufs normally. This makes
v4l2-compliance pass.

But is that correct change, or should v4l2-compliance be changed?

   Tomi

Good question.

So currently this is something that is rarely used. The few implementations
of this rely on the last set format to decide what the queue type will be.

But is this actually something you want? Wouldn't it be better to rely on the
queue type as passed with VIDIOC_REQBUFS/CREATE_BUFS? That's really the moment
where you lock in the queue type.

To do this you would have to make your own ioctl op:

int my_ioctl_reqbufs(struct file *file, void *priv,
                           struct v4l2_requestbuffers *p)
{
	struct video_device *vdev = video_devdata(file);

	if (p->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
	    p->type != V4L2_BUF_TYPE_META_CAPTURE)
		return -EINVAL;
	if (p->type != vdev->queue.type && vb2_queue_is_busy(vdev, file))
                 return -EBUSY;
	vdev->queue.type = p->type;
	return vb2_ioctl_reqbufs(file, priv, p);
}

And ditto for create_bufs.

I think this makes more sense than relying on the format.

Thanks! This works fine and I agree, it makes more sense.

  Tomi