[RFC,v3,11/22] libcamera: request: Store `MetadataList`

24914 diff mbox series

Add a `MetadataList` to the `Request` object to store the metadata items
returned by the camera. The metadata list is initialized using the camera's
`MetadataListPlan` object, which is supposed to be filled by the pipeline
handlers.

This new list is exposed as `metadata2`, and the old `ControlList` is
still present for now.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
---
 include/libcamera/request.h | 5 +++++
 src/libcamera/request.cpp   | 2 ++
 2 files changed, 7 insertions(+)

Quoting Barnabás Pőcze (2025-10-30 16:58:05)
> Add a `MetadataList` to the `Request` object to store the metadata items
> returned by the camera. The metadata list is initialized using the camera's
> `MetadataListPlan` object, which is supposed to be filled by the pipeline
> handlers.
> 
> This new list is exposed as `metadata2`, and the old `ControlList` is
> still present for now.

Why are we keeping metadata() as a ControlList? will we remove it and
replace as a MetadataList ?

Do applications have to use the new list quite differently ?

> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  include/libcamera/request.h | 5 +++++
>  src/libcamera/request.cpp   | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 0c5939f7b3..23e7bde72c 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -18,6 +18,7 @@
>  
>  #include <libcamera/controls.h>
>  #include <libcamera/fence.h>
> +#include <libcamera/metadata_list.h>
>  
>  namespace libcamera {
>  
> @@ -51,6 +52,9 @@ public:
>  
>         ControlList &controls() { return *controls_; }
>         ControlList &metadata() { return *metadata_; }
> +#ifndef __DOXYGEN__
> +       [[nodiscard]] MetadataList &metadata2() { return metadata2_; }
> +#endif
>         const BufferMap &buffers() const { return bufferMap_; }
>         int addBuffer(const Stream *stream, FrameBuffer *buffer,
>                       std::unique_ptr<Fence> &&fence = {});
> @@ -69,6 +73,7 @@ private:
>  
>         ControlList *controls_;
>         ControlList *metadata_;
> +       MetadataList metadata2_;
>         BufferMap bufferMap_;
>  
>         const uint64_t cookie_;
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 992d476e09..0ff465e6f7 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -357,6 +357,7 @@ void Request::Private::timeout()
>   */
>  Request::Request(Camera *camera, uint64_t cookie)
>         : Extensible(std::make_unique<Private>(camera)),
> +         metadata2_(camera->_d()->metadataPlan_),
>           cookie_(cookie), status_(RequestPending)
>  {
>         controls_ = new ControlList(controls::controls,
> @@ -410,6 +411,7 @@ void Request::reuse(ReuseFlag flags)
>  
>         controls_->clear();
>         metadata_->clear();
> +       metadata2_.clear();
>  }
>  
>  /**
> -- 
> 2.51.1
>

Hi

2025. 11. 02. 16:14 keltezéssel, Kieran Bingham írta:
> Quoting Barnabás Pőcze (2025-10-30 16:58:05)
>> Add a `MetadataList` to the `Request` object to store the metadata items
>> returned by the camera. The metadata list is initialized using the camera's
>> `MetadataListPlan` object, which is supposed to be filled by the pipeline
>> handlers.
>>
>> This new list is exposed as `metadata2`, and the old `ControlList` is
>> still present for now.
> 
> Why are we keeping metadata() as a ControlList? will we remove it and
> replace as a MetadataList ?

The two lists are kept mainly for testing. The intention is to remove the
`ControlList` one. Also see the last change in this series, which is not
too small and would have to be incorporated here if the switch was done right away.


> 
> Do applications have to use the new list quite differently ?

The intention is that changes are minimal, but if something hard-codes
e.g. the type `ControlList`, then it has to change.


> 
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
>> ---
>>   include/libcamera/request.h | 5 +++++
>>   src/libcamera/request.cpp   | 2 ++
>>   2 files changed, 7 insertions(+)
>>
>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
>> index 0c5939f7b3..23e7bde72c 100644
>> --- a/include/libcamera/request.h
>> +++ b/include/libcamera/request.h
>> @@ -18,6 +18,7 @@
>>   
>>   #include <libcamera/controls.h>
>>   #include <libcamera/fence.h>
>> +#include <libcamera/metadata_list.h>
>>   
>>   namespace libcamera {
>>   
>> @@ -51,6 +52,9 @@ public:
>>   
>>          ControlList &controls() { return *controls_; }
>>          ControlList &metadata() { return *metadata_; }
>> +#ifndef __DOXYGEN__
>> +       [[nodiscard]] MetadataList &metadata2() { return metadata2_; }
>> +#endif
>>          const BufferMap &buffers() const { return bufferMap_; }
>>          int addBuffer(const Stream *stream, FrameBuffer *buffer,
>>                        std::unique_ptr<Fence> &&fence = {});
>> @@ -69,6 +73,7 @@ private:
>>   
>>          ControlList *controls_;
>>          ControlList *metadata_;
>> +       MetadataList metadata2_;
>>          BufferMap bufferMap_;
>>   
>>          const uint64_t cookie_;
>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
>> index 992d476e09..0ff465e6f7 100644
>> --- a/src/libcamera/request.cpp
>> +++ b/src/libcamera/request.cpp
>> @@ -357,6 +357,7 @@ void Request::Private::timeout()
>>    */
>>   Request::Request(Camera *camera, uint64_t cookie)
>>          : Extensible(std::make_unique<Private>(camera)),
>> +         metadata2_(camera->_d()->metadataPlan_),
>>            cookie_(cookie), status_(RequestPending)
>>   {
>>          controls_ = new ControlList(controls::controls,
>> @@ -410,6 +411,7 @@ void Request::reuse(ReuseFlag flags)
>>   
>>          controls_->clear();
>>          metadata_->clear();
>> +       metadata2_.clear();
>>   }
>>   
>>   /**
>> -- 
>> 2.51.1
>>