Skip to content

Conversation

@urbste
Copy link
Contributor

@urbste urbste commented Dec 16, 2020

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

Hi,

I improved the Aruco module (I will call it Aruco3 as the authors did). It implements the improvements that were proposed in the paper: "Speeded up detection of squared fiducial markers", 2018, FJ Romera-Ramirez et al.

The implementation is not entirely finished, meaning that I would like to know from you guys where I should add tests and so on.
I uploaded simple test videos: https://drive.google.com/drive/folders/153UTdWMOwxIsn2nxdWW0itV8na4AANZH?usp=sharing
The folder contains 3 videos with 4k, 1k and VGA resolution to test the speed improvements and detection rates. However are there any particular tests for the Aruco module to compare against the original implementation?

I added the functionality to the example_aruco_detect_markers.cpp file. In addition, I added new flags to the detector_params.yml. If Aruco3 is turned off, the code should behave as the original Aruco implementation.

# new aruco functionality
useAruco3Detection: 0
minSideLengthCanonicalImg: 32 # 16, 32, 64 --> tau_c from the paper
minMarkerLengthRatioOriginalImg: 0.08 # range [0,0.2] --> tau_i from the paper
cameraMotionSpeed: 0.5 # range [0,1) --> tau_s from the paper
useGlobalThreshold: 0

The parameters are explained in the documentation and reference to equations and sections from the paper.

Some preliminary results for #detected and mean computation per image in [ms]:
Call:

./example_aruco_detect_markers -v=4k_test.mp4 -d=16 -dp=detector_params.yml -refine=1

For all datasets: tau_c = 32pixel

Dataset Aruco Aruco3 (tau_i = 0.1) Aruco3 (tau_i = 0.05) Aruco3 (tau_i = 0.02)
VGA 921 / 2.91ms 704 / 0.95ms 761 / 1.1ms 909 / 2.1ms
1080p 1186 / 14ms 995 / 2.6ms 1100 / 3.1ms 1180 / 5.3ms
4K 1013 / 63ms 931 / 7.6ms 1011 / 8.1ms 1071 / 13.3ms

For all datasets: tau_c = 16pixel

Dataset Aruco Aruco3 (tau_i = 0.1) Aruco3 (tau_i = 0.05) Aruco3 (tau_i = 0.02)
VGA 921 / 2.91ms 474 / 0.8ms 796 / 0.9ms 919 / 1.7ms
1080p 1186 / 14ms 132 / 2.9ms 1086 / 2.9 ms 1173 / 4.0 ms
4K 1013 / 63ms 118 / 7.0ms 1013 /7.4ms 1052 / 8.3ms

Now instead of fixing tau_i we can also let the algorithm decide it by using the marker with the smallest perimeter from the last frame. This only works for video streams of course. In addition, we can use global thresholding instead of adaptive thresholding, assuming that the illuminations does not change much.
Call:

./example_aruco_detect_markers -v=4k_test.mp4 -d=16 -dp=detector_params.yml -refine=1 -ar3vid

For all datasets: tau_c = 32pixel, tau_s = 0.1, useGlobalThreshold=1

Dataset Aruco Aruco3
VGA 921 / 2.91ms 758 / 1.0ms
1080p 1186 / 14ms 1174 / 2.9ms
4K 1013 / 63ms 1036 / 8.8ms

For all datasets: tau_c = 16pixel, tau_s = 0.1, useGlobalThreshold=1

Dataset Aruco Aruco3
VGA 921 / 2.91ms 905 / 0.7ms
1080p 1186 / 14ms 1156 / 2.8ms
4K 1013 / 63ms 1022 / 7.7ms

Let me know what you think.
Cheers
Steffen

@urbste
Copy link
Contributor Author

urbste commented Dec 17, 2020

Ok I can see that already some Aruco tests are failing. I will have a look into it.

@urbste
Copy link
Contributor Author

urbste commented Feb 10, 2021

I added tests and fixed some small bugs. Can someone check why the build checks are failing? When I click on details it looks like all tests run successfully.

@alalek
Copy link
Member

alalek commented Feb 10, 2021

You can see compilation warnings here (should be resolved before merge).

Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution!

Please take a look on comments below.

cameraMatrix.at< double >(1, 2) = imgSize.height / 2;

Mat distCoeffs(5, 1, CV_64FC1, Scalar::all(0));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove unnecessary changes from the patch (changes in this file are not needed)

* constant and global instead of adaptive thresholding can be applied, speeding up the binarization step.
* - foundGlobalThreshold: internal variable. It is used to cache the variable to the next detector call.
* - otsuGlobalThreshold: internal variable. It is used to cache the global otsu threshold to the next detector call.
* - foundMarkerInLastFrames: internal variable. It is used to cache if markers were found in the last frame.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal variable

Parameters structs are used in OpenCV as non-modified inputs only.
This requirement is necessary for Python and other bindings.

For such video processing case with internal state it make sense to create dedicated API with detector class which store this internal state (use PImpl pattern, see QRCodeDetector class from objdetect module)

return Vec2f(A.solve(B).val);
Matx22f A(nLine1.x, nLine1.y, nLine2.x, nLine2.y);
Vec2f B(-nLine1.z, -nLine2.z);
return Vec2f(A.solve(B).val);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid such massive space-only changes.
It is really hard to see/track the significant changes of PRs.

Consider moving these "whitespace" changes to the separate first commit or separate PR.

@asenyaev
Copy link
Contributor

asenyaev commented Apr 9, 2021

jenkins cn please retry a build

@joellyons
Copy link

What's the latest on this PR? Has it been abandoned?

@urbste
Copy link
Contributor Author

urbste commented Aug 3, 2021

I am still having it in the back of my head. However, the requested changes seem to be quite significant and I did not find the time to look at it. We were using this branch in a prototype, however, we moved away from it. So if someone likes to take over please go ahead!

@joellyons
Copy link

joellyons commented Aug 3, 2021

Thank you for the update @urbste . If you don't mind a bit more info... why did you move away from it? Were you having problems with it?

@urbste
Copy link
Contributor Author

urbste commented Aug 4, 2021

@joellyons no we did not have problems with it. We wanted to use it in WebAssembly running on a smartphone to track the pose in realtime. However our board had a lot of markers and we never reached >30fps on Android devices so the AR animations were pretty bumpy. So we kind of changed our algorithm and now only occasionally update the pose using the ARuco board. The Aruco3 algorithm however really shines if you want to track the board over time, so you can update the tau_i parameter and use the global otsu thesholding. So we decided to stick with the classic Aruco implementation.

Still with my PR it is possible to use the classic impementation. I will see if I find the time to finish this PR :/

@joellyons
Copy link

@urbste thank you so much for taking the time to explain. We are wanting to track a small board over time, so this sounds perfect. But, I agree it's hard to find extra time to work on open source projects like this.

@AleksandrPanov
Copy link
Contributor

Hello @urbste, thx for the contribution, your results look great. Don't you have time to finish your PR yet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants