Question about v4l2-compliance: cap->readbuffers

Question about v4l2-compliance: cap->readbuffers

[Ricardo Ribalda Delgado]

Hello

I am developing a driver for a camera that supports read/write and
mmap access to the buffers.

When I am running the compliance test, I cannot pass it because of
this test on v4l2-test-formats.cpp

904                 if (!(node->caps & V4L2_CAP_READWRITE))
905                         fail_on_test(cap->readbuffers);
906                 else if (node->caps & V4L2_CAP_STREAMING)
907                         fail_on_test(!cap->readbuffers);

What should be the value of cap-readbuffers for a driver such as mine,
that supports cap_readwrite and cap_streaming? Or I cannot support
both, although at least this drivers do the same?


$ git grep CAP_READWRITE *  | grep CAP_STREAMING
pci/cx25821/cx25821-video.c: V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
pci/cx88/cx88-video.c: cap->device_caps = V4L2_CAP_READWRITE |
V4L2_CAP_STREAMING;
pci/saa7134/saa7134-video.c: cap->device_caps = V4L2_CAP_READWRITE |
V4L2_CAP_STREAMING;
platform/marvell-ccic/mcam-core.c: V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
platform/via-camera.c: V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
usb/cx231xx/cx231xx-video.c: cap->device_caps = V4L2_CAP_READWRITE |
V4L2_CAP_STREAMING;
usb/em28xx/em28xx-video.c: V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE
| V4L2_CAP_STREAMING;
usb/stkwebcam/stk-webcam.c: | V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
usb/tlg2300/pd-video.c: V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;


Thanks!

-- 
Ricardo Ribalda

Re: Question about v4l2-compliance: cap->readbuffers

[Hans Verkuil]

On Tue 30 July 2013 15:12:57 Ricardo Ribalda Delgado wrote:
> Hello
> 
> I am developing a driver for a camera that supports read/write and
> mmap access to the buffers.
> 
> When I am running the compliance test, I cannot pass it because of
> this test on v4l2-test-formats.cpp
> 
> 904                 if (!(node->caps & V4L2_CAP_READWRITE))
> 905                         fail_on_test(cap->readbuffers);
> 906                 else if (node->caps & V4L2_CAP_STREAMING)
> 907                         fail_on_test(!cap->readbuffers);
> 
> What should be the value of cap-readbuffers for a driver such as mine,
> that supports cap_readwrite and cap_streaming? Or I cannot support
> both, although at least this drivers do the same?

The readbuffers parameter is highly dubious. I generally set it in a driver
to the lowest number of buffers the hardware allows (e.g. what VIDIOC_REQBUFS
returns if you give it a buffer count of 1).

In theory this value allows you to increase the number of buffers used
by read() so you can have more frames pending. In practice the only driver
using it is sn9c102, which is an obscure webcam that really should be folded
into the gspca driver suite.

I am really tempted to deprecate/obsolete readbuffers.

Anyway, to squash this compliance error just set readbuffers to the lowest
number of allowed buffers.

Regards,

	Hans

> 
> 
> $ git grep CAP_READWRITE *  | grep CAP_STREAMING
> pci/cx25821/cx25821-video.c: V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
> pci/cx88/cx88-video.c: cap->device_caps = V4L2_CAP_READWRITE |
> V4L2_CAP_STREAMING;
> pci/saa7134/saa7134-video.c: cap->device_caps = V4L2_CAP_READWRITE |
> V4L2_CAP_STREAMING;
> platform/marvell-ccic/mcam-core.c: V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
> platform/via-camera.c: V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
> usb/cx231xx/cx231xx-video.c: cap->device_caps = V4L2_CAP_READWRITE |
> V4L2_CAP_STREAMING;
> usb/em28xx/em28xx-video.c: V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE
> | V4L2_CAP_STREAMING;
> usb/stkwebcam/stk-webcam.c: | V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
> usb/tlg2300/pd-video.c: V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
> 
> 
> Thanks!
> 
> 

