Feature/aruco speedup (rebased with 4.x) #3151

AleksandrPanov · 2022-01-19T13:25:56Z

Added improvements to the ArUco module made by the author @urbste. New PR was created to fix issues and rebase with 4.x (and to avoid regression in original PR).

Original PR: Feature/aruco speedup #2790
Original article

This PR:

adds new 3 performance tests (19 configurations)
enables old tests with distance parameter 0.4 for ArUco3 (CV_ArucoBoardPose test)
fix several "index" bugs
removes global threshold parameters - to add this parameters need to add PImpl pattern and create ArucoDetector class (like QRCodeDetector class). I suggest return global threshold parameters in next PR after create ArucoDetector class and API changes (this changes could significantly increase "git diff").

Configuration - Ryzen 7 2700x, 32GB 2933 mhz DDR4
Performance tests result (image size is 1440x1440 - the number of pixels is the same as in FHD image with size 1920x1080):

with global threshold parameters:

Algorithm	Parameters	Number of markers	Result, ms
base alg	default	1	mean=6.93 median=6.97 min=4.61 stddev=1.20 (17.3%)
arcuo3	tau_c=32, tau_i=0.15	1	mean=1.52 median=1.48 min=1.41 stddev=0.10 (6.5%)
arcuo3	tau_c=16, tau_i=0.008	1	mean=1.46 median=1.46 min=1.37 stddev=0.04 (3.0%)
base alg	default	9	mean=8.49 median=8.55 min=5.96 stddev=1.35 (15.9%)
arcuo3	tau_c=32, tau_i=0.15	9	mean=2.03 median=2.03 min=1.67 stddev=0.10 (4.7%)
arcuo3	tau_c=16, tau_i=0.008	9	mean=2.03 median=2.02 min=1.91 stddev=0.07 (3.4%)
base alg	default	100	mean=17.91 median=18.05 min=13 stddev=1.78 (10.0%)
arcuo3	tau_c=32, tau_i=0.15	100	mean=5.09 median=5.22 min=4.53 stddev=0.32 (6.3%)
arcuo3	tau_c=16, tau_i=0.008	100	mean=5.11 median=5.13 min=4.67 stddev=0.29 (5.7%)

without global threshold parameters (these parameters will be returned in the next PR):

Algorithm	Parameters	Number of markers	Result, ms
base alg	default	1	mean=6.84 median=6.69 min=4.7 stddev=0.96 (14.0%)
arcuo3	tau_c=32, tau_i=0.15	1	mean=3.03 median=3.08 min=2.51 stddev=0.20 (6.5%)
arcuo3	tau_c=16, tau_i=0.008	1	mean=2.94 median=3.03 min=2.31 stddev=0.29 (9.9%)
base alg	default	9	mean=8.39 median=8.02 min=6.08 stddev=1.44 (17.1%)
arcuo3	tau_c=32, tau_i=0.15	9	mean=3.80 median=3.80 min=2.98 stddev=0.32 (8.4%)
arcuo3	tau_c=16, tau_i=0.008	9	mean=3.65 median=3.73 min=3.09 stddev=0.28 (7.8%)
base alg	default	100	mean=18.40 median=19.29 min=14.09 stddev=1.95 (10.6%)
arcuo3	tau_c=32, tau_i=0.15	100	mean=8.23 median=8.14 min=8.04 stddev=0.22 (2.6%)
arcuo3	tau_c=16, tau_i=0.008	100	mean=8.46 median=8.20 min=8.08 stddev=0.45 (5.4%)

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
The PR is proposed to proper branch
There is reference to original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

AleksandrPanov · 2022-01-20T13:02:52Z

modules/aruco/src/aruco.cpp

    vector< int > rotated(ncandidates, 0);
    vector< uint8_t > validCandidates(ncandidates, 0);

+    const int min_perimeter = params->minSideLengthCanonicalImg * params->minSideLengthCanonicalImg;


"perimeter" is calculated by multiplying the sides of the square

this was bug, fixed

asmorkalov · 2022-01-25T06:48:41Z

modules/aruco/perf/perf_aruco.cpp

