[libcamera-devel,09/12] libcamera: v4l2_controls: Use the ControlRange class for control info
diff mbox series

Message ID 20190928152238.23752-10-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Improve the application-facing controls API
Related show

Commit Message

Laurent Pinchart Sept. 28, 2019, 3:22 p.m. UTC 
Use the ControlRange class to express the range of a V4L2 control,
replacing the open-coded minimum and maximum fields in the
V4L2ControlInfo class.

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

Comments

Jacopo Mondi Sept. 29, 2019, 10:13 a.m. UTC | #1 
Hi Laurent,

On Sat, Sep 28, 2019 at 06:22:35PM +0300, Laurent Pinchart wrote:
> Use the ControlRange class to express the range of a V4L2 control,
> replacing the open-coded minimum and maximum fields in the
> V4L2ControlInfo class.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/include/v4l2_controls.h |  6 ++----
>  src/libcamera/pipeline/uvcvideo.cpp   |  2 +-
>  src/libcamera/pipeline/vimc.cpp       |  2 +-
>  src/libcamera/v4l2_controls.cpp       | 21 ++++++++++-----------
>  4 files changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> index f2b67c5d44e1..b39370b2e90e 100644
> --- a/src/libcamera/include/v4l2_controls.h
> +++ b/src/libcamera/include/v4l2_controls.h
> @@ -30,8 +30,7 @@ public:
>  	size_t size() const { return size_; }
>  	const std::string &name() const { return name_; }

Same question as I had on Controls. Do we want to serialize and
transport names ?

Also, not V4L2ControlInfo seems more like to be a V4L2ControlId, but
that's just naming, so it's fine

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Thanks
   j


