[v1,2/8] libcamera: Add a DebugMetadata helper

21473 diff mbox series

Debug metadata often occurs in places where the metadata control list is
not available e.g. in queueRequest() or processStatsBuffer() or even in
a class far away from the metadata handling code. It is therefore
difficult to add debug metadata without adding lot's of boilerplate
code. This can be mitigated by recording the metadata and forwarding it
to an upstream helper or the metadata control list when they become
available. Add such a helper.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 include/libcamera/internal/debug_controls.h |  52 +++++++
 include/libcamera/internal/meson.build      |   1 +
 src/libcamera/control_ids_core.yaml         |   6 +
 src/libcamera/debug_controls.cpp            | 144 ++++++++++++++++++++
 src/libcamera/meson.build                   |   1 +
 5 files changed, 204 insertions(+)
 create mode 100644 include/libcamera/internal/debug_controls.h
 create mode 100644 src/libcamera/debug_controls.cpp

Hi Stefan,

Thank you for the patch.

On Wed, Oct 02, 2024 at 06:19:20PM +0200, Stefan Klug wrote:
> Debug metadata often occurs in places where the metadata control list is
> not available e.g. in queueRequest() or processStatsBuffer() or even in
> a class far away from the metadata handling code. It is therefore
> difficult to add debug metadata without adding lot's of boilerplate

s/lot's/lots/