Re: Question about v4l2-compliance: cap->readbuffers

[Ricardo Ribalda Delgado]

Thanks for the explanation Hans!

I finaly manage to pass that one ;)

Just one more question. Why the compliance test checks if the DISABLED
flag is on on for qctrls?

http://git.linuxtv.org/v4l-utils.git/blob/3ae390e54a0ba627c9e74953081560192b996df4:/utils/v4l2-compliance/v4l2-test-controls.cpp#l137

 137         if (fl & V4L2_CTRL_FLAG_DISABLED)
 138                 return fail("DISABLED flag set\n");

Apparently that has been added on:
http://git.linuxtv.org/v4l-utils.git/commit/0a4d4accea7266d7b5f54dea7ddf46cce8421fbb

But I have failed to find a reason

Thanks again!






On Tue, Jul 30, 2013 at 3:45 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On Tue 30 July 2013 15:12:57 Ricardo Ribalda Delgado wrote:
>> Hello
>>
>> I am developing a driver for a camera that supports read/write and
>> mmap access to the buffers.
>>
>> When I am running the compliance test, I cannot pass it because of
>> this test on v4l2-test-formats.cpp
>>
>> 904                 if (!(node->caps & V4L2_CAP_READWRITE))
>> 905                         fail_on_test(cap->readbuffers);
>> 906                 else if (node->caps & V4L2_CAP_STREAMING)
>> 907                         fail_on_test(!cap->readbuffers);
>>
>> What should be the value of cap-readbuffers for a driver such as mine,
>> that supports cap_readwrite and cap_streaming? Or I cannot support
>> both, although at least this drivers do the same?
>
> The readbuffers parameter is highly dubious. I generally set it in a driver
> to the lowest number of buffers the hardware allows (e.g. what VIDIOC_REQBUFS
> returns if you give it a buffer count of 1).
>
> In theory this value allows you to increase the number of buffers used
> by read() so you can have more frames pending. In practice the only driver
> using it is sn9c102, which is an obscure webcam that really should be folded
> into the gspca driver suite.
>
> I am really tempted to deprecate/obsolete readbuffers.
>
> Anyway, to squash this compliance error just set readbuffers to the lowest
> number of allowed buffers.
>
> Regards,
>
>         Hans
>
>>
>>
>> $ git grep CAP_READWRITE *  | grep CAP_STREAMING
>> pci/cx25821/cx25821-video.c: V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
>> pci/cx88/cx88-video.c: cap->device_caps = V4L2_CAP_READWRITE |
>> V4L2_CAP_STREAMING;
>> pci/saa7134/saa7134-video.c: cap->device_caps = V4L2_CAP_READWRITE |
>> V4L2_CAP_STREAMING;
>> platform/marvell-ccic/mcam-core.c: V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
>> platform/via-camera.c: V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
>> usb/cx231xx/cx231xx-video.c: cap->device_caps = V4L2_CAP_READWRITE |
>> V4L2_CAP_STREAMING;
>> usb/em28xx/em28xx-video.c: V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE
>> | V4L2_CAP_STREAMING;
>> usb/stkwebcam/stk-webcam.c: | V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
>> usb/tlg2300/pd-video.c: V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
>>
>>
>> Thanks!
>>
>>



-- 
Ricardo Ribalda

Re: Question about v4l2-compliance: cap->readbuffers

[Hans Verkuil]

On Tue 30 July 2013 17:18:58 Ricardo Ribalda Delgado wrote:
> Thanks for the explanation Hans!
> 
> I finaly manage to pass that one ;)
> 
> Just one more question. Why the compliance test checks if the DISABLED
> flag is on on for qctrls?
> 
> http://git.linuxtv.org/v4l-utils.git/blob/3ae390e54a0ba627c9e74953081560192b996df4:/utils/v4l2-compliance/v4l2-test-controls.cpp#l137
> 
>  137         if (fl & V4L2_CTRL_FLAG_DISABLED)
>  138                 return fail("DISABLED flag set\n");
> 
> Apparently that has been added on:
> http://git.linuxtv.org/v4l-utils.git/commit/0a4d4accea7266d7b5f54dea7ddf46cce8421fbb
> 
> But I have failed to find a reason

