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

23070 diff mbox series

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.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 include/libcamera/request.h |  8 ++++----
 src/libcamera/request.cpp   | 17 ++++-------------
 2 files changed, 8 insertions(+), 17 deletions(-)

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@ideasonboard.com>

> Signed-off-by: Barnabás Pőcze <barnabas.pocze@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();
>  }
>  
>  /**
> -- 
> 2.49.0
>

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@ideasonboard.com>

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

> > Signed-off-by: Barnabás Pőcze <barnabas.pocze@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();
> >  }
> >  
> >  /**