Hi Hans,
On Thursday 27 September 2012 17:16:15 Laurent Pinchart wrote:
> When one of the requested controls doesn't exist the error_idx field
> must reflect that situation. For G_EXT_CTRLS and S_EXT_CTRLS, error_idx
> must be set to the control count. For TRY_EXT_CTRLS, it must be set to
> the index of the unexisting control.
>
> This issue was found by the v4l2-compliance tool.
I'm revisiting this patch as it has been reverted in v3.8-rc1.
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 17 ++++++++++-------
> drivers/media/usb/uvc/uvc_v4l2.c | 19 ++++++++++++-------
> 2 files changed, 22 insertions(+), 14 deletions(-)
[snip]
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index f00db30..e5817b9 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -591,8 +591,10 @@ static long uvc_v4l2_do_ioctl(struct file *file,
[snip]
> @@ -637,8 +639,9 @@ static long uvc_v4l2_do_ioctl(struct file *file,
> unsigned int cmd, void *arg) ret = uvc_ctrl_get(chain, ctrl);
> if (ret < 0) {
> uvc_ctrl_rollback(handle);
> - ctrls->error_idx = i;
> - return ret;
> + ctrls->error_idx = ret == -ENOENT
> + ? ctrls->count : i;
> + return ret == -ENOENT ? -EINVAL : ret;
> }
> }
> ctrls->error_idx = 0;
> @@ -661,8 +664,10 @@ static long uvc_v4l2_do_ioctl(struct file *file,
> unsigned int cmd, void *arg) ret = uvc_ctrl_set(chain, ctrl);
> if (ret < 0) {
> uvc_ctrl_rollback(handle);
> - ctrls->error_idx = i;
> - return ret;
> + ctrls->error_idx = (ret == -ENOENT &&
> + cmd == VIDIOC_S_EXT_CTRLS)
> + ? ctrls->count : i;
> + return ret == -ENOENT ? -EINVAL : ret;
> }
> }
I've reread the V4L2 specification, and the least I can say is that the text
is pretty ambiguous. Let's clarify it.
Is there a reason to differentiate between invalid control IDs and other
errors as far as error_idx is concerned ? It would be simpler if error_idx was
set to the index of the first error for get and try operations, regardless of
the error type. What do you think ?
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
