[RFC,v1,15/23] libcamera: pipeline_handler: Add metadataAvailable() function

23496 diff mbox series

From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Currently the way pipeline handlers have to store metadata results
for a Request is to access the Request::metadata_ list directly and
either merge a ControlList or set a single control value there.

Direct access to Request::metadata_ is however problematic as even if
metadata would be available earlier, pipeline handlers can only
accumulate the results in Request::metadata_ and they're only available
for applications at Request complete time.

Instead of letting pipeline handlers access Request::metadata_ directly
provide two helper functions, similar in spirit to
PipelineHandler::completeBuffer() and
PipelineHandler::completeRequest(), to allow pipeline handlers to
notify early availability of metadata.

Provide two overloads, one that accepts a ControlList and merges it into
Request::metadata_ and one that allows to set a single metadata result
there without going through an intermediate copy.

The newly provided helpers trigger the Camera::availableMetadata signal
from where applications can retrieve the list of controls that have
just been made available by the pipeline handler.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
[Fill both metadat lists, use `type_identity`, adjust commit message.]
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
Original: https://patchwork.libcamera.org/patch/22229/
---
 include/libcamera/internal/pipeline_handler.h | 21 +++++++
 src/libcamera/pipeline_handler.cpp            | 61 +++++++++++++++++++
 2 files changed, 82 insertions(+)

Hi Barnabás

On Fri, Jun 06, 2025 at 06:41:48PM +0200, Barnabás Pőcze wrote:
> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Currently the way pipeline handlers have to store metadata results
> for a Request is to access the Request::metadata_ list directly and
> either merge a ControlList or set a single control value there.
>
> Direct access to Request::metadata_ is however problematic as even if
> metadata would be available earlier, pipeline handlers can only
> accumulate the results in Request::metadata_ and they're only available
> for applications at Request complete time.
>
> Instead of letting pipeline handlers access Request::metadata_ directly
> provide two helper functions, similar in spirit to
> PipelineHandler::completeBuffer() and
> PipelineHandler::completeRequest(), to allow pipeline handlers to
> notify early availability of metadata.
>
> Provide two overloads, one that accepts a ControlList and merges it into
> Request::metadata_ and one that allows to set a single metadata result
> there without going through an intermediate copy.
>
> The newly provided helpers trigger the Camera::availableMetadata signal
> from where applications can retrieve the list of controls that have
> just been made available by the pipeline handler.

This paragraphs should be updated to report that that the
Camera::availableMetadata signal transports a Diff which can be used
by applications.

Also I would point out that this patch currently populates both the
existing Request::metadata_ list and the metadata2_ list as long as we
don't replace the former with the latter.

