Make sure to actually copy the frame during the copy constructor. (#2)

Otherwise we get bogus data in the destination sometimes. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
gonzodepedro · Dec 20, 2021 · a9a4156 · gonzodepedro · Dec 20, 2021 · a9a4156
1 parent 34fc227
commit a9a4156
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 25 deletions.
diff --git a/image_tools/include/image_tools/cv_mat_sensor_msgs_image_type_adapter.hpp b/image_tools/include/image_tools/cv_mat_sensor_msgs_image_type_adapter.hpp
@@ -97,32 +97,33 @@ class ROSCvMatContainer
   ROSCvMatContainer() = default;
 
   IMAGE_TOOLS_PUBLIC
-  explicit ROSCvMatContainer(const ROSCvMatContainer &other) :
-    header_(other.header_), frame_(other.frame_), is_bigendian_(other.is_bigendian_)
-    {
-      if(std::holds_alternative<std::shared_ptr<sensor_msgs::msg::Image>>(other.storage_)) {
+  explicit ROSCvMatContainer(const ROSCvMatContainer & other)
+  : header_(other.header_), frame_(other.frame_.clone()), is_bigendian_(other.is_bigendian_)
+  {
+    if (std::holds_alternative<std::shared_ptr<sensor_msgs::msg::Image>>(other.storage_)) {
+      storage_ = std::get<std::shared_ptr<sensor_msgs::msg::Image>>(other.storage_);
+    } else if (std::holds_alternative<std::unique_ptr<sensor_msgs::msg::Image>>(other.storage_)) {
+      storage_ = std::make_unique<sensor_msgs::msg::Image>(
+        *std::get<std::unique_ptr<sensor_msgs::msg::Image>>(other.storage_));
+    }
+  }
+
+  IMAGE_TOOLS_PUBLIC
+  ROSCvMatContainer & operator=(const ROSCvMatContainer & other)
+  {
+    if (this != &other) {
+      header_ = other.header_;
+      frame_ = other.frame_.clone();
+      is_bigendian_ = other.is_bigendian_;
+      if (std::holds_alternative<std::shared_ptr<sensor_msgs::msg::Image>>(other.storage_)) {
         storage_ = std::get<std::shared_ptr<sensor_msgs::msg::Image>>(other.storage_);
+      } else if (std::holds_alternative<std::unique_ptr<sensor_msgs::msg::Image>>(other.storage_)) {
+        storage_ = std::make_unique<sensor_msgs::msg::Image>(
+          *std::get<std::unique_ptr<sensor_msgs::msg::Image>>(other.storage_));
       }
-      else if(std::holds_alternative<std::unique_ptr<sensor_msgs::msg::Image>>(other.storage_)) {
-        storage_ = std::make_unique<sensor_msgs::msg::Image>( *std::get<std::unique_ptr<sensor_msgs::msg::Image>>(other.storage_) );
-      }
-
-    };
-
- ROSCvMatContainer& operator=(const ROSCvMatContainer& other){
-   if (this != &other) {
-     header_ = other.header_;
-     frame_ = other.frame_;
-     is_bigendian_ = other.is_bigendian_;
-     if(std::holds_alternative<std::shared_ptr<sensor_msgs::msg::Image>>(other.storage_)) {
-       storage_ = std::get<std::shared_ptr<sensor_msgs::msg::Image>>(other.storage_);
-     }
-     else if(std::holds_alternative<std::unique_ptr<sensor_msgs::msg::Image>>(other.storage_)) {
-       storage_ = std::make_unique<sensor_msgs::msg::Image>( *std::get<std::unique_ptr<sensor_msgs::msg::Image>>(other.storage_) );
-     }
-   }
-   return *this;
- }
+    }
+    return *this;
+  }
 
   /// Store an owning pointer to a sensor_msg::msg::Image, and create a cv::Mat that references it.
   IMAGE_TOOLS_PUBLIC

diff --git a/image_tools/src/cv_mat_sensor_msgs_image_type_adapter.cpp b/image_tools/src/cv_mat_sensor_msgs_image_type_adapter.cpp
@@ -73,7 +73,7 @@ ROSCvMatContainer::ROSCvMatContainer(
 : header_(NotNull(
       unique_sensor_msgs_image.get(),
       "unique_sensor_msgs_image cannot be nullptr"
-    ).pointer->header),
+).pointer->header),
   frame_(
     unique_sensor_msgs_image->height,
     unique_sensor_msgs_image->width,