From 66004ec44388555fe3f71ac87d4a90249fdce881 Mon Sep 17 00:00:00 2001 From: Matti Jukola Date: Fri, 1 Apr 2022 21:43:53 +0200 Subject: [PATCH 1/4] 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. --- modules/aruco/src/apriltag_quad_thresh.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/aruco/src/apriltag_quad_thresh.cpp b/modules/aruco/src/apriltag_quad_thresh.cpp index 296309267c1..20c193725fa 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 From 43ff44b34674290cc36f7850598f2ef4606f0a98 Mon Sep 17 00:00:00 2001 From: Matti Jukola Date: Tue, 5 Apr 2022 21:16:32 +0200 Subject: [PATCH 2/4] 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. --- modules/aruco/test/test_arucodetection.cpp | 89 ++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/modules/aruco/test/test_arucodetection.cpp b/modules/aruco/test/test_arucodetection.cpp index b2a82f4a184..85c6412eb6b 100644 --- a/modules/aruco/test/test_arucodetection.cpp +++ b/modules/aruco/test/test_arucodetection.cpp @@ -717,4 +717,93 @@ TEST(CV_ArucoDetectMarkers, regression_2492) } } +TEST(CV_ArucoDetectMarkers, number_of_threads_does_not_change_results) +{ + 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_; + }; + cv::Ptr 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 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); + img.setTo(255); + img_marker.copyTo(img(cv::Rect(shift, shift, height_marker, height_marker))); + + // Number of threads to be tested. OpenCV uses C++03 so no list-initialization + std::vector num_threads_to_test; + num_threads_to_test.push_back(1); + num_threads_to_test.push_back(2); + num_threads_to_test.push_back(8); + num_threads_to_test.push_back(16); + num_threads_to_test.push_back(32); + num_threads_to_test.push_back(height_img-1); + num_threads_to_test.push_back(height_img); + num_threads_to_test.push_back(height_img+1); + + std::vector methods_to_test; + methods_to_test.push_back(cv::aruco::CORNER_REFINE_NONE); + methods_to_test.push_back(cv::aruco::CORNER_REFINE_SUBPIX); + methods_to_test.push_back(cv::aruco::CORNER_REFINE_CONTOUR); + methods_to_test.push_back(cv::aruco::CORNER_REFINE_APRILTAG); + + for (size_t i_method = 0; i_method < methods_to_test.size(); ++i_method) { + params->cornerRefinementMethod = methods_to_test[i_method]; + + std::vector> original_corners; + std::vector original_ids; + for (size_t i_num_threads = 0; i_num_threads < num_threads_to_test.size(); ++i_num_threads) { + NumThreadsSetter thread_num_setter(num_threads_to_test[i_num_threads]); + + std::vector> corners; + std::vector ids; + cv::aruco::detectMarkers(img, dictionary, corners, ids, params); + + // If we don't find any markers, the test is broken + ASSERT_GE(ids.size(), 1); + + if (original_corners.empty()) { + original_corners = corners; + original_ids = ids; + } else { + // 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); + } + } + } + } + } +} + }} // namespace From a752d6780f96b75604be806480d81c4689ca241c Mon Sep 17 00:00:00 2001 From: Alexander Smorkalov Date: Mon, 11 Apr 2022 11:09:32 +0300 Subject: [PATCH 3/4] Test refactoring. --- modules/aruco/test/test_arucodetection.cpp | 98 +++++++++++----------- 1 file changed, 48 insertions(+), 50 deletions(-) diff --git a/modules/aruco/test/test_arucodetection.cpp b/modules/aruco/test/test_arucodetection.cpp index 85c6412eb6b..e29ac4ab542 100644 --- a/modules/aruco/test/test_arucodetection.cpp +++ b/modules/aruco/test/test_arucodetection.cpp @@ -717,7 +717,7 @@ TEST(CV_ArucoDetectMarkers, regression_2492) } } -TEST(CV_ArucoDetectMarkers, number_of_threads_does_not_change_results) +struct ArucoThreading: public testing::TestWithParam { struct NumThreadsSetter { NumThreadsSetter(const int num_threads) @@ -731,6 +731,10 @@ TEST(CV_ArucoDetectMarkers, number_of_threads_does_not_change_results) private: int original_num_threads_; }; +}; + +TEST_P(ArucoThreading, number_of_threads_does_not_change_results) +{ cv::Ptr params = cv::aruco::DetectorParameters::create(); // We are not testing against different dictionaries // As we are interested mostly in small images, smaller @@ -750,60 +754,54 @@ TEST(CV_ArucoDetectMarkers, number_of_threads_does_not_change_results) 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); - img.setTo(255); + cv::Mat img(height_img, height_img, CV_8UC1, cv::Scalar(255)); img_marker.copyTo(img(cv::Rect(shift, shift, height_marker, height_marker))); - // Number of threads to be tested. OpenCV uses C++03 so no list-initialization - std::vector num_threads_to_test; - num_threads_to_test.push_back(1); - num_threads_to_test.push_back(2); - num_threads_to_test.push_back(8); - num_threads_to_test.push_back(16); - num_threads_to_test.push_back(32); - num_threads_to_test.push_back(height_img-1); - num_threads_to_test.push_back(height_img); - num_threads_to_test.push_back(height_img+1); - - std::vector methods_to_test; - methods_to_test.push_back(cv::aruco::CORNER_REFINE_NONE); - methods_to_test.push_back(cv::aruco::CORNER_REFINE_SUBPIX); - methods_to_test.push_back(cv::aruco::CORNER_REFINE_CONTOUR); - methods_to_test.push_back(cv::aruco::CORNER_REFINE_APRILTAG); - - for (size_t i_method = 0; i_method < methods_to_test.size(); ++i_method) { - params->cornerRefinementMethod = methods_to_test[i_method]; - - std::vector> original_corners; - std::vector original_ids; - for (size_t i_num_threads = 0; i_num_threads < num_threads_to_test.size(); ++i_num_threads) { - NumThreadsSetter thread_num_setter(num_threads_to_test[i_num_threads]); - - std::vector> corners; - std::vector ids; - cv::aruco::detectMarkers(img, dictionary, corners, ids, params); - - // If we don't find any markers, the test is broken - ASSERT_GE(ids.size(), 1); - - if (original_corners.empty()) { - original_corners = corners; - original_ids = ids; - } else { - // 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); - } - } + params->cornerRefinementMethod = GetParam(); + + std::vector> original_corners; + std::vector 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> corners; + std::vector 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 From 1dfdff7c245db4d82ff264ac85395983095cb0fa Mon Sep 17 00:00:00 2001 From: Alexander Smorkalov Date: Thu, 14 Apr 2022 09:51:20 +0300 Subject: [PATCH 4/4] Syntax fix for pre-C++11 compilers. --- modules/aruco/test/test_arucodetection.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/aruco/test/test_arucodetection.cpp b/modules/aruco/test/test_arucodetection.cpp index e29ac4ab542..f1e77f7aaef 100644 --- a/modules/aruco/test/test_arucodetection.cpp +++ b/modules/aruco/test/test_arucodetection.cpp @@ -759,7 +759,7 @@ TEST_P(ArucoThreading, number_of_threads_does_not_change_results) params->cornerRefinementMethod = GetParam(); - std::vector> original_corners; + std::vector > original_corners; std::vector original_ids; { NumThreadsSetter thread_num_setter(1); @@ -774,7 +774,7 @@ TEST_P(ArucoThreading, number_of_threads_does_not_change_results) 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> corners; + std::vector > corners; std::vector ids; cv::aruco::detectMarkers(img, dictionary, corners, ids, params);