[libcamera-devel] [PATCH v2 06/12] libcamera: camera_configuration: Return config index

[Hirokazu Honda]

The return value is useful if the index of added point is not always
the deterministic place, like std::map.
However, like std::vector, the adding point is always the same, end of
vector, per the implementation.
If it is guaranteed at the interface level, I think the return value
is unnecessary.

On Sun, Sep 6, 2020 at 6:30 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Sep 02, 2020 at 05:22:30PM +0200, Jacopo Mondi wrote:
> > When adding a new StreamConfiguration to a CameraConfiguration instance
> > it might be useful to know how what is the index of the newly added item
> > to be able to retrieve it later.
> >
> > Return the index of the newly inserted item, which corresponds to the
> > 0-indexed number of items in the CameraConfiguration::config_ vector,
> > from CameraConfiguration::addConfiguration().
>
> Looking at the rest of the series, this is only used in 07/12 to replace
> a local counter. I'm not sure if it will also be useful for other users
> of the API, so I have no strong opinion. Please however note that you
> could already use CameraConfiguration::size() to replace the counter,
> without this patch. I'll trust you to decide which course of action is
> best (and the camera configuration API will be reworked, so this may not
> matter that much).
>
> Acked-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  include/libcamera/camera.h | 2 +-
> >  src/libcamera/camera.cpp   | 5 ++++-
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > index 272c12c3c473..53c32afa23a5 100644
> > --- a/include/libcamera/camera.h
> > +++ b/include/libcamera/camera.h
> > @@ -39,7 +39,7 @@ public:
> >
> >       virtual ~CameraConfiguration();
> >
> > -     void addConfiguration(const StreamConfiguration &cfg);
> > +     int addConfiguration(const StreamConfiguration &cfg);
> >       virtual Status validate() = 0;
> >
> >       StreamConfiguration &at(unsigned int index);
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 4a9c19c33cfb..9a8923620e24 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -104,10 +104,13 @@ CameraConfiguration::~CameraConfiguration()
> >  /**
> >   * \brief Add a stream configuration to the camera configuration
> >   * \param[in] cfg The stream configuration
> > + * \return The index of the newly added stream configuration
> >   */
> > -void CameraConfiguration::addConfiguration(const StreamConfiguration &cfg)
> > +int CameraConfiguration::addConfiguration(const StreamConfiguration &cfg)
> >  {
> >       config_.push_back(cfg);
> > +
> > +     return config_.size() - 1;
> >  }
> >
> >  /**
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel