[PATCH v2 1/3] libcamera: request: Make `controls_`/`metadata_` members

[Laurent Pinchart]

On Mon, Mar 31, 2025 at 07:01:38PM +0100, Kieran Bingham wrote:
> Quoting Barnabás Pőcze (2025-03-31 15:17:12)
> > The lifetimes of these two `ControlList`s are tied entirely to the request
> > object, so simplify the code by making them member variables instead of
> > manually managing their dynamic lifetime.
> 
> Fixing the lifetimes makes sense.
> 
> Later patches move this anyway, but I think that's ok here.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> > ---
> >  include/libcamera/request.h |  8 ++++----
> >  src/libcamera/request.cpp   | 17 ++++-------------
> >  2 files changed, 8 insertions(+), 17 deletions(-)
> > 
> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> > index e214a9d13..3061e2fb0 100644
> > --- a/include/libcamera/request.h
> > +++ b/include/libcamera/request.h
> > @@ -49,8 +49,8 @@ public:
> >  
> >         void reuse(ReuseFlag flags = Default);
> >  
> > -       ControlList &controls() { return *controls_; }
> > -       ControlList &metadata() { return *metadata_; }
> > +       ControlList &controls() { return controls_; }
> > +       ControlList &metadata() { return metadata_; }
> >         const BufferMap &buffers() const { return bufferMap_; }
> >         int addBuffer(const Stream *stream, FrameBuffer *buffer,
> >                       std::unique_ptr<Fence> fence = nullptr);
> > @@ -67,8 +67,8 @@ public:
> >  private:
> >         LIBCAMERA_DISABLE_COPY(Request)
> >  
> > -       ControlList *controls_;
> > -       ControlList *metadata_;
> > +       ControlList controls_;
> > +       ControlList metadata_;
> >         BufferMap bufferMap_;
> >  
> >         const uint64_t cookie_;
> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> > index b206ac132..e7eb1c0c8 100644
> > --- a/src/libcamera/request.cpp
> > +++ b/src/libcamera/request.cpp
> > @@ -354,16 +354,10 @@ void Request::Private::timeout()
> >   */
> >  Request::Request(Camera *camera, uint64_t cookie)
> >         : Extensible(std::make_unique<Private>(camera)),
> > +         controls_(controls::controls, camera->_d()->validator()),
> > +         metadata_(controls::controls), /* \todo Add a validator for metadata controls. */
> >           cookie_(cookie), status_(RequestPending)
> >  {
> > -       controls_ = new ControlList(controls::controls,
> > -                                   camera->_d()->validator());
> > -
> > -       /**
> > -        * \todo Add a validator for metadata controls.
> > -        */
> > -       metadata_ = new ControlList(controls::controls);
> > -
> >         LIBCAMERA_TRACEPOINT(request_construct, this);
> >  
> >         LOG(Request, Debug) << "Created request - cookie: " << cookie_;
> > @@ -372,9 +366,6 @@ Request::Request(Camera *camera, uint64_t cookie)
> >  Request::~Request()
> >  {
> >         LIBCAMERA_TRACEPOINT(request_destroy, this);
> > -
> > -       delete metadata_;
> > -       delete controls_;
> >  }
> >  
> >  /**
> > @@ -405,8 +396,8 @@ void Request::reuse(ReuseFlag flags)
> >  
> >         status_ = RequestPending;
> >  
> > -       controls_->clear();
> > -       metadata_->clear();
> > +       controls_.clear();
> > +       metadata_.clear();
> >  }
> >  
> >  /**

-- 
Regards,

Laurent Pinchart