[30/36] libcamera: value_node: Support looking up nested children in one operation

25750 diff mbox series

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/value_node.h |  1 +
 src/libcamera/value_node.cpp            | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

2026. 01. 13. 1:08 keltezéssel, Laurent Pinchart írta:
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>   include/libcamera/internal/value_node.h |  1 +
>   src/libcamera/value_node.cpp            | 23 +++++++++++++++++++++++
>   2 files changed, 24 insertions(+)
> 
> diff --git a/include/libcamera/internal/value_node.h b/include/libcamera/internal/value_node.h
> index abb0991fd87e..da72651d2161 100644
> --- a/include/libcamera/internal/value_node.h
> +++ b/include/libcamera/internal/value_node.h
> @@ -235,6 +235,7 @@ public:
>   	bool contains(std::string_view key) const;
>   	ValueNode *at(std::string_view key);
>   	const ValueNode &operator[](std::string_view key) const;
> +	const ValueNode &operator[](std::initializer_list<std::string_view> path) const;
>   
>   	ValueNode *add(std::unique_ptr<ValueNode> child);
>   	ValueNode *add(std::string key, std::unique_ptr<ValueNode> child);
> diff --git a/src/libcamera/value_node.cpp b/src/libcamera/value_node.cpp
> index 1256adf8361d..55d5a1a29991 100644
> --- a/src/libcamera/value_node.cpp
> +++ b/src/libcamera/value_node.cpp
> @@ -487,6 +487,29 @@ const ValueNode &ValueNode::operator[](std::string_view key) const
>   	return *iter->second;
>   }
>   
> +/**
> + * \brief Retrieve a descendant node by path
> + * \param[in] path The path
> + *
> + * This function retrieves a descendant of a ValueNode by following a \a path.
> + * The path is a list of keys that index nested dictionary nodes. If any node
> + * along the path is not a Dictionary node, an empty node is returned.
> + *
> + * \return The ValueNode corresponding to the \a path
> + */
> +const ValueNode &ValueNode::operator[](std::initializer_list<std::string_view> path) const
> +{
> +	const ValueNode *node = this;
> +
> +	for (const auto part : path) {

   const auto &part


> +		node = &(*node)[part];
> +		if (!*node)

I feel like it would be nicer to have a const qualified `at()` as checking
for a nullptr is simpler that reading out the type and comparing it with `Type::Empty`.
(At it would avoid the need for `&*`.) But in any case, it looks ok.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


> +			return empty;
> +	}
> +
> +	return *node;
> +}
> +
>   /**
>    * \brief Add a child node to a list
>    * \param[in] child The child node

On Tue, Jan 13, 2026 at 01:49:39PM +0100, Barnabás Pőcze wrote:
> 2026. 01. 13. 1:08 keltezéssel, Laurent Pinchart írta:
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >   include/libcamera/internal/value_node.h |  1 +
> >   src/libcamera/value_node.cpp            | 23 +++++++++++++++++++++++
> >   2 files changed, 24 insertions(+)
> > 
> > diff --git a/include/libcamera/internal/value_node.h b/include/libcamera/internal/value_node.h
> > index abb0991fd87e..da72651d2161 100644
> > --- a/include/libcamera/internal/value_node.h
> > +++ b/include/libcamera/internal/value_node.h
> > @@ -235,6 +235,7 @@ public:
> >   	bool contains(std::string_view key) const;
> >   	ValueNode *at(std::string_view key);
> >   	const ValueNode &operator[](std::string_view key) const;
> > +	const ValueNode &operator[](std::initializer_list<std::string_view> path) const;
> >   
> >   	ValueNode *add(std::unique_ptr<ValueNode> child);
> >   	ValueNode *add(std::string key, std::unique_ptr<ValueNode> child);
> > diff --git a/src/libcamera/value_node.cpp b/src/libcamera/value_node.cpp
> > index 1256adf8361d..55d5a1a29991 100644
> > --- a/src/libcamera/value_node.cpp
> > +++ b/src/libcamera/value_node.cpp
> > @@ -487,6 +487,29 @@ const ValueNode &ValueNode::operator[](std::string_view key) const
> >   	return *iter->second;
> >   }
> >   
> > +/**
> > + * \brief Retrieve a descendant node by path
> > + * \param[in] path The path
> > + *
> > + * This function retrieves a descendant of a ValueNode by following a \a path.
> > + * The path is a list of keys that index nested dictionary nodes. If any node
> > + * along the path is not a Dictionary node, an empty node is returned.
> > + *
> > + * \return The ValueNode corresponding to the \a path
> > + */
> > +const ValueNode &ValueNode::operator[](std::initializer_list<std::string_view> path) const
> > +{
> > +	const ValueNode *node = this;
> > +
> > +	for (const auto part : path) {
> 
>    const auto &part
> 
> > +		node = &(*node)[part];
> > +		if (!*node)
> 
> I feel like it would be nicer to have a const qualified `at()` as checking
> for a nullptr is simpler that reading out the type and comparing it with `Type::Empty`.
> (At it would avoid the need for `&*`.) But in any case, it looks ok.

Yes, I'm not too happy with operator[]. It is kind of OK for this
series, but once we'll build something on top it may become clear that a
nicer ValueNode API is needed. In particular, I'm thinking of allowing a
node to contain a value *and* child elements at the same time, as well
as merging list and dictionaries together into a single type. Nothing
would change for YAML parsing as a YAML document wouldn't make use of
those features, but it would simplify error handling.

I think we should decide on this when extending CameraManager::start()
with a parameter to specify configuration parameters (and the first
question we'll have to ask then will be if ValueNode should be exposed
to applications, or if we should pass the data in a different way)

> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> > +			return empty;
> > +	}
> > +
> > +	return *node;
> > +}
> > +
> >   /**
> >    * \brief Add a child node to a list
> >    * \param[in] child The child node
>