Re: vb2_queue type question

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

On 30/03/2021 11:56, Hans Verkuil wrote:
> 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.

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

  Tomi