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

[Laurent Pinchart]

Hi Jacopo,

On Wed, Oct 09, 2019 at 10:18:22AM +0200, Jacopo Mondi wrote:
> 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

But that map stores the name in ControlId :-) Where would you store it ?

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