Message ID | 20210419133451.263733-5-naush@raspberrypi.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Naush Thanks for implementing this! On Mon, 19 Apr 2021 at 14:35, Naushir Patuck <naush@raspberrypi.com> wrote: > > Add a new Merge method to the Metadata class. This will move all > key/value pairs between a source and destination metadata object. Once > complete, the source Metadata object will be empty. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Thanks! David > --- > src/ipa/raspberrypi/controller/metadata.hpp | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/src/ipa/raspberrypi/controller/metadata.hpp b/src/ipa/raspberrypi/controller/metadata.hpp > index 319f2320fc70..1d3e941b3e52 100644 > --- a/src/ipa/raspberrypi/controller/metadata.hpp > +++ b/src/ipa/raspberrypi/controller/metadata.hpp > @@ -75,6 +75,18 @@ public: > return *this; > } > > + void Merge(Metadata &other) > + { > + std::lock_guard<std::mutex> lock(mutex_); > + std::lock_guard<std::mutex> other_lock(other.mutex_); > + > + for (auto const &kv: other.data_) > + data_[kv.first] = std::move(kv.second); > + > + /* Render the other object as empty now! */ > + other.data_.clear(); > + } > + > template<typename T> > T *GetLocked(std::string const &tag) > { > -- > 2.25.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Naush, Thank you for the patch. On Mon, Apr 19, 2021 at 02:34:50PM +0100, Naushir Patuck wrote: > Add a new Merge method to the Metadata class. This will move all > key/value pairs between a source and destination metadata object. Once > complete, the source Metadata object will be empty. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/raspberrypi/controller/metadata.hpp | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/src/ipa/raspberrypi/controller/metadata.hpp b/src/ipa/raspberrypi/controller/metadata.hpp > index 319f2320fc70..1d3e941b3e52 100644 > --- a/src/ipa/raspberrypi/controller/metadata.hpp > +++ b/src/ipa/raspberrypi/controller/metadata.hpp > @@ -75,6 +75,18 @@ public: > return *this; > } > > + void Merge(Metadata &other) > + { > + std::lock_guard<std::mutex> lock(mutex_); > + std::lock_guard<std::mutex> other_lock(other.mutex_); > + > + for (auto const &kv: other.data_) > + data_[kv.first] = std::move(kv.second); Can't you use data_.merge(other.data_); N > + > + /* Render the other object as empty now! */ > + other.data_.clear(); > + } > + > template<typename T> > T *GetLocked(std::string const &tag) > {
Hi Laurent, Thank you for your review feedback. On Tue, 27 Apr 2021 at 08:18, Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Naush, > > Thank you for the patch. > > On Mon, Apr 19, 2021 at 02:34:50PM +0100, Naushir Patuck wrote: > > Add a new Merge method to the Metadata class. This will move all > > key/value pairs between a source and destination metadata object. Once > > complete, the source Metadata object will be empty. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > src/ipa/raspberrypi/controller/metadata.hpp | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/src/ipa/raspberrypi/controller/metadata.hpp > b/src/ipa/raspberrypi/controller/metadata.hpp > > index 319f2320fc70..1d3e941b3e52 100644 > > --- a/src/ipa/raspberrypi/controller/metadata.hpp > > +++ b/src/ipa/raspberrypi/controller/metadata.hpp > > @@ -75,6 +75,18 @@ public: > > return *this; > > } > > > > + void Merge(Metadata &other) > > + { > > + std::lock_guard<std::mutex> lock(mutex_); > > + std::lock_guard<std::mutex> other_lock(other.mutex_); > > + > > + for (auto const &kv: other.data_) > > + data_[kv.first] = std::move(kv.second); > > Can't you use > > data_.merge(other.data_); > Sadly not. std::map::merge() will not overwrite elements that are present in the destination, which is what I want. None of the insert type methods do actually. I toyed with doing something like: other.data_.merge(data_); std::swap(other.data_, data_); which would achieve what I want, but thought it might look a bit cryptic. Not sure about any performance benefits from using this construct vs the explicit loop. Perhaps I should be a bit clearer with the method name, and call it Overwrite()? Thoughts? Regards, Naush > > N > > > + > > + /* Render the other object as empty now! */ > > + other.data_.clear(); > > + } > > + > > template<typename T> > > T *GetLocked(std::string const &tag) > > { > > -- > Regards, > > Laurent Pinchart >
diff --git a/src/ipa/raspberrypi/controller/metadata.hpp b/src/ipa/raspberrypi/controller/metadata.hpp index 319f2320fc70..1d3e941b3e52 100644 --- a/src/ipa/raspberrypi/controller/metadata.hpp +++ b/src/ipa/raspberrypi/controller/metadata.hpp @@ -75,6 +75,18 @@ public: return *this; } + void Merge(Metadata &other) + { + std::lock_guard<std::mutex> lock(mutex_); + std::lock_guard<std::mutex> other_lock(other.mutex_); + + for (auto const &kv: other.data_) + data_[kv.first] = std::move(kv.second); + + /* Render the other object as empty now! */ + other.data_.clear(); + } + template<typename T> T *GetLocked(std::string const &tag) {
Add a new Merge method to the Metadata class. This will move all key/value pairs between a source and destination metadata object. Once complete, the source Metadata object will be empty. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/ipa/raspberrypi/controller/metadata.hpp | 12 ++++++++++++ 1 file changed, 12 insertions(+)