+
+TEST(EstimateAruco, ArucoThird)
+{
+    setNumThreads(1);


It's hack, is not it?

Yes, this was done for debugging.

urbste · 2022-01-25T06:53:44Z

@AleksandrPanov : great work! Thanks a lot for finishing this PR. I just did not find the time. :/

AleksandrPanov · 2022-01-27T09:44:43Z

modules/aruco/test/test_boarddetection.cpp

+    // aruco3 detection is a bit worse from large distances it seems
+    if (run_with == checkWithParameter::USE_ARUCO3) {
+        max_dist = 0.2;
+    }


In fact, a too small perimeter marker is created (marker perimeter < 32 * 4). Need set minSideLengthCanonicalImg as 16 to avoid skip this marker.

AleksandrPanov · 2022-01-28T13:34:33Z

modules/aruco/src/aruco.cpp

-    Mat image = _image.getMat();
-    CV_Assert(image.total() != 0);
-
-    /// 1. CONVERT TO GRAY
-    Mat grey;
-    _convertToGrey(image, grey);


input image was gray already

Add checks about assumptions of this code:

not .empty()

CV_CheckType()

these conditions were checked already (in detectMarkers)

added CV_DbgAssert:

CV_DbgAssert(grey.total() != 0); CV_DbgAssert(grey.type() == CV_8UC1);

AleksandrPanov · 2022-01-28T13:35:03Z

modules/aruco/src/aruco.cpp

-    Mat grey;
-    _convertToGrey(_image.getMat(), grey);


input image was gray already

AleksandrPanov · 2022-01-28T13:36:01Z

modules/aruco/src/aruco.cpp

-
    vector< vector< Point > > contours;

-    CV_Assert(_image.getMat().total() != 0);


this condition was checked already (in detectMarkers)

assumption checks are necessary, see above

added CV_DbgAssert:

CV_DbgAssert(grey.getMat().total() != 0); CV_DbgAssert(grey.getMat().type() == CV_8UC1);

fixed

AleksandrPanov · 2022-01-28T14:22:55Z

modules/aruco/src/aruco.cpp

-    CV_Assert(_corners.size() == 4);
-    CV_Assert(_image.getMat().total() != 0);
-    CV_Assert(params->markerBorderBits > 0);


these conditions were checked already (in detectMarkers)

It makes sense to keep them as CV_DbgAssert() anyway.

There are several static code analyzers which don't care about callers code but needs some checks.

added CV_DbgAssert():

CV_DbgAssert(_corners.size() == 4); CV_DbgAssert(_image.getMat().total() != 0); CV_DbgAssert(params->markerBorderBits > 0);

asmorkalov · 2022-01-31T06:36:10Z

modules/aruco/perf/perf_aruco.cpp

+    // detect markers
+    vector<vector<Point2f> > corners;
+    vector<int> ids;
+    TEST_CYCLE_N(10)


Why do you need fixed amount of iterations for performance test?

was removed, insert just TEST_CYCLE()

asmorkalov · 2022-01-31T06:41:59Z

modules/aruco/samples/detector_params.yml

 minOtsuStdDev: 5.0
 errorCorrectionRate: 0.6
+
+# new aruco functionality


# for aruco 3 should be better.

asmorkalov · 2022-01-31T06:43:51Z

modules/aruco/src/aruco.cpp

    checkRead |= readParameter(fn["maxErroneousBitsInBorderRate"], params->maxErroneousBitsInBorderRate);
    checkRead |= readParameter(fn["minOtsuStdDev"], params->minOtsuStdDev);
    checkRead |= readParameter(fn["errorCorrectionRate"], params->errorCorrectionRate);
+    // new aruco functionality


aruco 3 parameters

asmorkalov · 2022-01-31T07:04:41Z

modules/aruco/perf/perf_aruco.cpp

+        int id = ids[i];
+        const auto gold_corners = golds.find(id);
+        if (gold_corners != golds.end())
+        for (int c = 0; c < 4; c++)


Please fix indentation. Also it case of id is missing in golds the function returns 0 and test passes silently. I propose to return some very large value to highlight the issue.

inserted distMap with std::numeric_limits<double>::max() default distance. Now getMaxDistance return max value of distMap. And if ids has no gold id, then getMaxDistance will return std::numeric_limits<double>::max().

alalek · 2022-01-31T16:44:42Z

modules/aruco/src/aruco.cpp

    checkRead |= readParameter(fn["errorCorrectionRate"], params->errorCorrectionRate);
+    // new aruco 3 functionality
+    checkRead |= readParameter(fn["useAruco3Detection"], params->useAruco3Detection);
+    checkRead |= readParameter(fn["minSideLengthCanonicalImg"], params->minSideLengthCanonicalImg);
+    checkRead |= readParameter(fn["minMarkerLengthRatioOriginalImg"], params->minMarkerLengthRatioOriginalImg);
    return checkRead;


false result for old .xml files (from existed opencv versions) is an regression.

There is no false result for old .xml files, because |= operator is using.

Also, test can_find_diamondmarkers with read old .xml files was added in prev PR. This test successfully completed.

Only &= operator could add false result for old .xml files.

alalek · 2022-01-31T16:47:23Z

modules/aruco/src/aruco.cpp

-    CV_Assert(_corners.size() == 4);
-    CV_Assert(_image.getMat().total() != 0);
-    CV_Assert(params->markerBorderBits > 0);


It makes sense to keep them as CV_DbgAssert() anyway.

There are several static code analyzers which don't care about callers code but needs some checks.

alalek · 2022-01-31T16:49:35Z

modules/aruco/src/aruco.cpp

+    size_t optLevel = 0;
+    float dist = std::numeric_limits<float>::max();
+    for (size_t i = 0; i < img_pyr.size(); ++i) {
+        const float scale = img_pyr[i].cols / static_cast<float>(scaled_width);


scaled_width > 0 check is necessary.

scaled_width checked after resize image.
added CV_Assert(scaled_width > 0);

alalek · 2022-01-31T16:51:14Z

modules/aruco/src/aruco.cpp

-    Mat image = _image.getMat();
-    CV_Assert(image.total() != 0);
-
-    /// 1. CONVERT TO GRAY
-    Mat grey;
-    _convertToGrey(image, grey);


Add checks about assumptions of this code:

not .empty()

CV_CheckType()

AleksandrPanov · 2022-02-01T08:20:50Z

@alalek thx for review, I fixed your remarks

alalek · 2022-02-01T10:03:01Z

modules/aruco/test/test_boarddetection.cpp



-void CV_ArucoBoardPose::run(int) {
+void CV_ArucoBoardPose::run(int run_with) {


Wrong usage. Don't change semantic of parameters.

Check the base class:

// the main procedure of the test virtual void run( int start_from );

Use derived class or template.

removed run_with and add ArucoAlgParams to test classes constructor. Also field Ptr<aruco::DetectorParameters> params was added to test classes.

AleksandrPanov · 2022-02-01T16:23:22Z

@alalek thx, issues were fixed. Is there anything else to fix?

alalek · 2022-02-01T16:45:06Z

modules/aruco/test/test_arucodetection.cpp


+TEST(CV_Aruco3DetectionPerspective, algorithmic) {
+    CV_Aruco3DetectionPerspective test;
+    test.safe_run(CV_ArucoDetectionPerspective::USE_ARUCO3);


The issue with parameter misusing with this test too.

https://github.com/opencv/opencv/blob/4.5.5/modules/ts/include/opencv2/ts.hpp#L407-L411

@alalek, fixed:
parameter tryWith was removed, parameter ArucoAlgParams was added as field of class CV_ArucoDetectionPerspective

AleksandrPanov · 2022-02-02T16:11:05Z

@alalek thx a lot) issue with CV_Aruco3DetectionPerspective test class was fixed, is there anything else to fix?

zamlz · 2023-02-28T02:27:30Z

@AleksandrPanov I have some questions regarding the changes in this PR.

I'm currently using the Aruco detection algorithm in an older version of OpenCV. I've heard from other sources that the opencv implementation was outdated because of the GPL licensing issues #2242. But @asmorkalov recently commented that a new independent implementation was brought into OpenCV with this PR.

From my limited experience here, it seems like the primary improvement here was improving the processing speed (I also briefly skimmed the attached paper), but speed is not a huge concern for my use case at the moment. I'm curious about the following things however.

Is there a noticeable improvement to accuracy from these changes as well?
Could you elaborate on what "fix several 'index' bugs" means here? Would these bugs potentially effect accuracy depending on how I'm using Aruco?

initial commit of new aruco functionality

b6bab62

AleksandrPanov added feature category: aruco test labels Jan 19, 2022

AleksandrPanov commented Jan 20, 2022

View reviewed changes

asmorkalov reviewed Jan 25, 2022

View reviewed changes

AleksandrPanov force-pushed the rebase_aruco_speedup branch from 90becbc to 42018ae Compare January 26, 2022 15:08

AleksandrPanov commented Jan 27, 2022

View reviewed changes

AleksandrPanov force-pushed the rebase_aruco_speedup branch 2 times, most recently from 5291225 to 3b3e47c Compare January 28, 2022 08:28

add performance FHD tile tests and fix pyramid bugs

a0e036a

AleksandrPanov force-pushed the rebase_aruco_speedup branch 3 times, most recently from 51bc0d3 to 3ac9bf6 Compare January 28, 2022 13:30

AleksandrPanov commented Jan 28, 2022

View reviewed changes

AleksandrPanov force-pushed the rebase_aruco_speedup branch 3 times, most recently from ed21325 to ab550cd Compare January 28, 2022 13:54

AleksandrPanov commented Jan 28, 2022

View reviewed changes

AleksandrPanov marked this pull request as ready for review January 28, 2022 15:18

AleksandrPanov requested a review from alalek January 28, 2022 15:32

asmorkalov requested changes Jan 31, 2022

View reviewed changes

AleksandrPanov force-pushed the rebase_aruco_speedup branch from ab550cd to 051b1f4 Compare January 31, 2022 10:43

asmorkalov approved these changes Jan 31, 2022

View reviewed changes

AleksandrPanov force-pushed the rebase_aruco_speedup branch from 051b1f4 to 7e7e645 Compare January 31, 2022 14:35

alalek reviewed Jan 31, 2022

View reviewed changes

AleksandrPanov mentioned this pull request Jan 31, 2022

cv::aruco::detectMarkers crashes on AMD processors. #2736

Closed

AleksandrPanov force-pushed the rebase_aruco_speedup branch from 7e7e645 to a0e036a Compare January 31, 2022 18:49

remove global ArUco3 params (threshold)

318edea

alalek reviewed Feb 1, 2022

View reviewed changes

AleksandrPanov force-pushed the rebase_aruco_speedup branch 2 times, most recently from 289a3bc to ad9c69c Compare February 1, 2022 14:19

alalek reviewed Feb 1, 2022

View reviewed changes

add ArUco params to test classes

8e22f6d

AleksandrPanov force-pushed the rebase_aruco_speedup branch from ad9c69c to 8e22f6d Compare February 2, 2022 08:18

alalek assigned asmorkalov Feb 3, 2022

alalek merged commit 51078a5 into opencv:4.x Feb 3, 2022

asmorkalov mentioned this pull request Feb 16, 2022

ArUco library possible license incompatibility - ArUco is now GPLv3 #2242

Closed

alalek mentioned this pull request Feb 22, 2022

(5.x) Merge 4.x #3179

Merged


		vector< vector< Point > > contours;

		CV_Assert(_image.getMat().total() != 0);



		void CV_ArucoBoardPose::run(int) {
		void CV_ArucoBoardPose::run(int run_with) {

Feature/aruco speedup (rebased with 4.x) #3151

Feature/aruco speedup (rebased with 4.x) #3151

Uh oh!

Conversation

AleksandrPanov commented Jan 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

urbste commented Jan 25, 2022

Uh oh!

AleksandrPanov Jan 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AleksandrPanov Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AleksandrPanov Feb 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AleksandrPanov Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AleksandrPanov Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AleksandrPanov commented Jan 19, 2022 •

edited

Loading

AleksandrPanov Jan 27, 2022 •

edited

Loading

AleksandrPanov Jan 31, 2022 •

edited

Loading

AleksandrPanov Feb 1, 2022 •

edited

Loading

AleksandrPanov Jan 31, 2022 •

edited

Loading

AleksandrPanov Jan 31, 2022 •

edited

Loading

AleksandrPanov Feb 1, 2022 •

edited

Loading

AleksandrPanov Feb 1, 2022 •

edited

Loading

AleksandrPanov Feb 2, 2022 •

edited

Loading