[libcamera-devel] [PATCH v3 02/14] libcamera: v4l2_controls: Add min and max to V4L2ControlInfo

[Laurent Pinchart]

Hi Jacopo,

On Mon, Jul 01, 2019 at 04:56:26PM +0200, Jacopo Mondi wrote:
> Hi Laurent,
> 
> On Mon, Jul 01, 2019 at 02:38:05AM +0300, Laurent Pinchart wrote:
> > Add min() and max() methods to V4L2ControlInfo to report the control's
> > minimum and maximum value respectively.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/include/v4l2_controls.h |  6 ++++++
> >  src/libcamera/v4l2_controls.cpp       | 14 ++++++++++++++
> >  2 files changed, 20 insertions(+)
> >
> > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> > index 2c8cb9003f25..0047efab11fa 100644
> > --- a/src/libcamera/include/v4l2_controls.h
> > +++ b/src/libcamera/include/v4l2_controls.h
> > @@ -28,11 +28,17 @@ public:
> >  	size_t size() const { return size_; }
> >  	const std::string &name() const { return name_; }
> >
> > +	int64_t min() const { return min_; }
> > +	int64_t max() const { return max_; }
> > +
> >  private:
> >  	unsigned int id_;
> >  	unsigned int type_;
> >  	size_t size_;
> >  	std::string name_;
> > +
> > +	int64_t min_;
> > +	int64_t max_;
> >  };
> >
> >  class V4L2Control
> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> > index 78888de29642..af017bce48ba 100644
> > --- a/src/libcamera/v4l2_controls.cpp
> > +++ b/src/libcamera/v4l2_controls.cpp
> > @@ -74,6 +74,8 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> >  	type_ = ctrl.type;
> >  	name_ = static_cast<const char *>(ctrl.name);
> >  	size_ = ctrl.elem_size * ctrl.elems;
> > +	min_ = ctrl.minimum;
> > +	max_ = ctrl.maximum;
> 
> Please note that ctrl.minimum and ctrl.maximum are meaningful only for
> some of the types the v4l2_device supports at enumeration time
> 
> 		V4L2ControlInfo info(ctrl);
> 		switch (info.type()) {
> 		case V4L2_CTRL_TYPE_INTEGER:
> 		case V4L2_CTRL_TYPE_BOOLEAN:
> 		case V4L2_CTRL_TYPE_MENU:
> 		case V4L2_CTRL_TYPE_BUTTON:
> 		case V4L2_CTRL_TYPE_INTEGER64:
> 		case V4L2_CTRL_TYPE_BITMASK:
> 		case V4L2_CTRL_TYPE_INTEGER_MENU:
> 			break;
> 		/* \todo Support compound controls. */
> 		default:
> 			LOG(V4L2, Debug) << "Control type '" << info.type()
> 					 << "' not supported";
> 			continue;
> 		}
> 
> 		controls_.emplace(ctrl.id, info);
> 
> https://www.kernel.org/doc/html/v5.0/media/uapi/v4l/vidioc-queryctrl.html#c.v4l2_ctrl_type
> 
> The most problematic one seems to be the BITMASK type, but also for
> others, the driver might report nothing.

The V4L2 control framework should report 0 in those cases, so we should
be fine. We will have to handle default values and step later, and these
will be type-dependent as well, so we may end up reworking this code,
but for now I don't think it's a problem.

> That said, if that' fine in your opinion, please have my
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> >  }
> >
> >  /**
> > @@ -100,6 +102,18 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> >   * \return The V4L2 control user readable name
> >   */
> >
> > +/**
> > + * \fn V4L2ControlInfo::min()
> > + * \brief Retrieve the control minimum value
> > + * \return The V4L2 control minimum value
> > + */
> > +
> > +/**
> > + * \fn V4L2ControlInfo::max()
> > + * \brief Retrieve the control maximum value
> > + * \return The V4L2 control maximum value
> > + */
> > +
> >  /**
> >   * \class V4L2Control
> >   * \brief A V4L2 control value

-- 
Regards,

Laurent Pinchart