>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> [Fill both metadat lists, use `type_identity`, adjust commit message.]
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
> Original: https://patchwork.libcamera.org/patch/22229/
> ---
>  include/libcamera/internal/pipeline_handler.h | 21 +++++++
>  src/libcamera/pipeline_handler.cpp            | 61 +++++++++++++++++++
>  2 files changed, 82 insertions(+)
>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index 972a2fa65..bdec80589 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -13,11 +13,15 @@
>  #include <sys/types.h>
>  #include <vector>
>
> +#include <libcamera/base/details/cxx20.h>
>  #include <libcamera/base/object.h>
>
> +#include <libcamera/camera.h>
>  #include <libcamera/controls.h>
>  #include <libcamera/stream.h>
>
> +#include "libcamera/internal/request.h"
> +
>  namespace libcamera {
>
>  class Camera;
> @@ -58,6 +62,23 @@ public:
>  	void registerRequest(Request *request);
>  	void queueRequest(Request *request);
>
> +	void metadataAvailable(Request *request, const ControlList &metadata);
> +
> +	template<typename T>
> +	void metadataAvailable(Request *request, const Control<T> &ctrl,
> +			       const details::cxx20::type_identity_t<T> &value)
> +	{
> +		auto &m = request->metadata2();
> +		const auto c = m.checkpoint();
> +
> +		m.set(ctrl, value);
> +		request->metadata().set(ctrl, value);
> +
> +		const auto d = c.diffSince();
> +		if (d)
> +			request->_d()->camera()->metadataAvailable.emit(request, d);
> +	}
> +
>  	bool completeBuffer(Request *request, FrameBuffer *buffer);
>  	void completeRequest(Request *request);
>  	void cancelRequest(Request *request);
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index d84dff3c9..6a9d11241 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -507,6 +507,67 @@ void PipelineHandler::doQueueRequests()
>   * \return 0 on success or a negative error code otherwise
>   */
>
> +/**
> + * \brief Notify the availability of a list of metadata for \a request
> + * \param[in] request The request the metadata belongs to
> + * \param[in] metadata The metadata list
> + *
> + * This function should be called multiple times by pipeline handlers to signal
> + * the availability of a list of metadata results. It notifies applications
> + * by triggering the Camera::availableMetadata signal and accumulates the
> + * metadata results in Request::metadata().
> + *
> + * Early metadata completion allows pipeline handlers to fast track delivery of
> + * metadata results as soon as they are available before the completion of \a
> + * request. The full list of metadata results of a Request is available at
> + * Request completion time in Request::metadata().
> + *
> + * A metadata key is expected to be notified at most once. Metadata keys
> + * notified multiple times are ignored.
> + *
> + * This overload allows to signal the availability of a list of metadata and
> + * merges them in the Request::metadata() list. This operations is expensive
> + * as controls are copied from \a metadata to Request::metadata().

I would have proposed to adjust this documentation to mention
metadata2_ but as we're going to replace metadata_ with metadata2_
this documentation will be up-to-date with the final result of this
series

> + *
> + * \context This function shall be called from the CameraManager thread.
> + */
> +void PipelineHandler::metadataAvailable(Request *request, const ControlList &metadata)
> +{
> +	request->metadata().merge(metadata);
> +
> +	const auto d = request->metadata2().merge(metadata);
> +	if (d)
> +		request->_d()->camera()->metadataAvailable.emit(request, d);
> +}
> +
> +/**
> + * \fn void PipelineHandler::metadataAvailable(Request *request, const Control<T> &ctrl,
> + *					       const details::cxx20::type_identity_t<T> &value)
> + * \brief Notify the availability of a metadata result for \a request
> + * \param[in] request The request the metadata belongs to
> + * \param[in] ctrl The control id to notify
> + * \param[in] value the control value
> + *
> + * This function should be called multiple times by pipeline handlers to signal
> + * the availability of a metadata result. It notifies applications
> + * by triggering the Camera::availableMetadata signal and accumulates the
> + * metadata result in Request::metadata().
> + *
> + * Early metadata completion allows pipeline handlers to fast track delivery of
> + * metadata results as soon as they are available before the completion of \a
> + * request. The full list of metadata results of a Request is available at
> + * Request completion time in Request::metadata().
> + *
> + * A metadata key is expected to be notified at most once. Metadata keys
> + * notified multiple times are ignored.
> + *
> + * This overload allows to signal the availability of a single metadata and
> + * merge \a value in the Request::metadata() list. This operations copies \a
> + * value in the Request::metadata() list without creating intermediate copies.
> + *
> + * \context This function shall be called from the CameraManager thread.
> + */
> +

FWIW
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>  /**
>   * \brief Complete a buffer for a request
>   * \param[in] request The request the buffer belongs to
> --
> 2.49.0
>

Hi

2025. 06. 19. 15:02 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Fri, Jun 06, 2025 at 06:41:48PM +0200, Barnabás Pőcze wrote:
>> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>>
>> Currently the way pipeline handlers have to store metadata results
>> for a Request is to access the Request::metadata_ list directly and
>> either merge a ControlList or set a single control value there.
>>
>> Direct access to Request::metadata_ is however problematic as even if
>> metadata would be available earlier, pipeline handlers can only
>> accumulate the results in Request::metadata_ and they're only available
>> for applications at Request complete time.
>>
>> Instead of letting pipeline handlers access Request::metadata_ directly
>> provide two helper functions, similar in spirit to
>> PipelineHandler::completeBuffer() and
>> PipelineHandler::completeRequest(), to allow pipeline handlers to
>> notify early availability of metadata.
>>
>> Provide two overloads, one that accepts a ControlList and merges it into
>> Request::metadata_ and one that allows to set a single metadata result
>> there without going through an intermediate copy.
>>
>> The newly provided helpers trigger the Camera::availableMetadata signal
>> from where applications can retrieve the list of controls that have
>> just been made available by the pipeline handler.
> 
> This paragraphs should be updated to report that that the
> Camera::availableMetadata signal transports a Diff which can be used
> by applications.

Hmm... I thought about that but I came to the conclusion that the description
is general enough to be considered acceptable (at least by me). Do you have any
concrete text in mind?


> 
> Also I would point out that this patch currently populates both the
> existing Request::metadata_ list and the metadata2_ list as long as we
> don't replace the former with the latter.
> 
>>
>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>> [Fill both metadat lists, use `type_identity`, adjust commit message.]
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>> Original: https://patchwork.libcamera.org/patch/22229/
>> ---
>>   include/libcamera/internal/pipeline_handler.h | 21 +++++++
>>   src/libcamera/pipeline_handler.cpp            | 61 +++++++++++++++++++
>>   2 files changed, 82 insertions(+)
> [...]


Regards,
Barnabás Pőcze

Hi Barnabás

On Mon, Jun 23, 2025 at 04:37:25PM +0200, Barnabás Pőcze wrote:
> Hi
>
> 2025. 06. 19. 15:02 keltezéssel, Jacopo Mondi írta:
> > Hi Barnabás
> >
> > On Fri, Jun 06, 2025 at 06:41:48PM +0200, Barnabás Pőcze wrote:
> > > From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > >
> > > Currently the way pipeline handlers have to store metadata results
> > > for a Request is to access the Request::metadata_ list directly and
> > > either merge a ControlList or set a single control value there.
> > >
> > > Direct access to Request::metadata_ is however problematic as even if
> > > metadata would be available earlier, pipeline handlers can only
> > > accumulate the results in Request::metadata_ and they're only available
> > > for applications at Request complete time.
> > >
> > > Instead of letting pipeline handlers access Request::metadata_ directly
> > > provide two helper functions, similar in spirit to
> > > PipelineHandler::completeBuffer() and
> > > PipelineHandler::completeRequest(), to allow pipeline handlers to
> > > notify early availability of metadata.
> > >
> > > Provide two overloads, one that accepts a ControlList and merges it into
> > > Request::metadata_ and one that allows to set a single metadata result
> > > there without going through an intermediate copy.
> > >
> > > The newly provided helpers trigger the Camera::availableMetadata signal
> > > from where applications can retrieve the list of controls that have
> > > just been made available by the pipeline handler.
> >
> > This paragraphs should be updated to report that that the
> > Camera::availableMetadata signal transports a Diff which can be used
> > by applications.
>
> Hmm... I thought about that but I came to the conclusion that the description
> is general enough to be considered acceptable (at least by me). Do you have any
> concrete text in mind?
>

Since this paragraph comes straight from my old patch where
Camera::availableMetadata used to transport ControlId I thought it
would have been nice to update it to mention that it transports a
MetadataListDiff.

Up to you, it's the commit message, as you said, it's general enough
not to be wrong.

>
> >
> > Also I would point out that this patch currently populates both the
> > existing Request::metadata_ list and the metadata2_ list as long as we
> > don't replace the former with the latter.
> >
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > [Fill both metadat lists, use `type_identity`, adjust commit message.]
> > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > > ---
> > > Original: https://patchwork.libcamera.org/patch/22229/
> > > ---
> > >   include/libcamera/internal/pipeline_handler.h | 21 +++++++
> > >   src/libcamera/pipeline_handler.cpp            | 61 +++++++++++++++++++
> > >   2 files changed, 82 insertions(+)
> > [...]
>
>
> Regards,
> Barnabás Pőcze
>