It shouldn't be used anymore in drivers. With the control framework there is
no longer any reason to use the DISABLED flag.

If something has a valid use case for it, then I'd like to know what it is.

Regards,

	Hans

Re: Question about v4l2-compliance: cap->readbuffers

[Ricardo Ribalda Delgado]

Hello

I have a camera that works on two modes: Mono and colour. On color
mode it has 3 gains, on mono mode it has 1 gain.

When the user sets the output to mono I disable the color controls
(and the other way around).

Also on color mode the hflip and vflip do not work, therefore I dont show them.

I could return -EINVAL, but I rather not show the controls to the user.

What would be the proper way to do this?


Thanks gain.





On Tue, Jul 30, 2013 at 5:29 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On Tue 30 July 2013 17:18:58 Ricardo Ribalda Delgado wrote:
>> Thanks for the explanation Hans!
>>
>> I finaly manage to pass that one ;)
>>
>> Just one more question. Why the compliance test checks if the DISABLED
>> flag is on on for qctrls?
>>
>> http://git.linuxtv.org/v4l-utils.git/blob/3ae390e54a0ba627c9e74953081560192b996df4:/utils/v4l2-compliance/v4l2-test-controls.cpp#l137
>>
>>  137         if (fl & V4L2_CTRL_FLAG_DISABLED)
>>  138                 return fail("DISABLED flag set\n");
>>
>> Apparently that has been added on:
>> http://git.linuxtv.org/v4l-utils.git/commit/0a4d4accea7266d7b5f54dea7ddf46cce8421fbb
>>
>> But I have failed to find a reason
>
> It shouldn't be used anymore in drivers. With the control framework there is
> no longer any reason to use the DISABLED flag.
>
> If something has a valid use case for it, then I'd like to know what it is.
>
> Regards,
>
>         Hans



-- 
Ricardo Ribalda

Re: Question about v4l2-compliance: cap->readbuffers

[Hans Verkuil]

Hi Ricardo,

On 07/30/2013 05:46 PM, Ricardo Ribalda Delgado wrote:
> Hello
> 
> I have a camera that works on two modes: Mono and colour. On color
> mode it has 3 gains, on mono mode it has 1 gain.
> 
> When the user sets the output to mono I disable the color controls
> (and the other way around).
> 
> Also on color mode the hflip and vflip do not work, therefore I dont show them.
> 
> I could return -EINVAL, but I rather not show the controls to the user.
> 
> What would be the proper way to do this?

Use the INACTIVE flag, that's the way it is typically done. You can still set
such controls, but the new value won't be active until you switch back to a
mode where they do work.

Using INACTIVE will show such controls as disabled in a GUI like qv4l2. I highly
recommend using qv4l2 for testing this since it is the reference implementation
of how GUIs should interpret control flags.

Regards,

	Hans

> 
> 
> Thanks gain.
> 
> 
> 
> 
> 
> On Tue, Jul 30, 2013 at 5:29 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> On Tue 30 July 2013 17:18:58 Ricardo Ribalda Delgado wrote:
>>> Thanks for the explanation Hans!
>>>
>>> I finaly manage to pass that one ;)
>>>
>>> Just one more question. Why the compliance test checks if the DISABLED
>>> flag is on on for qctrls?
>>>
>>> http://git.linuxtv.org/v4l-utils.git/blob/3ae390e54a0ba627c9e74953081560192b996df4:/utils/v4l2-compliance/v4l2-test-controls.cpp#l137
>>>
>>>  137         if (fl & V4L2_CTRL_FLAG_DISABLED)
>>>  138                 return fail("DISABLED flag set\n");
>>>
>>> Apparently that has been added on:
>>> http://git.linuxtv.org/v4l-utils.git/commit/0a4d4accea7266d7b5f54dea7ddf46cce8421fbb
>>>
>>> But I have failed to find a reason
>>
>> It shouldn't be used anymore in drivers. With the control framework there is
>> no longer any reason to use the DISABLED flag.
>>
>> If something has a valid use case for it, then I'd like to know what it is.
>>
>> Regards,
>>
>>         Hans
> 
> 
> 

