[PATCH v1] libcamera: request: Make `controls_`/`metadata_` members

[Kieran Bingham]

Quoting Barnabás Pőcze (2025-03-10 17:03:43)
> 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.
> 

This one looks reasonable, but I suspect it changes the ABI in some
form?

Could you run 

./utils/abi-compat.sh

And check the report please? I'd like to ensure that any patches which
affect ABI or API breakage include that statement in the commit message
so we can identify them in the release notes. (Another candidate for the
CI builds too of course)

--
Kieran

> 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_;

If we're modifying Request public ABI - should they in fact go into the
Request::Private class though ?

>         BufferMap bufferMap_;
>  
>         const uint64_t cookie_;
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 6fa1801a0..59fc3fdf9 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -351,16 +351,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_;
> @@ -369,9 +363,6 @@ Request::Request(Camera *camera, uint64_t cookie)
>  Request::~Request()
>  {
>         LIBCAMERA_TRACEPOINT(request_destroy, this);
> -
> -       delete metadata_;
> -       delete controls_;
>  }
>  
>  /**
> @@ -402,8 +393,8 @@ void Request::reuse(ReuseFlag flags)
>  
>         status_ = RequestPending;
>  
> -       controls_->clear();
> -       metadata_->clear();
> +       controls_.clear();
> +       metadata_.clear();
>  }
>  
>  /**
> -- 
> 2.48.1
>