Message ID | 20250606164156.1442682-16-barnabas.pocze@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
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 >
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(). + * + * \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. + */ + /** * \brief Complete a buffer for a request * \param[in] request The request the buffer belongs to