Re: Question about v4l2-compliance: cap->readbuffers

[Ricardo Ribalda Delgado]

Hello Hans

Thanks for the explanation. I have tried changing the controls to
inactive and the are shown disabled on the gui, just as you say, but
they are there :S. I personally liked better the previous behaviour,
when the controls where not shown at all. But it is just that, a
taste, if it is more correct showing them as inactive they will be
inactive :).

For a case where a option is only available at a specific format: I
was also disablig the control (now inactiving it), and returning
-EINVAL if the user tried to set the control on an incompatible
format. Apparently the v4l2-compilance dont like that either, is this
a false positive or I should behave differently?.

Thank you again!




On Tue, Jul 30, 2013 at 6:17 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Hi Ricardo,
>
> On 07/30/2013 05:46 PM, Ricardo Ribalda Delgado wrote:
>> Hello
>>
>> I have a camera that works on two modes: Mono and colour. On color
>> mode it has 3 gains, on mono mode it has 1 gain.
>>
>> When the user sets the output to mono I disable the color controls
>> (and the other way around).
>>
>> Also on color mode the hflip and vflip do not work, therefore I dont show them.
>>
>> I could return -EINVAL, but I rather not show the controls to the user.
>>
>> What would be the proper way to do this?
>
> Use the INACTIVE flag, that's the way it is typically done. You can still set
> such controls, but the new value won't be active until you switch back to a
> mode where they do work.
>
> Using INACTIVE will show such controls as disabled in a GUI like qv4l2. I highly
> recommend using qv4l2 for testing this since it is the reference implementation
> of how GUIs should interpret control flags.
>
> Regards,
>
>         Hans
>
>>
>>
>> Thanks gain.
>>
>>
>>
>>
>>
>> On Tue, Jul 30, 2013 at 5:29 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>> On Tue 30 July 2013 17:18:58 Ricardo Ribalda Delgado wrote:
>>>> Thanks for the explanation Hans!
>>>>
>>>> I finaly manage to pass that one ;)
>>>>
>>>> Just one more question. Why the compliance test checks if the DISABLED
>>>> flag is on on for qctrls?
>>>>
>>>> http://git.linuxtv.org/v4l-utils.git/blob/3ae390e54a0ba627c9e74953081560192b996df4:/utils/v4l2-compliance/v4l2-test-controls.cpp#l137
>>>>
>>>>  137         if (fl & V4L2_CTRL_FLAG_DISABLED)
>>>>  138                 return fail("DISABLED flag set\n");
>>>>
>>>> Apparently that has been added on:
>>>> http://git.linuxtv.org/v4l-utils.git/commit/0a4d4accea7266d7b5f54dea7ddf46cce8421fbb
>>>>
>>>> But I have failed to find a reason
>>>
>>> It shouldn't be used anymore in drivers. With the control framework there is
>>> no longer any reason to use the DISABLED flag.
>>>
>>> If something has a valid use case for it, then I'd like to know what it is.
>>>
>>> Regards,
>>>
>>>         Hans
>>
>>
>>



-- 
Ricardo Ribalda

Re: Question about v4l2-compliance: cap->readbuffers

[Hans Verkuil]

