[libcamera-devel] [PATCH] libcamera: v4l2_device: Fix control enumeration bug

[Jacopo Mondi]

Hi Laurent,

On Wed, Jun 26, 2019 at 04:44:57PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Jun 26, 2019 at 11:17:23AM +0100, Kieran Bingham wrote:
> > On 26/06/2019 11:13, Jacopo Mondi wrote:
> > > When enumerating the available V4L2 controls at video device open
>
> s/video device/V4L2 device/
>
> > > time set the V4L2_CTRL_FLAG_NEXT_CTRL flag when hitting an unsupported
> > > control type not to loop forever.
> >
> > "set the V4L2_CTRL_FLAG_NEXT_CTRL flag if an unsupported control type is
> > encountered to prevent infinite loops."
> >
> > > While at it, downgrade the message reporting the control type in not
> >
> > "reporting the unsupported control type to Debug"
> >
> >
> > > supported to Debug, as it is not an error worth being reported
> > > unconditionally.
> > >
> > > Fixes: 030ce6491ed3 ("libcamera: v4l2_device: List valid controls at open")
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
> > > ---
> > >  src/libcamera/v4l2_device.cpp | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > > index 84758a811c27..13d4bce5c013 100644
> > > --- a/src/libcamera/v4l2_device.cpp
> > > +++ b/src/libcamera/v4l2_device.cpp
> > > @@ -341,8 +341,9 @@ void V4L2Device::listControls()
> > >  			break;
> > >  		/* \todo Support compound controls. */
> > >  		default:
> > > -			LOG(V4L2, Error) << "Control type '" << info.type()
> > > +			LOG(V4L2, Debug) << "Control type '" << info.type()
> > >  					 << "' not supported";
> > > +			ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL;
> > >  			continue;
> > >  		}
> > >
>
> Would the following (untested) change be a better solution, to avoid
> or'ing V4L2_CTRL_FLAG_NEXT_CTRL in multiple places ?

It is. I'll apply and merge. I assume I can ad your tag.

Thanks
  j

>
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 84758a811c27..f74c5b2aff02 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -321,13 +321,15 @@ void V4L2Device::listControls()
>  	struct v4l2_query_ext_ctrl ctrl = {};
>
>  	/* \todo Add support for menu and compound controls. */
> -	ctrl.id = V4L2_CTRL_FLAG_NEXT_CTRL;
> -	while (ioctl(VIDIOC_QUERY_EXT_CTRL, &ctrl) == 0) {
> +	while (1) {
> +		ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL;
> +		int ret = ioctl(VIDIOC_QUERY_EXT_CTRL, &ctrl);
> +		if (ret < 0)
> +			break;
> +
>  		if (ctrl.type == V4L2_CTRL_TYPE_CTRL_CLASS ||
> -		    ctrl.flags & V4L2_CTRL_FLAG_DISABLED) {
> -			ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL;
> +		    ctrl.flags & V4L2_CTRL_FLAG_DISABLED)
>  			continue;
> -		}
>
>  		V4L2ControlInfo info(ctrl);
>  		switch (info.type()) {
> @@ -347,7 +349,6 @@ void V4L2Device::listControls()
>  		}
>
>  		controls_.emplace(ctrl.id, info);
> -		ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL;
>  	}
>  }
>
>
>
> --
> Regards,
>
> Laurent Pinchart
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190627/be96f0bd/attachment-0001.sig>