[libcamera-devel] [PATCH 3/9] libcamera: controls: Store control name in ControlId

[Jacopo Mondi]

Hi Laurent,

On Tue, Oct 08, 2019 at 01:46:36AM +0300, Laurent Pinchart wrote:
> The ControlId class stores a pointer to the control name. This works
> fine for statically-defined controls, but requires code that allocates
> controls dynamically (for instance based on control discovery on a V4L2
> device) to keep a list of control names in separate storage. To ease
> usage of dynamically allocated controls, store a copy of the control
> name string in the ControlId class.
>

I would go as far as removing this from the ControlId, as it will need
to be serialized and it is used for debug purposes only.

In case we want to retrieve it for debug, we can use the controls
global map you have reintroduced in patch 1

If you like best to keep it:
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
   j

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  include/libcamera/controls.h | 6 +++---
>  src/libcamera/controls.cpp   | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 49ff1a6f747f..a5a6a135ec16 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -54,11 +54,11 @@ class ControlId
>  {
>  public:
>  	unsigned int id() const { return id_; }
> -	const char *name() const { return name_; }
> +	const std::string &name() const { return name_; }
>  	ControlType type() const { return type_; }
>
>  protected:
> -	ControlId(unsigned int id, const char *name, ControlType type)
> +	ControlId(unsigned int id, const std::string &name, ControlType type)
>  		: id_(id), name_(name), type_(type)
>  	{
>  	}
> @@ -68,7 +68,7 @@ private:
>  	ControlId &operator=(const ControlId &) = delete;
>
>  	unsigned int id_;
> -	const char *name_;
> +	std::string name_;
>  	ControlType type_;
>  };
>
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index f862a09adef9..e528dd80a2a7 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -205,7 +205,7 @@ std::string ControlValue::toString() const
>   */
>
>  /**
> - * \fn ControlId::ControlId(unsigned int id, const char *name, ControlType type)
> + * \fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)
>   * \brief Construct a ControlId instance
>   * \param[in] id The control numerical ID
>   * \param[in] name The control name
> --
> 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/e11b3b9b/attachment.sig>