On 22/09/17 18:31, Dave Stevenson wrote:
> On 22 September 2017 at 12:00, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>> On 13/09/17 17:49, Dave Stevenson wrote:
>>> OV5647
>>>
>>> v4l2-compliance SHA : f6ecbc90656815d91dc6ba90aac0ad8193a14b38
>>>
>>> Driver Info:
>>> Driver name : unicam
>>> Card type : unicam
>>> Bus info : platform:unicam 3f801000.csi1
>>> Driver version: 4.13.0
>>> Capabilities : 0x85200001
>>> Video Capture
>>> Read/Write
>>> Streaming
>>> Extended Pix Format
>>> Device Capabilities
>>> Device Caps : 0x05200001
>>> Video Capture
>>> Read/Write
>>> Streaming
>>> Extended Pix Format
>>>
>>> Compliance test for device /dev/video0 (not using libv4l2):
>>>
>>> Required ioctls:
>>> test VIDIOC_QUERYCAP: OK
>>>
>>> Allow for multiple opens:
>>> test second video open: OK
>>> test VIDIOC_QUERYCAP: OK
>>> test VIDIOC_G/S_PRIORITY: OK
>>> test for unlimited opens: OK
>>>
>>> Debug ioctls:
>>> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
>>> test VIDIOC_LOG_STATUS: OK
>>>
>>> Input ioctls:
>>> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>>> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>>> test VIDIOC_ENUMAUDIO: OK (Not Supported)
>>> test VIDIOC_G/S/ENUMINPUT: OK
>>> test VIDIOC_G/S_AUDIO: OK (Not Supported)
>>> Inputs: 1 Audio Inputs: 0 Tuners: 0
>>>
>>> Output ioctls:
>>> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>>> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>>> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>>> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>>> Outputs: 0 Audio Outputs: 0 Modulators: 0
>>>
>>> Input/Output configuration ioctls:
>>> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
>>> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
>>> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
>>> test VIDIOC_G/S_EDID: OK (Not Supported)
>>>
>>> Test input 0:
>>>
>>> Control ioctls:
>>> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
>>> test VIDIOC_QUERYCTRL: OK
>>> test VIDIOC_G/S_CTRL: OK
>>> fail: v4l2-test-controls.cpp(587): g_ext_ctrls does not
>>> support count == 0
>>
>> Huh. The issue here is that there are no controls at all, but the
>> control API is present. The class_check() function in v4l2-ctrls.c expects
>> that there are controls and if not it returns -EINVAL, causing this test
>> to fail.
>>
>> Try to apply this patch:
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> ---
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index dd1db678718c..4e53a8654690 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -2818,7 +2818,7 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
>> static int class_check(struct v4l2_ctrl_handler *hdl, u32 which)
>> {
>> if (which == 0 || which == V4L2_CTRL_WHICH_DEF_VAL)
>> - return list_empty(&hdl->ctrl_refs) ? -EINVAL : 0;
>> + return 0;
>> return find_ref_lock(hdl, which | 1) ? 0 : -EINVAL;
>> }
>>
>>
>> and see if it will pass the compliance test. There may be other issues.
>> I think that the compliance test should handle the case where there are no
>> controls, so this is a good test.
>
> Fails.
> fail: v4l2-test-controls.cpp(589): g_ext_ctrls worked even
> when no controls are present
> test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
Try this patch:
-----------------
diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp b/utils/v4l2-compliance/v4l2-test-controls.cpp
index 7514459f..508daf05 100644
--- a/utils/v4l2-compliance/v4l2-test-controls.cpp
+++ b/utils/v4l2-compliance/v4l2-test-controls.cpp
@@ -585,8 +585,6 @@ int testExtendedControls(struct node *node)
return ret;
if (ret)
return fail("g_ext_ctrls does not support count == 0\n");
- if (node->controls.empty())
- return fail("g_ext_ctrls worked even when no controls are present\n");
if (ctrls.which)
return fail("field which changed\n");
if (ctrls.count)
@@ -600,8 +598,6 @@ int testExtendedControls(struct node *node)
return ret;
if (ret)
return fail("try_ext_ctrls does not support count == 0\n");
- if (node->controls.empty())
- return fail("try_ext_ctrls worked even when no controls are present\n");
if (ctrls.which)
return fail("field which changed\n");
if (ctrls.count)
@@ -687,6 +683,8 @@ int testExtendedControls(struct node *node)
}
ctrls.which = 0;
+ ctrls.count = 1;
+ ctrls.controls = &ctrl;
ctrl.id = 0;
ctrl.size = 0;
ret = doioctl(node, VIDIOC_G_EXT_CTRLS, &ctrls);
@@ -745,6 +743,9 @@ int testExtendedControls(struct node *node)
if (ret)
return fail("could not set all controls\n");
+ if (!which)
+ return 0;
+
ctrls.which = which;
ret = doioctl(node, VIDIOC_G_EXT_CTRLS, &ctrls);
if (ret && !multiple_classes)
----------------
Hans
>
>> That said, another solution is that the driver detects that there are no
>> controls in unicam_probe_complete() and sets unicam->v4l2_dev.ctrl_handler
>> to NULL.
>>
>> I think you should do this in v4. Having control ioctls but no actual controls
>> is not wrong as such, but it is a bit misleading towards the application.
>
> OK, will do.
>
> Dave
>
