From 0596c05087d7f3b40d0183b6b10db6eb9fdb2d31 Mon Sep 17 00:00:00 2001
From: Matti Jukola <buq2.open.source@gmail.com>
Date: Thu, 14 Apr 2022 13:42:21 +0200
Subject: [PATCH] Merge pull request #3220 from
 buq2:aruco-apriltag-infinite-loop-fix

Fix infinite loop on ArUco apriltag refinement

* Fix infinite loop on ArUco apriltag refinement

Software entered infinite loop when image height
was smaller than 10*cv::getNumThreads(). With high
core count machines this could happen with very
reasonable image sizes.

Fix is to ensure that chunksize is at least 1.

* Test aruco detection with different number of threads

Test ensures that different aruco detection methods do not
produce different results based on number of threads.

Test was created after observing infinite loop caused
by small image and large number of threads when using
apriltag corner refinement.

* Test refactoring.

* Syntax fix for pre-C++11 compilers.

Co-authored-by: Alexander Smorkalov <alexander.smorkalov@xperience.ai>
---
 modules/aruco/src/apriltag_quad_thresh.cpp |  2 +-
 modules/aruco/test/test_arucodetection.cpp | 87 ++++++++++++++++++++++
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/modules/aruco/src/apriltag_quad_thresh.cpp b/modules/aruco/src/apriltag_quad_thresh.cpp
index 296309267..20c193725 100644
--- a/modules/aruco/src/apriltag_quad_thresh.cpp
+++ b/modules/aruco/src/apriltag_quad_thresh.cpp
@@ -1523,7 +1523,7 @@ out = Mat::zeros(h, w, CV_8UC3);
     zarray_t *quads = _zarray_create(sizeof(struct sQuad));
 
     //int chunksize = 1 + sz / (APRILTAG_TASKS_PER_THREAD_TARGET * numberOfThreads);
-    int chunksize = h / (10 * getNumThreads());
+    int chunksize = std::max(1, h / (10 * getNumThreads()));
     int sz = _zarray_size(clusters);
 
     // TODO PARALLELIZE
diff --git a/modules/aruco/test/test_arucodetection.cpp b/modules/aruco/test/test_arucodetection.cpp
index b2a82f4a1..f1e77f7aa 100644
--- a/modules/aruco/test/test_arucodetection.cpp
+++ b/modules/aruco/test/test_arucodetection.cpp
@@ -717,4 +717,91 @@ TEST(CV_ArucoDetectMarkers, regression_2492)
     }
 }
 
+struct ArucoThreading: public testing::TestWithParam<cv::aruco::CornerRefineMethod>
+{
+    struct NumThreadsSetter {
+        NumThreadsSetter(const int num_threads)
+        : original_num_threads_(cv::getNumThreads()) {
+            cv::setNumThreads(num_threads);
+        }
+
+        ~NumThreadsSetter() {
+            cv::setNumThreads(original_num_threads_);
+        }
+     private:
+        int original_num_threads_;
+    };
+};
+
+TEST_P(ArucoThreading, number_of_threads_does_not_change_results)
+{
+    cv::Ptr<cv::aruco::DetectorParameters> params = cv::aruco::DetectorParameters::create();
+    // We are not testing against different dictionaries
+    // As we are interested mostly in small images, smaller
+    // markers is better -> 4x4
+    cv::Ptr<cv::aruco::Dictionary> dictionary = cv::aruco::getPredefinedDictionary(cv::aruco::DICT_4X4_50);
+
+    // Height of the test image can be chosen quite freely
+    // We aim to test against small images as in those the
+    // number of threads has most effect
+    const int height_img = 20;
+    // Just to get nice white boarder
+    const int shift = height_img > 10 ? 5 : 1;
+    const int height_marker = height_img-2*shift;
+
+    // Create a test image
+    cv::Mat img_marker;
+    cv::aruco::drawMarker(dictionary, 23, height_marker, img_marker, 1);
+
+    // Copy to bigger image to get a white border
+    cv::Mat img(height_img, height_img, CV_8UC1, cv::Scalar(255));
+    img_marker.copyTo(img(cv::Rect(shift, shift, height_marker, height_marker)));
+
+    params->cornerRefinementMethod = GetParam();
+
+    std::vector<std::vector<cv::Point2f> > original_corners;
+    std::vector<int> original_ids;
+    {
+        NumThreadsSetter thread_num_setter(1);
+        cv::aruco::detectMarkers(img, dictionary, original_corners, original_ids, params);
+    }
+
+    ASSERT_EQ(original_ids.size(), 1);
+    ASSERT_EQ(original_corners.size(), 1);
+
+    int num_threads_to_test[] = { 2, 8, 16, 32, height_img-1, height_img, height_img+1};
+
+    for (size_t i_num_threads = 0; i_num_threads < sizeof(num_threads_to_test)/sizeof(int); ++i_num_threads) {
+        NumThreadsSetter thread_num_setter(num_threads_to_test[i_num_threads]);
+
+        std::vector<std::vector<cv::Point2f> > corners;
+        std::vector<int> ids;
+        cv::aruco::detectMarkers(img, dictionary, corners, ids, params);
+
+        // If we don't find any markers, the test is broken
+        ASSERT_EQ(ids.size(), 1);
+
+        // Make sure we got the same result as the first time
+        ASSERT_EQ(corners.size(), original_corners.size());
+        ASSERT_EQ(ids.size(), original_ids.size());
+        ASSERT_EQ(ids.size(), corners.size());
+        for (size_t i = 0; i < corners.size(); ++i) {
+            EXPECT_EQ(ids[i], original_ids[i]);
+            for (size_t j = 0; j < corners[i].size(); ++j) {
+                EXPECT_NEAR(corners[i][j].x, original_corners[i][j].x, 0.1f);
+                EXPECT_NEAR(corners[i][j].y, original_corners[i][j].y, 0.1f);
+            }
+        }
+    }
+}
+
+INSTANTIATE_TEST_CASE_P(
+        CV_ArucoDetectMarkers, ArucoThreading,
+        ::testing::Values(
+            cv::aruco::CORNER_REFINE_NONE,
+            cv::aruco::CORNER_REFINE_SUBPIX,
+            cv::aruco::CORNER_REFINE_CONTOUR,
+            cv::aruco::CORNER_REFINE_APRILTAG
+        ));
+
 }} // namespace