>
> -	int64_t min() const { return min_; }
> -	int64_t max() const { return max_; }
> +	const ControlRange &range() const { return range_; }
>
>  private:
>  	unsigned int id_;
> @@ -39,8 +38,7 @@ private:
>  	size_t size_;
>  	std::string name_;
>
> -	int64_t min_;
> -	int64_t max_;
> +	ControlRange range_;
>  };
>
>  using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index a80f8a43a116..17bbbad15838 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -364,7 +364,7 @@ int UVCCameraData::init(MediaEntity *entity)
>
>  		controlInfo_.emplace(std::piecewise_construct,
>  				     std::forward_as_tuple(id),
> -				     std::forward_as_tuple(info.min(), info.max()));
> +				     std::forward_as_tuple(info.range()));
>  	}
>
>  	return 0;
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 5d6909f58cf9..f9a7feb6571a 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -437,7 +437,7 @@ int VimcCameraData::init(MediaDevice *media)
>
>  		controlInfo_.emplace(std::piecewise_construct,
>  				     std::forward_as_tuple(id),
> -				     std::forward_as_tuple(info.min(), info.max()));
> +				     std::forward_as_tuple(info.range()));
>  	}
>
>  	return 0;
> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> index 64f0555fff7c..6f5f1578b139 100644
> --- a/src/libcamera/v4l2_controls.cpp
> +++ b/src/libcamera/v4l2_controls.cpp
> @@ -74,8 +74,13 @@ 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;
> +
> +	if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)
> +		range_ = ControlRange(static_cast<int64_t>(ctrl.minimum),
> +				      static_cast<int64_t>(ctrl.maximum));
> +	else
> +		range_ = ControlRange(static_cast<int32_t>(ctrl.minimum),
> +				      static_cast<int32_t>(ctrl.maximum));
>  }
>
>  /**
> @@ -103,15 +108,9 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
>   */
>
>  /**
> - * \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
> + * \fn V4L2ControlInfo::range()
> + * \brief Retrieve the control value range
> + * \return The V4L2 control value range
>   */
>
>  /**
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Sept. 29, 2019, 1:34 p.m. UTC | #2 
Hi Jacopo,

On Sun, Sep 29, 2019 at 12:13:08PM +0200, Jacopo Mondi wrote:
> On Sat, Sep 28, 2019 at 06:22:35PM +0300, Laurent Pinchart wrote:
> > Use the ControlRange class to express the range of a V4L2 control,
> > replacing the open-coded minimum and maximum fields in the
> > V4L2ControlInfo class.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/include/v4l2_controls.h |  6 ++----
> >  src/libcamera/pipeline/uvcvideo.cpp   |  2 +-
> >  src/libcamera/pipeline/vimc.cpp       |  2 +-
> >  src/libcamera/v4l2_controls.cpp       | 21 ++++++++++-----------
> >  4 files changed, 14 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> > index f2b67c5d44e1..b39370b2e90e 100644
> > --- a/src/libcamera/include/v4l2_controls.h
> > +++ b/src/libcamera/include/v4l2_controls.h
> > @@ -30,8 +30,7 @@ public:
> >  	size_t size() const { return size_; }
> >  	const std::string &name() const { return name_; }
> 
> Same question as I had on Controls. Do we want to serialize and
> transport names ?

No, there's no need to.

> Also, not V4L2ControlInfo seems more like to be a V4L2ControlId, but
> that's just naming, so it's fine
> 
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > -	int64_t min() const { return min_; }
> > -	int64_t max() const { return max_; }
> > +	const ControlRange &range() const { return range_; }
> >
> >  private:
> >  	unsigned int id_;
> > @@ -39,8 +38,7 @@ private:
> >  	size_t size_;
> >  	std::string name_;
> >
> > -	int64_t min_;
> > -	int64_t max_;
> > +	ControlRange range_;
> >  };
> >
> >  using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index a80f8a43a116..17bbbad15838 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -364,7 +364,7 @@ int UVCCameraData::init(MediaEntity *entity)
> >
> >  		controlInfo_.emplace(std::piecewise_construct,
> >  				     std::forward_as_tuple(id),
> > -				     std::forward_as_tuple(info.min(), info.max()));
> > +				     std::forward_as_tuple(info.range()));
> >  	}
> >
> >  	return 0;
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index 5d6909f58cf9..f9a7feb6571a 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -437,7 +437,7 @@ int VimcCameraData::init(MediaDevice *media)
> >
> >  		controlInfo_.emplace(std::piecewise_construct,
> >  				     std::forward_as_tuple(id),
> > -				     std::forward_as_tuple(info.min(), info.max()));
> > +				     std::forward_as_tuple(info.range()));
> >  	}
> >
> >  	return 0;
> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> > index 64f0555fff7c..6f5f1578b139 100644
> > --- a/src/libcamera/v4l2_controls.cpp
> > +++ b/src/libcamera/v4l2_controls.cpp
> > @@ -74,8 +74,13 @@ 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;
> > +
> > +	if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)
> > +		range_ = ControlRange(static_cast<int64_t>(ctrl.minimum),
> > +				      static_cast<int64_t>(ctrl.maximum));
> > +	else
> > +		range_ = ControlRange(static_cast<int32_t>(ctrl.minimum),
> > +				      static_cast<int32_t>(ctrl.maximum));
> >  }
> >
> >  /**
> > @@ -103,15 +108,9 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> >   */
> >
> >  /**
> > - * \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
> > + * \fn V4L2ControlInfo::range()
> > + * \brief Retrieve the control value range
> > + * \return The V4L2 control value range
> >   */
> >
> >  /**

Patch
diff mbox series 

diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
index f2b67c5d44e1..b39370b2e90e 100644
--- a/src/libcamera/include/v4l2_controls.h
+++ b/src/libcamera/include/v4l2_controls.h
@@ -30,8 +30,7 @@  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_; }
+	const ControlRange &range() const { return range_; }
 
 private:
 	unsigned int id_;
@@ -39,8 +38,7 @@  private:
 	size_t size_;
 	std::string name_;
 
-	int64_t min_;
-	int64_t max_;
+	ControlRange range_;
 };
 
 using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index a80f8a43a116..17bbbad15838 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -364,7 +364,7 @@  int UVCCameraData::init(MediaEntity *entity)
 
 		controlInfo_.emplace(std::piecewise_construct,
 				     std::forward_as_tuple(id),
-				     std::forward_as_tuple(info.min(), info.max()));
+				     std::forward_as_tuple(info.range()));
 	}
 
 	return 0;
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 5d6909f58cf9..f9a7feb6571a 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -437,7 +437,7 @@  int VimcCameraData::init(MediaDevice *media)
 
 		controlInfo_.emplace(std::piecewise_construct,
 				     std::forward_as_tuple(id),
-				     std::forward_as_tuple(info.min(), info.max()));
+				     std::forward_as_tuple(info.range()));
 	}
 
 	return 0;
diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
index 64f0555fff7c..6f5f1578b139 100644
--- a/src/libcamera/v4l2_controls.cpp
+++ b/src/libcamera/v4l2_controls.cpp
@@ -74,8 +74,13 @@  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;
+
+	if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)
+		range_ = ControlRange(static_cast<int64_t>(ctrl.minimum),
+				      static_cast<int64_t>(ctrl.maximum));
+	else
+		range_ = ControlRange(static_cast<int32_t>(ctrl.minimum),
+				      static_cast<int32_t>(ctrl.maximum));
 }
 
 /**
@@ -103,15 +108,9 @@  V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
  */
 
 /**
- * \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
+ * \fn V4L2ControlInfo::range()
+ * \brief Retrieve the control value range
+ * \return The V4L2 control value range
  */
 
 /**