[libcamera-devel,v2,10/14] libcamera: v4l2_controls: Add V4L2ControlId
diff mbox series

Message ID 20191012184407.31684-11-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Use ControlList for both libcamera and V4L2 controls
Related show

Commit Message

Laurent Pinchart Oct. 12, 2019, 6:44 p.m. UTC 
Add a V4L2 specialisation of the ControlId class, in order to construct
a ControlId from a v4l2_query_ext_ctrl. The V4L2ControlId is embedded in
V4L2ControlInfo, and thus needs to be copyable to allow for
V4L2ControlInfo to be passed to IPAs. The ControlId copy constructor and
assignment operators are thus restored, but made protected to avoid the
Control class being copyable.

This is needed in order to use ControlList for V4L2 controls, as
ControlList requires ControlId instances for all controls.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/controls.h          |  7 +--
 src/libcamera/include/v4l2_controls.h | 12 +++--
 src/libcamera/v4l2_controls.cpp       | 67 +++++++++++++++++++++++----
 src/libcamera/v4l2_device.cpp         |  4 +-
 4 files changed, 72 insertions(+), 18 deletions(-)

Comments

Jacopo Mondi Oct. 13, 2019, 3:03 p.m. UTC | #1 
Hi Laurent

On Sat, Oct 12, 2019 at 09:44:03PM +0300, Laurent Pinchart wrote:
> Add a V4L2 specialisation of the ControlId class, in order to construct
> a ControlId from a v4l2_query_ext_ctrl. The V4L2ControlId is embedded in
> V4L2ControlInfo, and thus needs to be copyable to allow for
> V4L2ControlInfo to be passed to IPAs. The ControlId copy constructor and
> assignment operators are thus restored, but made protected to avoid the
> Control class being copyable.
>
> This is needed in order to use ControlList for V4L2 controls, as
> ControlList requires ControlId instances for all controls.

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

Thanks
  j

