Re: vb2_queue type question

[Hans Verkuil <hverkuil@xs4all.nl>]

On 3/30/21 10:18 AM, Tomi Valkeinen wrote:
> Hi Hans,
> 
> On 26/03/2021 12:18, Hans Verkuil wrote:
> 
>> 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.

The reason it relied on the format was that the ivtv driver that introduced
this is old and only supports the read() interface. Since you cannot specify a type
when starting streaming that was the only way it could be implemented.

But for modern drivers it would be much better to lock it in when you request
buffers for the first time.

So vivid (the only other driver that supports this) has to be changed to use this
as well.

Regards,

	Hans