<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, May 4, 2021 at 5:11 PM Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Jacopo,<br>
<br>
On 03/05/2021 11:41, Jacopo Mondi wrote:<br>
> From: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
> <br>
> Add a new ControlList::merge() function to merge two control lists by<br>
> copying in the values in the list passed as parameters.<br>
> <br>
> This can be used by pipeline handlers to merge metadata they populate<br>
> with metadata received from an IPA.<br>
> <br>
> Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
> [reimplement the function by not using std::unordered_map::merge()]<br>
> Signed-off-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br>
<br>
With the cost/const fixed<br>
<br>
Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>><br>
<br>
> ---<br>
>  include/libcamera/controls.h |  2 ++<br>
>  src/libcamera/controls.cpp   | 30 ++++++++++++++++++++++++++++++<br>
>  2 files changed, 32 insertions(+)<br>
> <br>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h<br>
> index 1a5690a5ccbe..1c9b37e617bc 100644<br>
> --- a/include/libcamera/controls.h<br>
> +++ b/include/libcamera/controls.h<br>
> @@ -363,7 +363,9 @@ public:<br>
>  <br>
>       bool empty() const { return controls_.empty(); }<br>
>       std::size_t size() const { return controls_.size(); }<br>
> +<br>
>       void clear() { controls_.clear(); }<br>
> +     void merge(const ControlList &source);<br>
>  <br>
>       bool contains(const ControlId &id) const;<br>
>       bool contains(unsigned int id) const;<br>
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp<br>
> index c58ed3946f3b..d8f104e3734b 100644<br>
> --- a/src/libcamera/controls.cpp<br>
> +++ b/src/libcamera/controls.cpp<br>
> @@ -874,6 +874,36 @@ ControlList::ControlList(const ControlInfoMap &infoMap, ControlValidator *valida<br>
>   * \brief Removes all controls from the list<br>
>   */<br>
>  <br>
> +/**<br>
> + * \brief Merge the \a source into the ControlList<br>
> + * \param[in] source The ControlList to merge into this object<br>
> + *<br>
> + * Merging two control lists copies elements from the \a source and inserts<br>
> + * them in *this. If the \a source contains elements whose key is already<br>
> + * present in *this, then those elements are not overwritten.<br>
> + *<br>
> + * Only control lists created from the same ControlIdMap or ControlInfoMap may<br>
> + * be merged. Attempting to do otherwise results in undefined behaviour.<br>
> + *<br>
> + * \todo Reimplement or implement an overloaded version which internally uses<br>
> + * std::unordered_map::merge() and accepts a non-cost argument.<br>
> + */<br>
> +void ControlList::merge(const ControlList &source)<br>
> +{<br>
> +     ASSERT(idmap_ == source.idmap_);<br></blockquote><div><br></div><div>Comparing std::unordered_map is O(NlogN) although the following for-loop is O(M), where N is the size of idmap_ and M is the size of source.</div><div>Would you consider if this is worth doing?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +<br>
> +     for (const auto &ctrl : source) {<br>
> +             if (contains(ctrl.first)) {<br>
> +                     const ControlId *id = idmap_->at(ctrl.first);<br>
> +                     LOG(Controls, Warning)<br></blockquote><div><br></div><div>I wonder if this is worth Warning. Debug seems to be natural for me.</div><div>Could you explain the reason?<br></div><div><br></div><div>Thanks,</div><div>-Hiro</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +                             << "Control " << id->name() << " not overwritten";<br>
> +                     continue;<br>
> +             }<br>
> +<br>
> +             set(ctrl.first, ctrl.second);<br>
> +     }<br>
> +}<br>
> +<br>
>  /**<br>
>   * \brief Check if the list contains a control with the specified \a id<br>
>   * \param[in] id The control ID<br>
> <br>
<br>
-- <br>
Regards<br>
--<br>
Kieran<br>
_______________________________________________<br>
libcamera-devel mailing list<br>
<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a><br>
<a href="https://lists.libcamera.org/listinfo/libcamera-devel" rel="noreferrer" target="_blank">https://lists.libcamera.org/listinfo/libcamera-devel</a><br>
</blockquote></div></div>