>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/controls.h          |  7 +--
>  src/libcamera/include/v4l2_controls.h | 12 +++--
>  src/libcamera/v4l2_controls.cpp       | 67 +++++++++++++++++++++++----
>  src/libcamera/v4l2_device.cpp         |  4 +-
>  4 files changed, 72 insertions(+), 18 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 5e6708fe570b..ebc4204f98fd 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -68,11 +68,12 @@ protected:
>  		: id_(id), name_(name), type_(type)
>  	{
>  	}
> +#ifndef __DOXYGEN__
> +	ControlId &operator=(const ControlId &) = default;
> +	ControlId(const ControlId &) = default;
> +#endif
>
>  private:
> -	ControlId(const ControlId &) = delete;
> -	ControlId &operator=(const ControlId &) = delete;
> -
>  	unsigned int id_;
>  	std::string name_;
>  	ControlType type_;
> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> index b39370b2e90e..71ce377fe4c5 100644
> --- a/src/libcamera/include/v4l2_controls.h
> +++ b/src/libcamera/include/v4l2_controls.h
> @@ -20,23 +20,27 @@
>
>  namespace libcamera {
>
> +class V4L2ControlId : public ControlId
> +{
> +public:
> +	V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl);
> +};
> +
>  class V4L2ControlInfo
>  {
>  public:
>  	V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
>
> -	unsigned int id() const { return id_; }
> +	const ControlId &id() const { return id_; }
>  	unsigned int type() const { return type_; }
>  	size_t size() const { return size_; }
> -	const std::string &name() const { return name_; }
>
>  	const ControlRange &range() const { return range_; }
>
>  private:
> -	unsigned int id_;
> +	V4L2ControlId id_;
>  	unsigned int type_;
>  	size_t size_;
> -	std::string name_;
>
>  	ControlRange range_;
>  };
> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> index 6f5f1578b139..a630a2583d33 100644
> --- a/src/libcamera/v4l2_controls.cpp
> +++ b/src/libcamera/v4l2_controls.cpp
> @@ -7,6 +7,8 @@
>
>  #include "v4l2_controls.h"
>
> +#include <string.h>
> +
>  /**
>   * \file v4l2_controls.h
>   * \brief Support for V4L2 Controls using the V4L2 Extended Controls APIs
> @@ -47,6 +49,60 @@
>
>  namespace libcamera {
>
> +namespace {
> +
> +std::string v4l2_ctrl_name(const struct v4l2_query_ext_ctrl &ctrl)
> +{
> +	size_t len = strnlen(ctrl.name, sizeof(ctrl.name));
> +	return std::string(static_cast<const char *>(ctrl.name), len);
> +}
> +
> +ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)
> +{
> +	switch (ctrl.type) {
> +	case V4L2_CTRL_TYPE_BOOLEAN:
> +		return ControlTypeBool;
> +
> +	case V4L2_CTRL_TYPE_INTEGER:
> +		return ControlTypeInteger32;
> +
> +	case V4L2_CTRL_TYPE_INTEGER64:
> +		return ControlTypeInteger64;
> +
> +	case V4L2_CTRL_TYPE_MENU:
> +	case V4L2_CTRL_TYPE_BUTTON:
> +	case V4L2_CTRL_TYPE_BITMASK:
> +	case V4L2_CTRL_TYPE_INTEGER_MENU:
> +		/*
> +		 * More precise types may be needed, for now use a 32-bit
> +		 * integer type.
> +		 */
> +		return ControlTypeInteger32;
> +
> +	default:
> +		return ControlTypeNone;
> +	}
> +}
> +
> +} /* namespace */
> +
> +/**
> + * \class V4L2ControlId
> + * \brief V4L2 control static metadata
> + *
> + * The V4L2ControlId class is a specialisation of the ControlId for V4L2
> + * controls.
> + */
> +
> +/**
> + * \brief Construct a V4L2ControlId from a struct v4l2_query_ext_ctrl
> + * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel
> + */
> +V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)
> +	: ControlId(ctrl.id, v4l2_ctrl_name(ctrl), v4l2_ctrl_type(ctrl))
> +{
> +}
> +
>  /**
>   * \class V4L2ControlInfo
>   * \brief Information on a V4L2 control
> @@ -66,13 +122,12 @@ namespace libcamera {
>
>  /**
>   * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl
> - * \param ctrl The struct v4l2_query_ext_ctrl as returned by the kernel
> + * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel
>   */
>  V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> +	: id_(ctrl)
>  {
> -	id_ = ctrl.id;
>  	type_ = ctrl.type;
> -	name_ = static_cast<const char *>(ctrl.name);
>  	size_ = ctrl.elem_size * ctrl.elems;
>
>  	if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)
> @@ -101,12 +156,6 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
>   * \return The V4L2 control value data size
>   */
>
> -/**
> - * \fn V4L2ControlInfo::name()
> - * \brief Retrieve the control user readable name
> - * \return The V4L2 control user readable name
> - */
> -
>  /**
>   * \fn V4L2ControlInfo::range()
>   * \brief Retrieve the control value range
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 3bd82ff23212..466c3d41f6e3 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -184,7 +184,7 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)
>
>  		const V4L2ControlInfo *info = &iter->second;
>  		controlInfo[i] = info;
> -		v4l2Ctrls[i].id = info->id();
> +		v4l2Ctrls[i].id = ctrl->id();
>  	}
>
>  	struct v4l2_ext_controls v4l2ExtCtrls = {};
> @@ -259,7 +259,7 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)
>
>  		const V4L2ControlInfo *info = &iter->second;
>  		controlInfo[i] = info;
> -		v4l2Ctrls[i].id = info->id();
> +		v4l2Ctrls[i].id = ctrl->id();
>
>  		/* Set the v4l2_ext_control value for the write operation. */
>  		switch (info->type()) {
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Niklas Söderlund Oct. 13, 2019, 4:05 p.m. UTC | #2 
Hi Laurent,

Thanks for your work.

On 2019-10-12 21:44:03 +0300, Laurent Pinchart wrote:
> Add a V4L2 specialisation of the ControlId class, in order to construct
> a ControlId from a v4l2_query_ext_ctrl. The V4L2ControlId is embedded in
> V4L2ControlInfo, and thus needs to be copyable to allow for
> V4L2ControlInfo to be passed to IPAs. The ControlId copy constructor and
> assignment operators are thus restored, but made protected to avoid the
> Control class being copyable.
> 
> This is needed in order to use ControlList for V4L2 controls, as
> ControlList requires ControlId instances for all controls.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/libcamera/controls.h          |  7 +--
>  src/libcamera/include/v4l2_controls.h | 12 +++--
>  src/libcamera/v4l2_controls.cpp       | 67 +++++++++++++++++++++++----
>  src/libcamera/v4l2_device.cpp         |  4 +-
>  4 files changed, 72 insertions(+), 18 deletions(-)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 5e6708fe570b..ebc4204f98fd 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -68,11 +68,12 @@ protected:
>  		: id_(id), name_(name), type_(type)
>  	{
>  	}
> +#ifndef __DOXYGEN__
> +	ControlId &operator=(const ControlId &) = default;
> +	ControlId(const ControlId &) = default;
> +#endif
>  
>  private:
> -	ControlId(const ControlId &) = delete;
> -	ControlId &operator=(const ControlId &) = delete;
> -
>  	unsigned int id_;
>  	std::string name_;
>  	ControlType type_;
> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> index b39370b2e90e..71ce377fe4c5 100644
> --- a/src/libcamera/include/v4l2_controls.h
> +++ b/src/libcamera/include/v4l2_controls.h
> @@ -20,23 +20,27 @@
>  
>  namespace libcamera {
>  
> +class V4L2ControlId : public ControlId
> +{
> +public:
> +	V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl);
> +};
> +
>  class V4L2ControlInfo
>  {
>  public:
>  	V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
>  
> -	unsigned int id() const { return id_; }
> +	const ControlId &id() const { return id_; }
>  	unsigned int type() const { return type_; }
>  	size_t size() const { return size_; }
> -	const std::string &name() const { return name_; }
>  
>  	const ControlRange &range() const { return range_; }
>  
>  private:
> -	unsigned int id_;
> +	V4L2ControlId id_;
>  	unsigned int type_;
>  	size_t size_;
> -	std::string name_;
>  
>  	ControlRange range_;
>  };
> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> index 6f5f1578b139..a630a2583d33 100644
> --- a/src/libcamera/v4l2_controls.cpp
> +++ b/src/libcamera/v4l2_controls.cpp
> @@ -7,6 +7,8 @@
>  
>  #include "v4l2_controls.h"
>  
> +#include <string.h>
> +
>  /**
>   * \file v4l2_controls.h
>   * \brief Support for V4L2 Controls using the V4L2 Extended Controls APIs
> @@ -47,6 +49,60 @@
>  
>  namespace libcamera {
>  
> +namespace {
> +
> +std::string v4l2_ctrl_name(const struct v4l2_query_ext_ctrl &ctrl)
> +{
> +	size_t len = strnlen(ctrl.name, sizeof(ctrl.name));
> +	return std::string(static_cast<const char *>(ctrl.name), len);
> +}
> +
> +ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)
> +{
> +	switch (ctrl.type) {
> +	case V4L2_CTRL_TYPE_BOOLEAN:
> +		return ControlTypeBool;
> +
> +	case V4L2_CTRL_TYPE_INTEGER:
> +		return ControlTypeInteger32;
> +
> +	case V4L2_CTRL_TYPE_INTEGER64:
> +		return ControlTypeInteger64;
> +
> +	case V4L2_CTRL_TYPE_MENU:
> +	case V4L2_CTRL_TYPE_BUTTON:
> +	case V4L2_CTRL_TYPE_BITMASK:
> +	case V4L2_CTRL_TYPE_INTEGER_MENU:
> +		/*
> +		 * More precise types may be needed, for now use a 32-bit
> +		 * integer type.
> +		 */
> +		return ControlTypeInteger32;
> +
> +	default:
> +		return ControlTypeNone;
> +	}
> +}
> +
> +} /* namespace */
> +
> +/**
> + * \class V4L2ControlId
> + * \brief V4L2 control static metadata
> + *
> + * The V4L2ControlId class is a specialisation of the ControlId for V4L2
> + * controls.
> + */
> +
> +/**
> + * \brief Construct a V4L2ControlId from a struct v4l2_query_ext_ctrl
> + * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel
> + */
> +V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)
> +	: ControlId(ctrl.id, v4l2_ctrl_name(ctrl), v4l2_ctrl_type(ctrl))
> +{
> +}
> +
>  /**
>   * \class V4L2ControlInfo
>   * \brief Information on a V4L2 control
> @@ -66,13 +122,12 @@ namespace libcamera {
>  
>  /**
>   * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl
> - * \param ctrl The struct v4l2_query_ext_ctrl as returned by the kernel
> + * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel
>   */
>  V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> +	: id_(ctrl)
>  {
> -	id_ = ctrl.id;
>  	type_ = ctrl.type;
> -	name_ = static_cast<const char *>(ctrl.name);
>  	size_ = ctrl.elem_size * ctrl.elems;
>  
>  	if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)
> @@ -101,12 +156,6 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
>   * \return The V4L2 control value data size
>   */
>  
> -/**
> - * \fn V4L2ControlInfo::name()
> - * \brief Retrieve the control user readable name
> - * \return The V4L2 control user readable name
> - */
> -
>  /**
>   * \fn V4L2ControlInfo::range()
>   * \brief Retrieve the control value range
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 3bd82ff23212..466c3d41f6e3 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -184,7 +184,7 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)
>  
>  		const V4L2ControlInfo *info = &iter->second;
>  		controlInfo[i] = info;
> -		v4l2Ctrls[i].id = info->id();
> +		v4l2Ctrls[i].id = ctrl->id();
>  	}
>  
>  	struct v4l2_ext_controls v4l2ExtCtrls = {};
> @@ -259,7 +259,7 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)
>  
>  		const V4L2ControlInfo *info = &iter->second;
>  		controlInfo[i] = info;
> -		v4l2Ctrls[i].id = info->id();
> +		v4l2Ctrls[i].id = ctrl->id();
>  
>  		/* Set the v4l2_ext_control value for the write operation. */
>  		switch (info->type()) {
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index 5e6708fe570b..ebc4204f98fd 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -68,11 +68,12 @@  protected:
 		: id_(id), name_(name), type_(type)
 	{
 	}
+#ifndef __DOXYGEN__
+	ControlId &operator=(const ControlId &) = default;
+	ControlId(const ControlId &) = default;
+#endif
 
 private:
-	ControlId(const ControlId &) = delete;
-	ControlId &operator=(const ControlId &) = delete;
-
 	unsigned int id_;
 	std::string name_;
 	ControlType type_;
diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
index b39370b2e90e..71ce377fe4c5 100644
--- a/src/libcamera/include/v4l2_controls.h
+++ b/src/libcamera/include/v4l2_controls.h
@@ -20,23 +20,27 @@ 
 
 namespace libcamera {
 
+class V4L2ControlId : public ControlId
+{
+public:
+	V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl);
+};
+
 class V4L2ControlInfo
 {
 public:
 	V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
 
-	unsigned int id() const { return id_; }
+	const ControlId &id() const { return id_; }
 	unsigned int type() const { return type_; }
 	size_t size() const { return size_; }
-	const std::string &name() const { return name_; }
 
 	const ControlRange &range() const { return range_; }
 
 private:
-	unsigned int id_;
+	V4L2ControlId id_;
 	unsigned int type_;
 	size_t size_;
-	std::string name_;
 
 	ControlRange range_;
 };
diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
index 6f5f1578b139..a630a2583d33 100644
--- a/src/libcamera/v4l2_controls.cpp
+++ b/src/libcamera/v4l2_controls.cpp
@@ -7,6 +7,8 @@ 
 
 #include "v4l2_controls.h"
 
+#include <string.h>
+
 /**
  * \file v4l2_controls.h
  * \brief Support for V4L2 Controls using the V4L2 Extended Controls APIs
@@ -47,6 +49,60 @@ 
 
 namespace libcamera {
 
+namespace {
+
+std::string v4l2_ctrl_name(const struct v4l2_query_ext_ctrl &ctrl)
+{
+	size_t len = strnlen(ctrl.name, sizeof(ctrl.name));
+	return std::string(static_cast<const char *>(ctrl.name), len);
+}
+
+ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)
+{
+	switch (ctrl.type) {
+	case V4L2_CTRL_TYPE_BOOLEAN:
+		return ControlTypeBool;
+
+	case V4L2_CTRL_TYPE_INTEGER:
+		return ControlTypeInteger32;
+
+	case V4L2_CTRL_TYPE_INTEGER64:
+		return ControlTypeInteger64;
+
+	case V4L2_CTRL_TYPE_MENU:
+	case V4L2_CTRL_TYPE_BUTTON:
+	case V4L2_CTRL_TYPE_BITMASK:
+	case V4L2_CTRL_TYPE_INTEGER_MENU:
+		/*
+		 * More precise types may be needed, for now use a 32-bit
+		 * integer type.
+		 */
+		return ControlTypeInteger32;
+
+	default:
+		return ControlTypeNone;
+	}
+}
+
+} /* namespace */
+
+/**
+ * \class V4L2ControlId
+ * \brief V4L2 control static metadata
+ *
+ * The V4L2ControlId class is a specialisation of the ControlId for V4L2
+ * controls.
+ */
+
+/**
+ * \brief Construct a V4L2ControlId from a struct v4l2_query_ext_ctrl
+ * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel
+ */
+V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)
+	: ControlId(ctrl.id, v4l2_ctrl_name(ctrl), v4l2_ctrl_type(ctrl))
+{
+}
+
 /**
  * \class V4L2ControlInfo
  * \brief Information on a V4L2 control
@@ -66,13 +122,12 @@  namespace libcamera {
 
 /**
  * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl
- * \param ctrl The struct v4l2_query_ext_ctrl as returned by the kernel
+ * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel
  */
 V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
