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

[Jacopo Mondi]

Hi Laurent,

On Wed, Oct 09, 2019 at 11:23:10AM +0300, Laurent Pinchart wrote:
> 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 ?
>

A derived class only used there?

Anyway, scratch this, too complex now and not really something that
concerns this series.

Please take my tag in.

Thanks
  j

> > 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
-------------- 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/ca226eb2/attachment-0001.sig>