> code. This can be mitigated by recording the metadata and forwarding it
> to an upstream helper or the metadata control list when they become
> available. Add such a helper.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  include/libcamera/internal/debug_controls.h |  52 +++++++
>  include/libcamera/internal/meson.build      |   1 +
>  src/libcamera/control_ids_core.yaml         |   6 +
>  src/libcamera/debug_controls.cpp            | 144 ++++++++++++++++++++
>  src/libcamera/meson.build                   |   1 +
>  5 files changed, 204 insertions(+)
>  create mode 100644 include/libcamera/internal/debug_controls.h
>  create mode 100644 src/libcamera/debug_controls.cpp
> 
> diff --git a/include/libcamera/internal/debug_controls.h b/include/libcamera/internal/debug_controls.h
> new file mode 100644
> index 000000000000..d95c03e1db1f
> --- /dev/null
> +++ b/include/libcamera/internal/debug_controls.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas on Board Oy
> + *
> + * Debug metadata helpers
> + */
> +
> +#pragma once
> +
> +#include <libcamera/control_ids.h>
> +
> +namespace libcamera {
> +
> +class DebugMetadata
> +{
> +public:
> +	DebugMetadata() = default;
> +
> +	void checkForEnable(const ControlList &controls);
> +	void enable(bool enable = true);
> +	void assignUpstream(DebugMetadata *upstream);
> +	void assignControlList(ControlList *list);
> +
> +	template<typename T, typename V>
> +	void set(const Control<T> &ctrl, const V &value)
> +	{
> +		if (upstream_) {
> +			upstream_->set<T, V>(ctrl, value);

Do you need to explicitly mention <T, V> here, won't type deduction work
automatically ? Same below.

> +			return;
> +		}
> +
> +		if (!enabled_)
> +			return;
> +
> +		if (list_) {
> +			list_->set<T, V>(ctrl, value);
> +			return;
> +		}
> +
> +		cache_.set<T, V>(ctrl, value);
> +	}
> +
> +	void set(unsigned int id, const ControlValue &value);
> +
> +private:
> +	bool enabled_ = false;
> +	ControlList *list_ = nullptr;
> +	DebugMetadata *upstream_ = nullptr;
> +	ControlList cache_;
> +};
> +
> +} /* namespace libcamera */
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 1c5eef9cab80..1dddcd50c90b 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -14,6 +14,7 @@ libcamera_internal_headers = files([
>      'control_serializer.h',
>      'control_validator.h',
>      'converter.h',
> +    'debug_controls.h',
>      'delayed_controls.h',
>      'device_enumerator.h',
>      'device_enumerator_sysfs.h',
> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> index 1b1bd9507d25..103bcb593c4a 100644
> --- a/src/libcamera/control_ids_core.yaml
> +++ b/src/libcamera/control_ids_core.yaml
> @@ -967,5 +967,11 @@ controls:
>  
>          The default gamma value must be 2.2 which closely mimics sRGB gamma.
>          Note that this is camera gamma, so it is applied as 1.0/gamma.
> +        
> +  - DebugMetadataEnable:
> +      type: bool
> +      description: |
> +        Enable or disable the debug metadata.
> +

Extra blank line.

>  
>  ...
> diff --git a/src/libcamera/debug_controls.cpp b/src/libcamera/debug_controls.cpp
> new file mode 100644
> index 000000000000..11a92a6b5871
> --- /dev/null
> +++ b/src/libcamera/debug_controls.cpp
> @@ -0,0 +1,144 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas on Board Oy.
> + *
> + * Helper to easily record debug metadata inside libcamera.
> + */
> +
> +#include "libcamera/internal/debug_controls.h"
> +
> +namespace libcamera {
> +
> +/**
> + * \file debug_controls.h
> + * \brief Helper to easily record debug metadata inside libcamera
> + */
> +
> +/**
> + * \class DebugMetadata
> + * \brief Helper to record metadata for later use
> + *
> + * When one wants to record debug metadata, the metadata list is often not
> + * directly available (either because we are inside process() of an IPA or
> + * because we are in a closed module). This class allows to record the data and
> + * at a later point in time forward it either to another DebugMetadata instance
> + * or to a ControlList.
> + */
> +
> +/**
> + * \fn DebugMetadata::checkForEnable
> + * \brief Check for DebugMetadataEnable in the supplied ControlList
> + * \param[in] controls The supplied ControlList
> + *
> + * Looks for controls::DebugMetadataEnable and enables or disables debug
> + * metadata handling accordingly.
> + */
> +void DebugMetadata::checkForEnable(const ControlList &controls)
> +{
> +	const auto &ctrl = controls.get(controls::DebugMetadataEnable);
> +	if (ctrl)
> +		enable(*ctrl);
> +}
> +
> +/**
> + * \fn DebugMetadata::enable
> + * \brief Enables or disabled metadata handling
> + * \param[in] enable The enable state
> + *
> + * Enables or disables metadata handling according to \a enable. When enable is
> + * false, the cache gets cleared and no further metadata is recorded.
> + */
> +void DebugMetadata::enable(bool enable)
> +{
> +	enabled_ = enable;
> +	if (!enabled_)
> +		cache_.clear();
> +}
> +
> +/**
> + * \fn DebugMetadata::assignUpstream
> + * \brief Assign an upstream metadata handler
> + * \param[in] upstream Pointer to the upstream handler
> + *
> + * When a upstream gets set, the cache is copies to that list and all further

s/copies/copied/

> + * calls to DebugMetadata::set are forwarded to that list.
> + *
> + * The upstream can be reset by passing a nullptr.

I think "parent" is a better name than "upstream", the parent concept is
widely used. I'd call the function setParent() as we use "set" rather
than "assign".

> + */
> +void DebugMetadata::assignUpstream(DebugMetadata *upstream)
> +{
> +	upstream_ = upstream;
> +
> +	if (!upstream_)
> +		return;
> +
> +	for (const auto &ctrl : cache_)
> +		upstream_->set(ctrl.first, ctrl.second);
> +
> +	cache_.clear();
> +}
> +
> +/**
> + * \fn DebugMetadata::assignControlList
> + * \brief Assign a control list
> + * \param[in] list Pointer to the list
> + *
> + * When a list gets set, the cache is copies to that list and all further calls

s/copies/copied/

> + * to DebugMetadata::set are forwarded to that list.
> + *
> + * The upstream can be reset by passing a nullptr.

s/upstream/list/

> + */
> +void DebugMetadata::assignControlList(ControlList *list)
> +{
> +	list_ = list;
> +
> +	if (list_) {
> +		list_->merge(cache_);
> +		cache_.clear();
> +	}
> +}
> +
> +/**
> + * \fn DebugMetadata::set(const Control<T> &ctrl, const V &value)
> + * \brief Set a value
> + * \param[in] ctrl The ctrl to set
> + * \param[in] value The control value
> + *
> + * Sets the debug metadata for ctrl to value \a value. If an upstream or a
> + * control list is set, the value gets passed there (upstream takes precedence).
> + * Otherwise the value is cached until one of them gets assigned.
> + *
> + * When the instance is disabled and upstream is not set, the value gets dropped.
> + */
> +
> +/**
> + * \fn DebugMetadata::set(unsigned int id, const ControlValue &value)
> + * \brief Set a value
> + * \param[in] id The id of the control
> + * \param[in] value The control value
> + *
> + * Sets the debug metadata to value \a value. If an upstream or a control list
> + * is set, the value gets passed there (upstream takes precedence).
> + * Otherwise the value is cached until one of them gets assigned.
> + *
> + * When the instance is disabled and upstream is not set, the value gets dropped.
> + */
> +void DebugMetadata::set(unsigned int id, const ControlValue &value)
> +{
> +	if (upstream_) {
> +		upstream_->set(id, value);
> +		return;
> +	}
> +
> +	if (!enabled_)
> +		return;
> +
> +	if (list_) {
> +		list_->set(id, value);
> +		return;
> +	}
> +
> +	cache_.set(id, value);
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index aa9ab0291854..f7b5ee8dcc34 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -25,6 +25,7 @@ libcamera_internal_sources = files([
>      'control_validator.cpp',
>      'converter.cpp',
>      'delayed_controls.cpp',
> +    'debug_controls.cpp',
>      'device_enumerator.cpp',
>      'device_enumerator_sysfs.cpp',
>      'dma_buf_allocator.cpp',

Hi Laurent,

Thank you for the review. 

On Fri, Oct 04, 2024 at 12:23:05AM +0300, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thank you for the patch.
> 
> On Wed, Oct 02, 2024 at 06:19:20PM +0200, Stefan Klug wrote:
> > Debug metadata often occurs in places where the metadata control list is
> > not available e.g. in queueRequest() or processStatsBuffer() or even in
> > a class far away from the metadata handling code. It is therefore
> > difficult to add debug metadata without adding lot's of boilerplate
> 
> s/lot's/lots/
> 
> > code. This can be mitigated by recording the metadata and forwarding it
> > to an upstream helper or the metadata control list when they become
> > available. Add such a helper.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >  include/libcamera/internal/debug_controls.h |  52 +++++++
> >  include/libcamera/internal/meson.build      |   1 +
> >  src/libcamera/control_ids_core.yaml         |   6 +
> >  src/libcamera/debug_controls.cpp            | 144 ++++++++++++++++++++
> >  src/libcamera/meson.build                   |   1 +
> >  5 files changed, 204 insertions(+)
> >  create mode 100644 include/libcamera/internal/debug_controls.h
> >  create mode 100644 src/libcamera/debug_controls.cpp
> > 
> > diff --git a/include/libcamera/internal/debug_controls.h b/include/libcamera/internal/debug_controls.h
> > new file mode 100644
> > index 000000000000..d95c03e1db1f
> > --- /dev/null
> > +++ b/include/libcamera/internal/debug_controls.h
> > @@ -0,0 +1,52 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2024, Ideas on Board Oy
> > + *
> > + * Debug metadata helpers
> > + */
> > +
> > +#pragma once
> > +
> > +#include <libcamera/control_ids.h>
> > +
> > +namespace libcamera {
> > +
> > +class DebugMetadata
> > +{
> > +public:
> > +	DebugMetadata() = default;
> > +
> > +	void checkForEnable(const ControlList &controls);
> > +	void enable(bool enable = true);
> > +	void assignUpstream(DebugMetadata *upstream);
> > +	void assignControlList(ControlList *list);
> > +
> > +	template<typename T, typename V>
> > +	void set(const Control<T> &ctrl, const V &value)
> > +	{
> > +		if (upstream_) {
> > +			upstream_->set<T, V>(ctrl, value);
> 
> Do you need to explicitly mention <T, V> here, won't type deduction work
> automatically ? Same below.

Interesting question. I would say it should work. But I can't tell for
sure if there could be cases where it doesn't work. By specifying it
explicitly I ensure that the user of the function can specify it
manually if needed. Can you say for sure, that there no cases
where auto deduction fails?

> 
> > +			return;
> > +		}
> > +
> > +		if (!enabled_)
> > +			return;
> > +
> > +		if (list_) {
> > +			list_->set<T, V>(ctrl, value);
> > +			return;
> > +		}
> > +
> > +		cache_.set<T, V>(ctrl, value);
> > +	}
> > +
> > +	void set(unsigned int id, const ControlValue &value);
> > +
> > +private:
> > +	bool enabled_ = false;
> > +	ControlList *list_ = nullptr;
> > +	DebugMetadata *upstream_ = nullptr;
> > +	ControlList cache_;
> > +};
> > +
> > +} /* namespace libcamera */
> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> > index 1c5eef9cab80..1dddcd50c90b 100644
> > --- a/include/libcamera/internal/meson.build
> > +++ b/include/libcamera/internal/meson.build
> > @@ -14,6 +14,7 @@ libcamera_internal_headers = files([
> >      'control_serializer.h',
> >      'control_validator.h',
> >      'converter.h',
> > +    'debug_controls.h',
> >      'delayed_controls.h',
> >      'device_enumerator.h',
> >      'device_enumerator_sysfs.h',
> > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> > index 1b1bd9507d25..103bcb593c4a 100644
> > --- a/src/libcamera/control_ids_core.yaml
> > +++ b/src/libcamera/control_ids_core.yaml
> > @@ -967,5 +967,11 @@ controls:
> >  
> >          The default gamma value must be 2.2 which closely mimics sRGB gamma.
> >          Note that this is camera gamma, so it is applied as 1.0/gamma.
> > +        
> > +  - DebugMetadataEnable:
> > +      type: bool
> > +      description: |
> > +        Enable or disable the debug metadata.
> > +
> 
> Extra blank line.
> 
> >  
> >  ...
> > diff --git a/src/libcamera/debug_controls.cpp b/src/libcamera/debug_controls.cpp
> > new file mode 100644
> > index 000000000000..11a92a6b5871
> > --- /dev/null
> > +++ b/src/libcamera/debug_controls.cpp
> > @@ -0,0 +1,144 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2024, Ideas on Board Oy.
> > + *
> > + * Helper to easily record debug metadata inside libcamera.
> > + */
> > +
> > +#include "libcamera/internal/debug_controls.h"
> > +
> > +namespace libcamera {
> > +
> > +/**
> > + * \file debug_controls.h
> > + * \brief Helper to easily record debug metadata inside libcamera
> > + */
> > +
> > +/**
> > + * \class DebugMetadata
> > + * \brief Helper to record metadata for later use
> > + *
> > + * When one wants to record debug metadata, the metadata list is often not
> > + * directly available (either because we are inside process() of an IPA or
> > + * because we are in a closed module). This class allows to record the data and
> > + * at a later point in time forward it either to another DebugMetadata instance
> > + * or to a ControlList.
> > + */
> > +
> > +/**
> > + * \fn DebugMetadata::checkForEnable
> > + * \brief Check for DebugMetadataEnable in the supplied ControlList
> > + * \param[in] controls The supplied ControlList
> > + *
> > + * Looks for controls::DebugMetadataEnable and enables or disables debug
> > + * metadata handling accordingly.
> > + */
> > +void DebugMetadata::checkForEnable(const ControlList &controls)
> > +{
> > +	const auto &ctrl = controls.get(controls::DebugMetadataEnable);
> > +	if (ctrl)
> > +		enable(*ctrl);
> > +}
> > +
> > +/**
> > + * \fn DebugMetadata::enable
> > + * \brief Enables or disabled metadata handling
> > + * \param[in] enable The enable state
> > + *
> > + * Enables or disables metadata handling according to \a enable. When enable is
> > + * false, the cache gets cleared and no further metadata is recorded.
> > + */
> > +void DebugMetadata::enable(bool enable)
> > +{
> > +	enabled_ = enable;
> > +	if (!enabled_)
> > +		cache_.clear();
> > +}
> > +
> > +/**
> > + * \fn DebugMetadata::assignUpstream
> > + * \brief Assign an upstream metadata handler
> > + * \param[in] upstream Pointer to the upstream handler
> > + *
> > + * When a upstream gets set, the cache is copies to that list and all further
> 
> s/copies/copied/
> 
> > + * calls to DebugMetadata::set are forwarded to that list.
> > + *
> > + * The upstream can be reset by passing a nullptr.
> 
> I think "parent" is a better name than "upstream", the parent concept is
> widely used. I'd call the function setParent() as we use "set" rather
> than "assign".

As discussed, I will change that.

> 
> > + */
> > +void DebugMetadata::assignUpstream(DebugMetadata *upstream)
> > +{
> > +	upstream_ = upstream;
> > +
> > +	if (!upstream_)
> > +		return;
> > +
> > +	for (const auto &ctrl : cache_)
> > +		upstream_->set(ctrl.first, ctrl.second);
> > +
> > +	cache_.clear();
> > +}
> > +
> > +/**
> > + * \fn DebugMetadata::assignControlList
> > + * \brief Assign a control list
> > + * \param[in] list Pointer to the list
> > + *
> > + * When a list gets set, the cache is copies to that list and all further calls
> 
> s/copies/copied/
> 
> > + * to DebugMetadata::set are forwarded to that list.
> > + *
> > + * The upstream can be reset by passing a nullptr.
> 
> s/upstream/list/
> 
> > + */
> > +void DebugMetadata::assignControlList(ControlList *list)
> > +{
> > +	list_ = list;
> > +
> > +	if (list_) {
> > +		list_->merge(cache_);
> > +		cache_.clear();
> > +	}
> > +}
> > +
> > +/**
> > + * \fn DebugMetadata::set(const Control<T> &ctrl, const V &value)
> > + * \brief Set a value
> > + * \param[in] ctrl The ctrl to set
> > + * \param[in] value The control value
> > + *
> > + * Sets the debug metadata for ctrl to value \a value. If an upstream or a
> > + * control list is set, the value gets passed there (upstream takes precedence).
> > + * Otherwise the value is cached until one of them gets assigned.
> > + *
> > + * When the instance is disabled and upstream is not set, the value gets dropped.
> > + */
> > +
> > +/**
> > + * \fn DebugMetadata::set(unsigned int id, const ControlValue &value)
> > + * \brief Set a value
> > + * \param[in] id The id of the control
> > + * \param[in] value The control value
> > + *
> > + * Sets the debug metadata to value \a value. If an upstream or a control list
> > + * is set, the value gets passed there (upstream takes precedence).
> > + * Otherwise the value is cached until one of them gets assigned.
> > + *
> > + * When the instance is disabled and upstream is not set, the value gets dropped.
> > + */
> > +void DebugMetadata::set(unsigned int id, const ControlValue &value)
> > +{
> > +	if (upstream_) {
> > +		upstream_->set(id, value);
> > +		return;
> > +	}
> > +
> > +	if (!enabled_)
> > +		return;
> > +
> > +	if (list_) {
> > +		list_->set(id, value);
> > +		return;
> > +	}
> > +
> > +	cache_.set(id, value);
> > +}
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index aa9ab0291854..f7b5ee8dcc34 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -25,6 +25,7 @@ libcamera_internal_sources = files([
> >      'control_validator.cpp',
> >      'converter.cpp',
> >      'delayed_controls.cpp',
> > +    'debug_controls.cpp',
> >      'device_enumerator.cpp',
> >      'device_enumerator_sysfs.cpp',
> >      'dma_buf_allocator.cpp',
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Hi Stefan,

On Fri, Oct 04, 2024 at 04:33:36PM +0200, Stefan Klug wrote:
> On Fri, Oct 04, 2024 at 12:23:05AM +0300, Laurent Pinchart wrote:
> > On Wed, Oct 02, 2024 at 06:19:20PM +0200, Stefan Klug wrote:
> > > Debug metadata often occurs in places where the metadata control list is
> > > not available e.g. in queueRequest() or processStatsBuffer() or even in
> > > a class far away from the metadata handling code. It is therefore
> > > difficult to add debug metadata without adding lot's of boilerplate
> > 
> > s/lot's/lots/
> > 
> > > code. This can be mitigated by recording the metadata and forwarding it
> > > to an upstream helper or the metadata control list when they become
> > > available. Add such a helper.
> > > 
> > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > > ---
> > >  include/libcamera/internal/debug_controls.h |  52 +++++++
> > >  include/libcamera/internal/meson.build      |   1 +
> > >  src/libcamera/control_ids_core.yaml         |   6 +
> > >  src/libcamera/debug_controls.cpp            | 144 ++++++++++++++++++++
> > >  src/libcamera/meson.build                   |   1 +
> > >  5 files changed, 204 insertions(+)
> > >  create mode 100644 include/libcamera/internal/debug_controls.h
> > >  create mode 100644 src/libcamera/debug_controls.cpp
> > > 
> > > diff --git a/include/libcamera/internal/debug_controls.h b/include/libcamera/internal/debug_controls.h
> > > new file mode 100644
> > > index 000000000000..d95c03e1db1f
> > > --- /dev/null
> > > +++ b/include/libcamera/internal/debug_controls.h
> > > @@ -0,0 +1,52 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2024, Ideas on Board Oy
> > > + *
> > > + * Debug metadata helpers
> > > + */
> > > +
> > > +#pragma once
> > > +
> > > +#include <libcamera/control_ids.h>
> > > +
> > > +namespace libcamera {
> > > +
> > > +class DebugMetadata
> > > +{
> > > +public:
> > > +	DebugMetadata() = default;
> > > +
> > > +	void checkForEnable(const ControlList &controls);
> > > +	void enable(bool enable = true);
> > > +	void assignUpstream(DebugMetadata *upstream);
> > > +	void assignControlList(ControlList *list);
> > > +
> > > +	template<typename T, typename V>
> > > +	void set(const Control<T> &ctrl, const V &value)
> > > +	{
> > > +		if (upstream_) {
> > > +			upstream_->set<T, V>(ctrl, value);
> > 
> > Do you need to explicitly mention <T, V> here, won't type deduction work
> > automatically ? Same below.
> 
> Interesting question. I would say it should work. But I can't tell for
> sure if there could be cases where it doesn't work. By specifying it
> explicitly I ensure that the user of the function can specify it
> manually if needed. Can you say for sure, that there no cases
> where auto deduction fails?

I don't see how it could fail.

> > > +			return;
> > > +		}
> > > +
> > > +		if (!enabled_)
> > > +			return;
> > > +
> > > +		if (list_) {
> > > +			list_->set<T, V>(ctrl, value);
> > > +			return;
> > > +		}
> > > +
> > > +		cache_.set<T, V>(ctrl, value);
> > > +	}
> > > +
> > > +	void set(unsigned int id, const ControlValue &value);
> > > +
> > > +private:
> > > +	bool enabled_ = false;
> > > +	ControlList *list_ = nullptr;
> > > +	DebugMetadata *upstream_ = nullptr;
> > > +	ControlList cache_;
> > > +};
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> > > index 1c5eef9cab80..1dddcd50c90b 100644
> > > --- a/include/libcamera/internal/meson.build
> > > +++ b/include/libcamera/internal/meson.build
> > > @@ -14,6 +14,7 @@ libcamera_internal_headers = files([
> > >      'control_serializer.h',
> > >      'control_validator.h',
> > >      'converter.h',
> > > +    'debug_controls.h',
> > >      'delayed_controls.h',
> > >      'device_enumerator.h',
> > >      'device_enumerator_sysfs.h',
> > > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> > > index 1b1bd9507d25..103bcb593c4a 100644
> > > --- a/src/libcamera/control_ids_core.yaml
> > > +++ b/src/libcamera/control_ids_core.yaml
> > > @@ -967,5 +967,11 @@ controls:
> > >  
> > >          The default gamma value must be 2.2 which closely mimics sRGB gamma.
> > >          Note that this is camera gamma, so it is applied as 1.0/gamma.
> > > +        
> > > +  - DebugMetadataEnable:
> > > +      type: bool
> > > +      description: |
> > > +        Enable or disable the debug metadata.
> > > +
> > 
> > Extra blank line.
> > 
> > >  
> > >  ...
> > > diff --git a/src/libcamera/debug_controls.cpp b/src/libcamera/debug_controls.cpp
> > > new file mode 100644
> > > index 000000000000..11a92a6b5871
> > > --- /dev/null
> > > +++ b/src/libcamera/debug_controls.cpp
> > > @@ -0,0 +1,144 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2024, Ideas on Board Oy.
> > > + *
> > > + * Helper to easily record debug metadata inside libcamera.
> > > + */
> > > +
> > > +#include "libcamera/internal/debug_controls.h"
> > > +
> > > +namespace libcamera {
> > > +
> > > +/**
> > > + * \file debug_controls.h
> > > + * \brief Helper to easily record debug metadata inside libcamera
> > > + */
> > > +
> > > +/**
> > > + * \class DebugMetadata
> > > + * \brief Helper to record metadata for later use
> > > + *
> > > + * When one wants to record debug metadata, the metadata list is often not
> > > + * directly available (either because we are inside process() of an IPA or
> > > + * because we are in a closed module). This class allows to record the data and
> > > + * at a later point in time forward it either to another DebugMetadata instance
> > > + * or to a ControlList.
> > > + */
> > > +
> > > +/**
> > > + * \fn DebugMetadata::checkForEnable
> > > + * \brief Check for DebugMetadataEnable in the supplied ControlList
> > > + * \param[in] controls The supplied ControlList
> > > + *
> > > + * Looks for controls::DebugMetadataEnable and enables or disables debug
> > > + * metadata handling accordingly.
> > > + */
> > > +void DebugMetadata::checkForEnable(const ControlList &controls)
> > > +{
> > > +	const auto &ctrl = controls.get(controls::DebugMetadataEnable);
> > > +	if (ctrl)
> > > +		enable(*ctrl);
> > > +}
> > > +
> > > +/**
> > > + * \fn DebugMetadata::enable
> > > + * \brief Enables or disabled metadata handling
> > > + * \param[in] enable The enable state
> > > + *
> > > + * Enables or disables metadata handling according to \a enable. When enable is
> > > + * false, the cache gets cleared and no further metadata is recorded.
> > > + */
> > > +void DebugMetadata::enable(bool enable)
> > > +{
> > > +	enabled_ = enable;
> > > +	if (!enabled_)
> > > +		cache_.clear();
> > > +}
> > > +
> > > +/**
> > > + * \fn DebugMetadata::assignUpstream
> > > + * \brief Assign an upstream metadata handler
> > > + * \param[in] upstream Pointer to the upstream handler
> > > + *
> > > + * When a upstream gets set, the cache is copies to that list and all further
> > 
> > s/copies/copied/
> > 
> > > + * calls to DebugMetadata::set are forwarded to that list.
> > > + *
> > > + * The upstream can be reset by passing a nullptr.
> > 
> > I think "parent" is a better name than "upstream", the parent concept is
> > widely used. I'd call the function setParent() as we use "set" rather
> > than "assign".
> 
> As discussed, I will change that.
> 
> > > + */
> > > +void DebugMetadata::assignUpstream(DebugMetadata *upstream)
> > > +{
> > > +	upstream_ = upstream;
> > > +
> > > +	if (!upstream_)
> > > +		return;
> > > +
> > > +	for (const auto &ctrl : cache_)
> > > +		upstream_->set(ctrl.first, ctrl.second);
> > > +
> > > +	cache_.clear();
> > > +}
> > > +
> > > +/**
> > > + * \fn DebugMetadata::assignControlList
> > > + * \brief Assign a control list
> > > + * \param[in] list Pointer to the list
> > > + *
> > > + * When a list gets set, the cache is copies to that list and all further calls
> > 
> > s/copies/copied/
> > 
> > > + * to DebugMetadata::set are forwarded to that list.
> > > + *
> > > + * The upstream can be reset by passing a nullptr.
> > 
> > s/upstream/list/
> > 
> > > + */
> > > +void DebugMetadata::assignControlList(ControlList *list)
> > > +{
> > > +	list_ = list;
> > > +
> > > +	if (list_) {
> > > +		list_->merge(cache_);
> > > +		cache_.clear();
> > > +	}
> > > +}
> > > +
> > > +/**
> > > + * \fn DebugMetadata::set(const Control<T> &ctrl, const V &value)
> > > + * \brief Set a value
> > > + * \param[in] ctrl The ctrl to set
> > > + * \param[in] value The control value
> > > + *
> > > + * Sets the debug metadata for ctrl to value \a value. If an upstream or a
> > > + * control list is set, the value gets passed there (upstream takes precedence).
> > > + * Otherwise the value is cached until one of them gets assigned.
> > > + *
> > > + * When the instance is disabled and upstream is not set, the value gets dropped.
> > > + */
> > > +
> > > +/**
> > > + * \fn DebugMetadata::set(unsigned int id, const ControlValue &value)
> > > + * \brief Set a value
> > > + * \param[in] id The id of the control
> > > + * \param[in] value The control value
> > > + *
> > > + * Sets the debug metadata to value \a value. If an upstream or a control list
> > > + * is set, the value gets passed there (upstream takes precedence).
> > > + * Otherwise the value is cached until one of them gets assigned.
> > > + *
> > > + * When the instance is disabled and upstream is not set, the value gets dropped.
> > > + */
> > > +void DebugMetadata::set(unsigned int id, const ControlValue &value)
> > > +{
> > > +	if (upstream_) {
> > > +		upstream_->set(id, value);
> > > +		return;
> > > +	}
> > > +
> > > +	if (!enabled_)
> > > +		return;
> > > +
> > > +	if (list_) {
> > > +		list_->set(id, value);
> > > +		return;
> > > +	}
> > > +
> > > +	cache_.set(id, value);
> > > +}
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > > index aa9ab0291854..f7b5ee8dcc34 100644
> > > --- a/src/libcamera/meson.build
> > > +++ b/src/libcamera/meson.build
> > > @@ -25,6 +25,7 @@ libcamera_internal_sources = files([
> > >      'control_validator.cpp',
> > >      'converter.cpp',
> > >      'delayed_controls.cpp',
> > > +    'debug_controls.cpp',
> > >      'device_enumerator.cpp',
> > >      'device_enumerator_sysfs.cpp',
> > >      'dma_buf_allocator.cpp',