+	: id_(ctrl)
 {
-	id_ = ctrl.id;
 	type_ = ctrl.type;
-	name_ = static_cast<const char *>(ctrl.name);
 	size_ = ctrl.elem_size * ctrl.elems;
 
 	if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)
@@ -101,12 +156,6 @@  V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
  * \return The V4L2 control value data size
  */
 
-/**
- * \fn V4L2ControlInfo::name()
- * \brief Retrieve the control user readable name
- * \return The V4L2 control user readable name
- */
-
 /**
  * \fn V4L2ControlInfo::range()
  * \brief Retrieve the control value range
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
index 3bd82ff23212..466c3d41f6e3 100644
--- a/src/libcamera/v4l2_device.cpp
+++ b/src/libcamera/v4l2_device.cpp
@@ -184,7 +184,7 @@  int V4L2Device::getControls(V4L2ControlList *ctrls)
 
 		const V4L2ControlInfo *info = &iter->second;
 		controlInfo[i] = info;
-		v4l2Ctrls[i].id = info->id();
+		v4l2Ctrls[i].id = ctrl->id();
 	}
 
 	struct v4l2_ext_controls v4l2ExtCtrls = {};
@@ -259,7 +259,7 @@  int V4L2Device::setControls(V4L2ControlList *ctrls)
 
 		const V4L2ControlInfo *info = &iter->second;
 		controlInfo[i] = info;
-		v4l2Ctrls[i].id = info->id();
+		v4l2Ctrls[i].id = ctrl->id();
 
 		/* Set the v4l2_ext_control value for the write operation. */
 		switch (info->type()) {