[libcamera-devel] [PATCH 6/9] libcamera: v4l2_controls: Add V4L2ControlId

[Jacopo Mondi]

Hi Laurent,

On Tue, Oct 08, 2019 at 01:46:39AM +0300, Laurent Pinchart wrote:
> Add a V4L2 specialisation of the ControlId class, in order to construct
> a ControlId from a v4l2_query_ext_ctrl. This is needed in order to use
> ControlList for V4L2 controls.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/include/v4l2_controls.h | 12 +++--
>  src/libcamera/v4l2_controls.cpp       | 67 +++++++++++++++++++++++----
>  src/libcamera/v4l2_device.cpp         |  4 +-
>  3 files changed, 68 insertions(+), 15 deletions(-)
>
> 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);
> +};
> +

This only add a constructor without any field. Do we need inheritance
or can we just add a constructor to ControlId ?

>  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;
> +	}
> +}

Probably it's ok to have inheritance as we need this in
v4l2_controls.cpp

This considered
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> +
> +} /* 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 at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191009/bd6479a7/attachment.sig>