[libcamera-devel,PATCH/WIP] libcamera: controls: Add a function to merge two control lists

11364 diff mbox series

Add a new ControlList::merge() function to merge two control lists. The
semantics is identical to std::unordered_map::merge(). This can be used
by pipeline handlers to merge metadata they populate with metadata
received from an IPA.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/controls.h |  2 ++
 src/libcamera/controls.cpp   | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

Hello,

This may come handy for the IPU3 IPA. The patch hasn't been tested yet,
I'm in particular not entirely sure of the idmap_ check. I've opted for
an ASSERT() to be restrictive and catch potential issues, we may decide
to relax the requirement (or handle it differently) if the assertion is
triggered and the analysis of the problem shows that a different
approach is required.

Hi Laurent,

Thanks for your work.

On 2021-02-22 15:21:21 +0200, Laurent Pinchart wrote:
> Add a new ControlList::merge() function to merge two control lists. The
> semantics is identical to std::unordered_map::merge(). This can be used
> by pipeline handlers to merge metadata they populate with metadata
> received from an IPA.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/controls.h |  2 ++
>  src/libcamera/controls.cpp   | 20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> Hello,
> 
> This may come handy for the IPU3 IPA. The patch hasn't been tested yet,
> I'm in particular not entirely sure of the idmap_ check. I've opted for
> an ASSERT() to be restrictive and catch potential issues, we may decide
> to relax the requirement (or handle it differently) if the assertion is
> triggered and the analysis of the problem shows that a different
> approach is required.

I think it make sens to have an ASSERT() check for this.

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 1a5690a5ccbe..16726ce9b1ed 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -363,7 +363,9 @@ public:
>  
>  	bool empty() const { return controls_.empty(); }
>  	std::size_t size() const { return controls_.size(); }
> +
>  	void clear() { controls_.clear(); }
> +	void merge(ControlList &source);
>  
>  	bool contains(const ControlId &id) const;
>  	bool contains(unsigned int id) const;
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index c58ed3946f3b..58555ea64c95 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -874,6 +874,26 @@ ControlList::ControlList(const ControlInfoMap &infoMap, ControlValidator *valida
>   * \brief Removes all controls from the list
>   */
>  
> +/**
> + * \brief Merge the \a source into the ControlList
> + * \param[in] source The ControlList to merge into this object
> + *
> + * Merging two control lists extracts elements from the \a source and insert
> + * them in *this. If the \a source contains elements whose key is already
> + * present in *this, then those elements are not extracted. The semantics is
> + * identical to std::unordered_map::merge() in that only internal pointers to
> + * nodes are updated, without copying or moving the elements.
> + *
> + * Only control lists created from the same ControlIdMap or ControlInfoMap may
> + * be merged. Attempting to do otherwise results in undefined behaviour.
> + */
> +void ControlList::merge(ControlList &source)
> +{
> +	ASSERT(idmap_ == source.idmap_);
> +
> +	controls_.merge(source.controls_);
> +}
> +
>  /**
>   * \brief Check if the list contains a control with the specified \a id
>   * \param[in] id The control ID
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel