[libcamera-devel] [PATCH v5 01/17] libcamera: controls: Add a function to merge two control lists

[Kieran Bingham]

Hi Jacopo,

On 03/05/2021 11:41, Jacopo Mondi wrote:
> From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> Add a new ControlList::merge() function to merge two control lists by
> copying in the values in the list passed as parameters.
> 
> 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 at ideasonboard.com>
> [reimplement the function by not using std::unordered_map::merge()]
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

With the cost/const fixed

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  include/libcamera/controls.h |  2 ++
>  src/libcamera/controls.cpp   | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 1a5690a5ccbe..1c9b37e617bc 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(const 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..d8f104e3734b 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -874,6 +874,36 @@ 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 copies elements from the \a source and inserts
> + * them in *this. If the \a source contains elements whose key is already
> + * present in *this, then those elements are not overwritten.
> + *
> + * Only control lists created from the same ControlIdMap or ControlInfoMap may
> + * be merged. Attempting to do otherwise results in undefined behaviour.
> + *
> + * \todo Reimplement or implement an overloaded version which internally uses
> + * std::unordered_map::merge() and accepts a non-cost argument.
> + */
> +void ControlList::merge(const ControlList &source)
> +{
> +	ASSERT(idmap_ == source.idmap_);
> +
> +	for (const auto &ctrl : source) {
> +		if (contains(ctrl.first)) {
> +			const ControlId *id = idmap_->at(ctrl.first);
> +			LOG(Controls, Warning)
> +				<< "Control " << id->name() << " not overwritten";
> +			continue;
> +		}
> +
> +		set(ctrl.first, ctrl.second);
> +	}
> +}
> +
>  /**
>   * \brief Check if the list contains a control with the specified \a id
>   * \param[in] id The control ID
> 

-- 
Regards
--
Kieran