[libcamera-devel,v3,02/14] libcamera: v4l2_controls: Add min and max to V4L2ControlInfo
diff mbox series

Message ID 20190630233817.10130-3-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera Controls
Related show

Commit Message

Laurent Pinchart June 30, 2019, 11:38 p.m. UTC 
Add min() and max() methods to V4L2ControlInfo to report the control's
minimum and maximum value respectively.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/include/v4l2_controls.h |  6 ++++++
 src/libcamera/v4l2_controls.cpp       | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

Comments

Kieran Bingham July 1, 2019, 8:02 a.m. UTC | #1 
Hi Laurent,

On 01/07/2019 00:38, Laurent Pinchart wrote:
> Add min() and max() methods to V4L2ControlInfo to report the control's
> minimum and maximum value respectively.

Great this ties in to the existing V4L2 Controls easily.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@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;
>  }
>  
>  /**
> @@ -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
>
Jacopo Mondi July 1, 2019, 2:56 p.m. UTC | #2 
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@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.

That said, if that' fine in your opinion, please have my
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>  }
>
>  /**
> @@ -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
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart July 1, 2019, 3:50 p.m. UTC | #3 
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@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@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

Patch
diff mbox series 

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;
 }
 
 /**
@@ -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