vb2_queue type question

vb2_queue type question

[Tomi Valkeinen]

Hi Hans,

We were discussing this with Laurent and Sakari, I thought I'd ask if 
you have any feedback on this.

struct vb2_queue has 'type' field, so you can only use a queue for 
buffers of a single type. struct video_device has 'queue' field, so you 
can only use a single queue for a video_device instance.

TI's SoCs have a CSI-2 receiver, with a bunch of DMA engines. The HW 
doesn't care if we are currently capturing pixel buffers or metadata 
buffers (I don't have experience with other HW, but I imagine this 
shouldn't be a rare case). However, due to vb2_queue, the driver needs 
to decide which one to support, which limits the possible use cases.

I was browsing the code, and afaics the type field doesn't do much. It 
is, of course, used to reject queuing buffers of wrong type, and also 
(mostly in mem-2-mem code) to find out if functions are called in input 
or output context.

The latter one could be easily removed by just comparing the given queue 
pointer to a stored pointer (e.g. queue == priv->input_queue).

Do you see any problems if we were to change the type field to 
type_mask, allowing multiple buffer types per queue? Or even remove the 
vb2_queue->type. This raises some questions, like should a queue contain 
only buffers of a single type or can it contain a mix of buffers (I 
think it shouldn't contain a mix of buffers), or can a queue's type_mask 
contain both input and output types (I don't see why not).

An alternate which I tried was creating two vb2_queues, and switching 
the video_device->queue at runtime based on set_format. It kind of 
works, but I think the behavior is a bit unclear, and it might be 
difficult to catch all the corner cases.

  Tomi

Re: vb2_queue type question

[Hans Verkuil]

On 22/03/2021 10:18, Tomi Valkeinen wrote:
> Hi Hans,
> 
> We were discussing this with Laurent and Sakari, I thought I'd ask if 
> you have any feedback on this.
> 
> struct vb2_queue has 'type' field, so you can only use a queue for 
> buffers of a single type. struct video_device has 'queue' field, so you 
> can only use a single queue for a video_device instance.
> 
> TI's SoCs have a CSI-2 receiver, with a bunch of DMA engines. The HW 
> doesn't care if we are currently capturing pixel buffers or metadata 
> buffers (I don't have experience with other HW, but I imagine this 
> shouldn't be a rare case). However, due to vb2_queue, the driver needs 
> to decide which one to support, which limits the possible use cases.
> 
> I was browsing the code, and afaics the type field doesn't do much. It 
> is, of course, used to reject queuing buffers of wrong type, and also 
> (mostly in mem-2-mem code) to find out if functions are called in input 
> or output context.
> 
> The latter one could be easily removed by just comparing the given queue 
> pointer to a stored pointer (e.g. queue == priv->input_queue).
> 
> Do you see any problems if we were to change the type field to 
> type_mask, allowing multiple buffer types per queue? Or even remove the 
> vb2_queue->type. This raises some questions, like should a queue contain 
> only buffers of a single type or can it contain a mix of buffers (I 
> think it shouldn't contain a mix of buffers), or can a queue's type_mask 
> contain both input and output types (I don't see why not).
> 
> An alternate which I tried was creating two vb2_queues, and switching 
> the video_device->queue at runtime based on set_format. It kind of 
> works, but I think the behavior is a bit unclear, and it might be 
> difficult to catch all the corner cases.

A vb2_queue basically represents a buffer queue that will be fed to a
DMA engine. It assumes that all the buffers are of the same format,
which typically is tied directly to the type.

The type of a vb2_queue can be changed if you like, but once buffers
are allocated it is fixed and can't be changed again until all buffers
are released. So you can't mix buffers of different types.

This is actually done in the vivid driver: see vidioc_s_fmt_vbi_cap()
and vidioc_s_fmt_sliced_vbi_cap(): depending on the format the queue
type will be set to either capture raw or sliced VBI.

The ivtv driver does the same thing.

So as long as vb2_is_busy() returns false, you are free to change the
queue type.

There is no need for a type_mask or anything like that. That's up to
the bridge driver to check. The vb2_queue type is there to ensure that
userspace isn't trying to mix buffers of different types, but as long
as no buffers are allocated it doesn't do anything and you are free to
change it.

Regards,

	Hans

Re: vb2_queue type question

[Laurent Pinchart]

Hi Hans,

On Mon, Mar 22, 2021 at 10:49:26AM +0100, Hans Verkuil wrote:
> On 22/03/2021 10:18, Tomi Valkeinen wrote:
> > Hi Hans,
> > 
> > We were discussing this with Laurent and Sakari, I thought I'd ask if 
> > you have any feedback on this.
> > 
> > struct vb2_queue has 'type' field, so you can only use a queue for 
> > buffers of a single type. struct video_device has 'queue' field, so you 
> > can only use a single queue for a video_device instance.
> > 
> > TI's SoCs have a CSI-2 receiver, with a bunch of DMA engines. The HW 
> > doesn't care if we are currently capturing pixel buffers or metadata 
> > buffers (I don't have experience with other HW, but I imagine this 
> > shouldn't be a rare case). However, due to vb2_queue, the driver needs 
> > to decide which one to support, which limits the possible use cases.
> > 
> > I was browsing the code, and afaics the type field doesn't do much. It 
> > is, of course, used to reject queuing buffers of wrong type, and also 
> > (mostly in mem-2-mem code) to find out if functions are called in input 
> > or output context.
> > 
> > The latter one could be easily removed by just comparing the given queue 
> > pointer to a stored pointer (e.g. queue == priv->input_queue).
> > 
> > Do you see any problems if we were to change the type field to 
> > type_mask, allowing multiple buffer types per queue? Or even remove the 
> > vb2_queue->type. This raises some questions, like should a queue contain 
> > only buffers of a single type or can it contain a mix of buffers (I 
> > think it shouldn't contain a mix of buffers), or can a queue's type_mask 
> > contain both input and output types (I don't see why not).
> > 
> > An alternate which I tried was creating two vb2_queues, and switching 
> > the video_device->queue at runtime based on set_format. It kind of 
> > works, but I think the behavior is a bit unclear, and it might be 
> > difficult to catch all the corner cases.
> 
> A vb2_queue basically represents a buffer queue that will be fed to a
> DMA engine. It assumes that all the buffers are of the same format,
> which typically is tied directly to the type.
> 
> The type of a vb2_queue can be changed if you like, but once buffers
> are allocated it is fixed and can't be changed again until all buffers
> are released. So you can't mix buffers of different types.
> 
> This is actually done in the vivid driver: see vidioc_s_fmt_vbi_cap()
> and vidioc_s_fmt_sliced_vbi_cap(): depending on the format the queue
> type will be set to either capture raw or sliced VBI.
> 
> The ivtv driver does the same thing.
> 
> So as long as vb2_is_busy() returns false, you are free to change the
> queue type.
> 
> There is no need for a type_mask or anything like that. That's up to
> the bridge driver to check. The vb2_queue type is there to ensure that
> userspace isn't trying to mix buffers of different types, but as long
> as no buffers are allocated it doesn't do anything and you are free to
> change it.

I wasn't aware of this design rationale. It would be useful to expand
the documentation of vb2_queue.type to document this. Or have I missed a
different location where this is already explained ?

-- 
Regards,

Laurent Pinchart

Re: vb2_queue type question

[Hans Verkuil]

On 22/03/2021 10:59, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Mon, Mar 22, 2021 at 10:49:26AM +0100, Hans Verkuil wrote:
>> On 22/03/2021 10:18, Tomi Valkeinen wrote:
>>> Hi Hans,
>>>
>>> We were discussing this with Laurent and Sakari, I thought I'd ask if 
>>> you have any feedback on this.
>>>
>>> struct vb2_queue has 'type' field, so you can only use a queue for 
>>> buffers of a single type. struct video_device has 'queue' field, so you 
>>> can only use a single queue for a video_device instance.
>>>
>>> TI's SoCs have a CSI-2 receiver, with a bunch of DMA engines. The HW 
>>> doesn't care if we are currently capturing pixel buffers or metadata 
>>> buffers (I don't have experience with other HW, but I imagine this 
>>> shouldn't be a rare case). However, due to vb2_queue, the driver needs 
>>> to decide which one to support, which limits the possible use cases.
>>>
>>> I was browsing the code, and afaics the type field doesn't do much. It 
>>> is, of course, used to reject queuing buffers of wrong type, and also 
>>> (mostly in mem-2-mem code) to find out if functions are called in input 
>>> or output context.
>>>
>>> The latter one could be easily removed by just comparing the given queue 
>>> pointer to a stored pointer (e.g. queue == priv->input_queue).
>>>
>>> Do you see any problems if we were to change the type field to 
>>> type_mask, allowing multiple buffer types per queue? Or even remove the 
>>> vb2_queue->type. This raises some questions, like should a queue contain 
>>> only buffers of a single type or can it contain a mix of buffers (I 
>>> think it shouldn't contain a mix of buffers), or can a queue's type_mask 
>>> contain both input and output types (I don't see why not).
>>>
>>> An alternate which I tried was creating two vb2_queues, and switching 
>>> the video_device->queue at runtime based on set_format. It kind of 
>>> works, but I think the behavior is a bit unclear, and it might be 
>>> difficult to catch all the corner cases.
>>
>> A vb2_queue basically represents a buffer queue that will be fed to a
>> DMA engine. It assumes that all the buffers are of the same format,
>> which typically is tied directly to the type.
>>
>> The type of a vb2_queue can be changed if you like, but once buffers
>> are allocated it is fixed and can't be changed again until all buffers
>> are released. So you can't mix buffers of different types.
>>
>> This is actually done in the vivid driver: see vidioc_s_fmt_vbi_cap()
>> and vidioc_s_fmt_sliced_vbi_cap(): depending on the format the queue
>> type will be set to either capture raw or sliced VBI.
>>
>> The ivtv driver does the same thing.
>>
>> So as long as vb2_is_busy() returns false, you are free to change the
>> queue type.
>>
>> There is no need for a type_mask or anything like that. That's up to
>> the bridge driver to check. The vb2_queue type is there to ensure that
>> userspace isn't trying to mix buffers of different types, but as long
>> as no buffers are allocated it doesn't do anything and you are free to
>> change it.
> 
> I wasn't aware of this design rationale. It would be useful to expand
> the documentation of vb2_queue.type to document this. Or have I missed a
> different location where this is already explained ?
> 

It's probably not explained anywhere. It such a rare corner case that nobody
bothered. If this is going to be used more often, then I think it would be
a good idea to make a proper vb2 helper function through which you can change
the type, and that will also check with vb2_is_busy() if you are allowed to do
that.

Then that function can be documented in the header.

Changing types may also require you to change other vb2_queue fields as well,
but that's the responsibility of the bridge driver as well. For this particular
use-case I would expect that only the type field requires changing. Everything
else would stay the same, except perhaps for the vb2_ops.

Regards,

	Hans

Re: vb2_queue type question

[Tomi Valkeinen]

On 22/03/2021 11:49, Hans Verkuil wrote:
> On 22/03/2021 10:18, Tomi Valkeinen wrote:
>> Hi Hans,
>>
>> We were discussing this with Laurent and Sakari, I thought I'd ask if
>> you have any feedback on this.
>>
>> struct vb2_queue has 'type' field, so you can only use a queue for
>> buffers of a single type. struct video_device has 'queue' field, so you
>> can only use a single queue for a video_device instance.
>>
>> TI's SoCs have a CSI-2 receiver, with a bunch of DMA engines. The HW
>> doesn't care if we are currently capturing pixel buffers or metadata
>> buffers (I don't have experience with other HW, but I imagine this
>> shouldn't be a rare case). However, due to vb2_queue, the driver needs
>> to decide which one to support, which limits the possible use cases.
>>
>> I was browsing the code, and afaics the type field doesn't do much. It
>> is, of course, used to reject queuing buffers of wrong type, and also
>> (mostly in mem-2-mem code) to find out if functions are called in input
>> or output context.
>>
>> The latter one could be easily removed by just comparing the given queue
>> pointer to a stored pointer (e.g. queue == priv->input_queue).
>>
>> Do you see any problems if we were to change the type field to
>> type_mask, allowing multiple buffer types per queue? Or even remove the
>> vb2_queue->type. This raises some questions, like should a queue contain
>> only buffers of a single type or can it contain a mix of buffers (I
>> think it shouldn't contain a mix of buffers), or can a queue's type_mask
>> contain both input and output types (I don't see why not).
>>
>> An alternate which I tried was creating two vb2_queues, and switching
>> the video_device->queue at runtime based on set_format. It kind of
>> works, but I think the behavior is a bit unclear, and it might be
>> difficult to catch all the corner cases.
> 
> A vb2_queue basically represents a buffer queue that will be fed to a
> DMA engine. It assumes that all the buffers are of the same format,
> which typically is tied directly to the type.
> 
> The type of a vb2_queue can be changed if you like, but once buffers
> are allocated it is fixed and can't be changed again until all buffers
> are released. So you can't mix buffers of different types.
> 
> This is actually done in the vivid driver: see vidioc_s_fmt_vbi_cap()
> and vidioc_s_fmt_sliced_vbi_cap(): depending on the format the queue
> type will be set to either capture raw or sliced VBI.
> 
> The ivtv driver does the same thing.
> 
> So as long as vb2_is_busy() returns false, you are free to change the
> queue type.
> 
> There is no need for a type_mask or anything like that. That's up to
> the bridge driver to check. The vb2_queue type is there to ensure that
> userspace isn't trying to mix buffers of different types, but as long
> as no buffers are allocated it doesn't do anything and you are free to
> change it.

Thanks, this works fine and is simple to manage.

  Tomi

Re: vb2_queue type question

[Tomi Valkeinen]

Hi Hans,

On 22/03/2021 11:49, Hans Verkuil wrote:
> On 22/03/2021 10:18, Tomi Valkeinen wrote:
>> Hi Hans,
>>
>> We were discussing this with Laurent and Sakari, I thought I'd ask if
>> you have any feedback on this.
>>
>> struct vb2_queue has 'type' field, so you can only use a queue for
>> buffers of a single type. struct video_device has 'queue' field, so you
>> can only use a single queue for a video_device instance.
>>
>> TI's SoCs have a CSI-2 receiver, with a bunch of DMA engines. The HW
>> doesn't care if we are currently capturing pixel buffers or metadata
>> buffers (I don't have experience with other HW, but I imagine this
>> shouldn't be a rare case). However, due to vb2_queue, the driver needs
>> to decide which one to support, which limits the possible use cases.
>>
>> I was browsing the code, and afaics the type field doesn't do much. It
>> is, of course, used to reject queuing buffers of wrong type, and also
>> (mostly in mem-2-mem code) to find out if functions are called in input
>> or output context.
>>
>> The latter one could be easily removed by just comparing the given queue
>> pointer to a stored pointer (e.g. queue == priv->input_queue).
>>
>> Do you see any problems if we were to change the type field to
>> type_mask, allowing multiple buffer types per queue? Or even remove the
>> vb2_queue->type. This raises some questions, like should a queue contain
>> only buffers of a single type or can it contain a mix of buffers (I
>> think it shouldn't contain a mix of buffers), or can a queue's type_mask
>> contain both input and output types (I don't see why not).
>>
>> An alternate which I tried was creating two vb2_queues, and switching
>> the video_device->queue at runtime based on set_format. It kind of
>> works, but I think the behavior is a bit unclear, and it might be
>> difficult to catch all the corner cases.
> 
> A vb2_queue basically represents a buffer queue that will be fed to a
> DMA engine. It assumes that all the buffers are of the same format,
> which typically is tied directly to the type.
> 
> The type of a vb2_queue can be changed if you like, but once buffers
> are allocated it is fixed and can't be changed again until all buffers
> are released. So you can't mix buffers of different types.
> 
> This is actually done in the vivid driver: see vidioc_s_fmt_vbi_cap()
> and vidioc_s_fmt_sliced_vbi_cap(): depending on the format the queue
> type will be set to either capture raw or sliced VBI.
> 
> The ivtv driver does the same thing.
> 
> So as long as vb2_is_busy() returns false, you are free to change the
> queue type.

So what's the expected behavior here if I have both normal video ops 
(vidioc_g_fmt_vid_cap & co.) and metadata ops (vidioc_g_fmt_meta_cap & 
co.) defined?

I can change the queue type in s_fmt as you suggest above, but what 
should, say, vidioc_g_fmt_meta_cap return if the current format is not 
metadata format? I made it return -EINVAL, but then v4l2-compliance says 
"Metadata Capture G_FMT failed, but Metadata Capture formats defined". I 
could also make vidioc_enum_fmt_meta_cap return an error, but then it 
would look like no metadata is supported.

Should all the metadata ops always change the queue type? That would 
prevent g_fmt from working when the queue is busy.

  Tomi

Re: vb2_queue type question

[Hans Verkuil]

On 26/03/2021 10:57, Tomi Valkeinen wrote:
> Hi Hans,
> 
> On 22/03/2021 11:49, Hans Verkuil wrote:
>> On 22/03/2021 10:18, Tomi Valkeinen wrote:
>>> Hi Hans,
>>>
>>> We were discussing this with Laurent and Sakari, I thought I'd ask if
>>> you have any feedback on this.
>>>
>>> struct vb2_queue has 'type' field, so you can only use a queue for
>>> buffers of a single type. struct video_device has 'queue' field, so you
>>> can only use a single queue for a video_device instance.
>>>
>>> TI's SoCs have a CSI-2 receiver, with a bunch of DMA engines. The HW
>>> doesn't care if we are currently capturing pixel buffers or metadata
>>> buffers (I don't have experience with other HW, but I imagine this
>>> shouldn't be a rare case). However, due to vb2_queue, the driver needs
>>> to decide which one to support, which limits the possible use cases.
>>>
>>> I was browsing the code, and afaics the type field doesn't do much. It
>>> is, of course, used to reject queuing buffers of wrong type, and also
>>> (mostly in mem-2-mem code) to find out if functions are called in input
>>> or output context.
>>>
>>> The latter one could be easily removed by just comparing the given queue
>>> pointer to a stored pointer (e.g. queue == priv->input_queue).
>>>
>>> Do you see any problems if we were to change the type field to
>>> type_mask, allowing multiple buffer types per queue? Or even remove the
>>> vb2_queue->type. This raises some questions, like should a queue contain
>>> only buffers of a single type or can it contain a mix of buffers (I
>>> think it shouldn't contain a mix of buffers), or can a queue's type_mask
>>> contain both input and output types (I don't see why not).
>>>
>>> An alternate which I tried was creating two vb2_queues, and switching
>>> the video_device->queue at runtime based on set_format. It kind of
>>> works, but I think the behavior is a bit unclear, and it might be
>>> difficult to catch all the corner cases.
>>
>> A vb2_queue basically represents a buffer queue that will be fed to a
>> DMA engine. It assumes that all the buffers are of the same format,
>> which typically is tied directly to the type.
>>
>> The type of a vb2_queue can be changed if you like, but once buffers
>> are allocated it is fixed and can't be changed again until all buffers
>> are released. So you can't mix buffers of different types.
>>
>> This is actually done in the vivid driver: see vidioc_s_fmt_vbi_cap()
>> and vidioc_s_fmt_sliced_vbi_cap(): depending on the format the queue
>> type will be set to either capture raw or sliced VBI.
>>
>> The ivtv driver does the same thing.
>>
>> So as long as vb2_is_busy() returns false, you are free to change the
>> queue type.
> 
> So what's the expected behavior here if I have both normal video ops 
> (vidioc_g_fmt_vid_cap & co.) and metadata ops (vidioc_g_fmt_meta_cap & 
> co.) defined?
> 
> I can change the queue type in s_fmt as you suggest above, but what 
> should, say, vidioc_g_fmt_meta_cap return if the current format is not 
> metadata format? I made it return -EINVAL, but then v4l2-compliance says 
> "Metadata Capture G_FMT failed, but Metadata Capture formats defined". I 
> could also make vidioc_enum_fmt_meta_cap return an error, but then it 
> would look like no metadata is supported.

No, the format ioctls are not changed.

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.

Regards,

	Hans

> 
> Should all the metadata ops always change the queue type? That would 
> prevent g_fmt from working when the queue is busy.
> 
>   Tomi
> 

Re: vb2_queue type question

[Tomi Valkeinen]

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

Re: vb2_queue type question

[Hans Verkuil]

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

Re: vb2_queue type question

[Tomi Valkeinen]

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