On Wed 31 July 2013 09:09:05 Ricardo Ribalda Delgado wrote:
> Hello Hans
> 
> Thanks for the explanation. I have tried changing the controls to
> inactive and the are shown disabled on the gui, just as you say, but
> they are there :S. I personally liked better the previous behaviour,
> when the controls where not shown at all. But it is just that, a
> taste, if it is more correct showing them as inactive they will be
> inactive :).

The problem with removing them dynamically is that that means that a GUI
also has to refresh itself whenever that happens. That's an expensive
operation and that generally also means that the layout of the controls
changes, which is very confusing for the user.

Note that the DISABLED flag is meant for situations where a control was
simply invalid for the particular hardware version, i.e. the flag was
static and would never change. With the control framework it is much
easier to just not add the control in the first place.

> For a case where a option is only available at a specific format: I
> was also disablig the control (now inactiving it), and returning
> -EINVAL if the user tried to set the control on an incompatible
> format. Apparently the v4l2-compilance dont like that either, is this
> a false positive or I should behave differently?.

The general approach in a driver is that you can still set an inactive
control. The new value will simply be stored and only becomes active
when the control becomes active as well. In some cases that doesn't
make sense, and in that case the driver will just ignore the new value,
but it still returns 0.

There are good reasons for this behavior: it allows you to get the values
of all controls (active and inactive) and set them as well. This is useful
for a program that saves the state of controls and restores them on startup.

Regards,

	Hans

> 
> Thank you again!
> 
> 
> 
> 
> On Tue, Jul 30, 2013 at 6:17 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> > Hi Ricardo,
> >
> > On 07/30/2013 05:46 PM, Ricardo Ribalda Delgado wrote:
> >> Hello
> >>
> >> I have a camera that works on two modes: Mono and colour. On color
> >> mode it has 3 gains, on mono mode it has 1 gain.
> >>
> >> When the user sets the output to mono I disable the color controls
> >> (and the other way around).
> >>
> >> Also on color mode the hflip and vflip do not work, therefore I dont show them.
> >>
> >> I could return -EINVAL, but I rather not show the controls to the user.
> >>
> >> What would be the proper way to do this?
> >
> > Use the INACTIVE flag, that's the way it is typically done. You can still set
> > such controls, but the new value won't be active until you switch back to a
> > mode where they do work.
> >
> > Using INACTIVE will show such controls as disabled in a GUI like qv4l2. I highly
> > recommend using qv4l2 for testing this since it is the reference implementation
> > of how GUIs should interpret control flags.
> >
> > Regards,
> >
> >         Hans
> >
> >>
> >>
> >> Thanks gain.
> >>
> >>
> >>
> >>
> >>
> >> On Tue, Jul 30, 2013 at 5:29 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >>> On Tue 30 July 2013 17:18:58 Ricardo Ribalda Delgado wrote:
> >>>> Thanks for the explanation Hans!
> >>>>
> >>>> I finaly manage to pass that one ;)
> >>>>
> >>>> Just one more question. Why the compliance test checks if the DISABLED
> >>>> flag is on on for qctrls?
> >>>>
> >>>> http://git.linuxtv.org/v4l-utils.git/blob/3ae390e54a0ba627c9e74953081560192b996df4:/utils/v4l2-compliance/v4l2-test-controls.cpp#l137
> >>>>
> >>>>  137         if (fl & V4L2_CTRL_FLAG_DISABLED)
> >>>>  138                 return fail("DISABLED flag set\n");
> >>>>
> >>>> Apparently that has been added on:
> >>>> http://git.linuxtv.org/v4l-utils.git/commit/0a4d4accea7266d7b5f54dea7ddf46cce8421fbb
> >>>>
> >>>> But I have failed to find a reason
> >>>
> >>> It shouldn't be used anymore in drivers. With the control framework there is
> >>> no longer any reason to use the DISABLED flag.
> >>>
> >>> If something has a valid use case for it, then I'd like to know what it is.
> >>>
> >>> Regards,
> >>>
> >>>         Hans
> >>
> >>
> >>
> 
> 
> 
>