[libcamera-devel] [PATCH 02/10] libcamera: v4l2_controls: Move V4L2ControlId out of V4L2ControlInfo

[Niklas Söderlund]

Hi Jacopo, Laurent,

Thanks for your patch.

On 2019-10-14 02:27:48 +0300, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
> 
> In order to reconcile the libcamera and V4L2 control info maps, we need
> to move the V4L2ControlId embedded in V4L2ControlInfo map out of the
> class. Store the V4L2ControlId instances in the V4L2Device that creates
> them, and only reference them from V4L2ControlInfo.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

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

> ---
>  src/libcamera/include/v4l2_controls.h | 7 ++++---
>  src/libcamera/include/v4l2_device.h   | 4 +++-
>  src/libcamera/v4l2_controls.cpp       | 6 ++++--
>  src/libcamera/v4l2_device.cpp         | 3 ++-
>  4 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> index 133f5262febf..4d7ac1a133c7 100644
> --- a/src/libcamera/include/v4l2_controls.h
> +++ b/src/libcamera/include/v4l2_controls.h
> @@ -28,13 +28,14 @@ public:
>  class V4L2ControlInfo
>  {
>  public:
> -	V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
> +	V4L2ControlInfo(const V4L2ControlId &id,
> +			const struct v4l2_query_ext_ctrl &ctrl);
>  
> -	const ControlId &id() const { return id_; }
> +	const ControlId &id() const { return *id_; }
>  	const ControlRange &range() const { return range_; }
>  
>  private:
> -	V4L2ControlId id_;
> +	const V4L2ControlId *id_;
>  	ControlRange range_;
>  };
>  
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index daa762d58d2b..5a5b85827f23 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -8,7 +8,8 @@
>  #define __LIBCAMERA_V4L2_DEVICE_H__
>  
>  #include <map>
> -#include <string>
> +#include <memory>
> +#include <vector>
>  
>  #include <linux/videodev2.h>
>  
> @@ -48,6 +49,7 @@ private:
>  			    const struct v4l2_ext_control *v4l2Ctrls,
>  			    unsigned int count);
>  
> +	std::vector<std::unique_ptr<V4L2ControlId>> controlIds_;
>  	V4L2ControlInfoMap controls_;
>  	std::string deviceNode_;
>  	int fd_;
> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> index dcf31b7a8f26..12c4fb271ba5 100644
> --- a/src/libcamera/v4l2_controls.cpp
> +++ b/src/libcamera/v4l2_controls.cpp
> @@ -122,10 +122,12 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)
>  
>  /**
>   * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl
> + * \param[in] id The V4L2 control ID
>   * \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)
> +V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id,
> +				 const struct v4l2_query_ext_ctrl &ctrl)
> +	: id_(&id)
>  {
>  	if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)
>  		range_ = ControlRange(static_cast<int64_t>(ctrl.minimum),
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 54cc214ecce9..144a60b4fe93 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -386,9 +386,10 @@ void V4L2Device::listControls()
>  			continue;
>  		}
>  
> +		controlIds_.emplace_back(utils::make_unique<V4L2ControlId>(ctrl));
>  		ctrls.emplace(std::piecewise_construct,
>  			      std::forward_as_tuple(ctrl.id),
> -			      std::forward_as_tuple(ctrl));
> +			      std::forward_as_tuple(*controlIds_.back().get(), ctrl));
>  	}
>  
>  	controls_ = std::move(ctrls);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund