v4l2-compliance: Let uvcvideo return -EACCES

  Setting a control while inactive is meant to work, but it might
not be actually written to the hardware until control becomes active.

v4l2-compliance should allow -EACCES as an error code, but only for
the uvcdriver when an attempt is made to set inactive controls.

The control framework is able to handle this case more elegantly:
it will remember the last set inactive value, and when the control
becomes active it will update the hardware. But that's really hard
to model in uvc.

Suggested-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
---
 utils/v4l2-compliance/v4l2-compliance.cpp    |  2 ++
 utils/v4l2-compliance/v4l2-compliance.h      |  1 +
 utils/v4l2-compliance/v4l2-test-controls.cpp | 21 +++++++++++++++-----
 3 files changed, 19 insertions(+), 5 deletions(-)
  

On 15/03/2021 18:25, Ricardo Ribalda wrote:
> Setting a control while inactive is meant to work, but it might
> not be actually written to the hardware until control becomes active.
> 
> v4l2-compliance should allow -EACCES as an error code, but only for
> the uvcdriver when an attempt is made to set inactive controls.
> 
> The control framework is able to handle this case more elegantly:
> it will remember the last set inactive value, and when the control
> becomes active it will update the hardware. But that's really hard
> to model in uvc.
> 
> Suggested-by: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
> ---
>  utils/v4l2-compliance/v4l2-compliance.cpp    |  2 ++
>  utils/v4l2-compliance/v4l2-compliance.h      |  1 +
>  utils/v4l2-compliance/v4l2-test-controls.cpp | 21 +++++++++++++++-----
>  3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> index 9f71332c..1c21197b 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> @@ -84,6 +84,7 @@ bool show_colors;
>  bool exit_on_fail;
>  bool exit_on_warn;
>  bool is_vivid;
> +bool is_uvcvideo;
>  int media_fd = -1;
>  unsigned warnings;
>  
> @@ -958,6 +959,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
>  	if (node.is_v4l2()) {
>  		doioctl(&node, VIDIOC_QUERYCAP, &vcap);
>  		driver = reinterpret_cast<const char *>(vcap.driver);
> +		is_uvcvideo = driver == "uvcvideo";
>  		is_vivid = driver == "vivid";
>  		if (is_vivid)
>  			node.bus_info = reinterpret_cast<const char *>(vcap.bus_info);
> diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h
> index 4d5c3a5c..db4790a6 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.h
> +++ b/utils/v4l2-compliance/v4l2-compliance.h
> @@ -50,6 +50,7 @@ extern bool no_progress;
>  extern bool exit_on_fail;
>  extern bool exit_on_warn;
>  extern bool is_vivid; // We're testing the vivid driver
> +extern bool is_uvcvideo; // We're testing the uvc driver
>  extern int kernel_version;
>  extern int media_fd;
>  extern unsigned warnings;
> diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp b/utils/v4l2-compliance/v4l2-test-controls.cpp
> index 4be2f61c..70a8353a 100644
> --- a/utils/v4l2-compliance/v4l2-test-controls.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-controls.cpp
> @@ -485,6 +485,8 @@ int testSimpleControls(struct node *node)
>  		} else if (ret == EILSEQ) {
>  			warn("s_ctrl returned EILSEQ\n");
>  			ret = 0;
> +		} else if (ret == EACCES && is_uvcvideo) {
> +			ret = 0;
>  		} else if (ret) {
>  			return fail("s_ctrl returned an error (%d)\n", ret);
>  		}
> @@ -498,7 +500,8 @@ int testSimpleControls(struct node *node)
>  			ctrl.id = qctrl.id;
>  			ctrl.value = qctrl.minimum - 1;
>  			ret = doioctl(node, VIDIOC_S_CTRL, &ctrl);
> -			if (ret && ret != EIO && ret != EILSEQ && ret != ERANGE)
> +			if (ret && ret != EIO && ret != EILSEQ && ret != ERANGE &&
> +			    !(ret == EACCES && is_uvcvideo))
>  				return fail("invalid minimum range check\n");
>  			if (!ret && checkSimpleCtrl(ctrl, qctrl))
>  				return fail("invalid control %08x\n", qctrl.id);
> @@ -508,7 +511,8 @@ int testSimpleControls(struct node *node)
>  			ctrl.id = qctrl.id;
>  			ctrl.value = qctrl.maximum + 1;
>  			ret = doioctl(node, VIDIOC_S_CTRL, &ctrl);
> -			if (ret && ret != EIO && ret != EILSEQ && ret != ERANGE)
> +			if (ret && ret != EIO && ret != EILSEQ && ret != ERANGE &&
> +			    !(ret == EACCES && is_uvcvideo))
>  				return fail("invalid maximum range check\n");
>  			if (!ret && checkSimpleCtrl(ctrl, qctrl))
>  				return fail("invalid control %08x\n", qctrl.id);

For the 'if (qctrl.step > 1 && qctrl.maximum > qctrl.minimum) {' section an
EACCES check is also needed (it fails there for my Logitech webcam).

The same is true for the 'if (qctrl.type == V4L2_CTRL_TYPE_MENU || qctrl.type == V4L2_CTRL_TYPE_INTEGER_MENU) {'
section. Unfortunately, I don't have a uvc webcam that has an inactive menu.

Regards,

	Hans

> @@ -551,15 +555,18 @@ int testSimpleControls(struct node *node)
>  			ctrl.id = qctrl.id;
>  			ctrl.value = qctrl.minimum;
>  			ret = doioctl(node, VIDIOC_S_CTRL, &ctrl);
> -			if (ret && ret != EIO && ret != EILSEQ)
> +			if (ret && ret != EIO && ret != EILSEQ &&
> +			    !(ret == EACCES && is_uvcvideo))
>  				return fail("could not set minimum value\n");
>  			ctrl.value = qctrl.maximum;
>  			ret = doioctl(node, VIDIOC_S_CTRL, &ctrl);
> -			if (ret && ret != EIO && ret != EILSEQ)
> +			if (ret && ret != EIO && ret != EILSEQ &&
> +			    !(ret == EACCES && is_uvcvideo))
>  				return fail("could not set maximum value\n");
>  			ctrl.value = qctrl.default_value;
>  			ret = doioctl(node, VIDIOC_S_CTRL, &ctrl);
> -			if (ret && ret != EIO && ret != EILSEQ)
> +			if (ret && ret != EIO && ret != EILSEQ &&
> +			    !(ret == EACCES && is_uvcvideo))
>  				return fail("could not set default value\n");
>  		}
>  	}
> @@ -731,6 +738,8 @@ int testExtendedControls(struct node *node)
>  			} else if (ret == EILSEQ) {
>  				warn("s_ext_ctrls returned EILSEQ\n");
>  				ret = 0;
> +			} else if (ret == EACCES && is_uvcvideo) {
> +				ret = 0;
>  			}
>  			if (ret)
>  				return fail("s_ext_ctrls returned an error (%d)\n", ret);
> @@ -806,6 +815,8 @@ int testExtendedControls(struct node *node)
>  	} else if (ret == EILSEQ) {
>  		warn("s_ext_ctrls returned EILSEQ\n");
>  		ret = 0;
> +	} else if (ret == EACCES && is_uvcvideo) {
> +		ret = 0;
>  	}
>  	if (ret)
>  		return fail("could not set all controls\n");
>