Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inv marker contour detection improved #2236

Merged

Conversation

szk1509
Copy link
Contributor

@szk1509 szk1509 commented Aug 21, 2019

This pull request improves the accuracy of the white markers

Resolves #2055.

  • An accuracy improvement of the white marker corners was achieved.
  • A slight improvement for the precision of the black markers corners was achieved.
  • The achieved accuracy remains stable along different noises.

invertedMarkerDetection

This pull request modifies the contour selection for the white marker contours, by modifying the selection process of the corners and contour candidates, in _filterTooCloseCandidates.

candidatesSet: The candidates and the contours are saved in a structur, for future improvements.

@alalek
Copy link
Member

alalek commented Aug 22, 2019

Please squash patch into one commit (remove unrelated commits from the patch and remove merge commits (use rebase instead of merge if necessary)).
After that rebase patch onto 3.4 branch: https://github.com/opencv/opencv/wiki/Branches

@szk1509 szk1509 changed the base branch from master to 3.4 August 22, 2019 12:59
@szk1509 szk1509 force-pushed the invMarkerContourDetectionImproved branch from 1d57129 to f937283 Compare August 23, 2019 09:15
@alalek
Copy link
Member

alalek commented Aug 23, 2019

I rebased PR changes onto 3.4 branch (please fetch commit from your fork, do not use merges, cherry-pick new commits if any).

@szk1509
Copy link
Contributor Author

szk1509 commented Aug 23, 2019

Thank you @alalek
🤔 fetch commits, use cherry pick and do not merge (use rebase instead of merge) are instructions for future PR? or do i have to fetch, then commit and squash the commits if so?
Sorry!! i am a bit (too) confused 😭

and again, thank you for your help 😃

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!

modules/aruco/src/aruco.cpp Show resolved Hide resolved
*/
static void _filterTooCloseCandidates(const vector< vector< Point2f > > &candidatesIn,
vector< vector< Point2f > > &candidatesOut,
vector< vector< vector< Point2f > > > &candidatesSetOut,
Copy link
Member

Choose a reason for hiding this comment

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

vector< vector< vector<

There are really two many nested vectors.
Two values are expected in outer dimension.

Perhaps we should split this parameter into two:

vector< vector< Point2f > > &candidatesOut,
vector< vector< Point2f > > &candidatesInvertedOut,

if(biggerIdx > -1){

biggerCandidates.push_back(candidatesIn[biggerIdx]);
biggerContours.push_back(contoursIn[biggerIdx]);
Copy link
Member

Choose a reason for hiding this comment

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

if we observing lack of performance in this code, then we can try to utilize smart pointers:

-const vector< vector< Point2f > > &candidatesIn
+typedef vector< Point2f > CandidatePoints;
+typedef cv::Ptr<CandidatePoints> CandidatePointsPtr;
+const vector<CandidatePointsPtr> &candidatesIn

@szk1509 szk1509 force-pushed the invMarkerContourDetectionImproved branch from d7cd6d8 to 93f910e Compare August 29, 2019 13:57
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.

Looks good to me! Thank you 👍

@opencv-pushbot opencv-pushbot merged commit 93f910e into opencv:3.4 Aug 30, 2019
@alalek alalek mentioned this pull request Aug 30, 2019
@hamddan4
Copy link

@szk1509
Copy link
Contributor Author

szk1509 commented Oct 10, 2019

Hi @hamddan4,
The good news is that it works for me, so i think i know where the problem is :)
the bad news is that it comes soon on the next PR...

if you like i can give you some more details, so that you can keep working ;)

@hamddan4
Copy link

Hi! Yes, i would like to know how to add this functionality to my code. Afaik, detectInvertedMarkers option of parameters is on the source code, but it doesn't seem to work.. what I am missing here?

@szk1509
Copy link
Contributor Author

szk1509 commented Oct 11, 2019

@hamddan4 🤔 well ...
The good news is that after trying 3.4 ad 4.1, it worked for me, so it works... my bad, I was in a hurry and couldn't check it properly.
The bad news is that I can't reproduce your error 🤔 so let's see...

import cv2
from cv2 import aruco
dic = aruco.getPredefinedDictionary(aruco.DICT_APRILTAG_16h5)
par = aruco.DetectorParameters_create()
par.detectInvertedMarker = True
image = cv2.imread("error/MNSGxZT.png")
dMarkers = aruco.detectMarkers( image, dic, parameters=par )
cv2.imshow("d", aruco.drawDetectedMarkers(image, dMarkers[0]))
cv2.waitKey(0)

i don't actually see what are you missing 🤔

@hamddan4
Copy link

So if it works with you, it has to be some problem with the my installed libraries? Maybe the headers are good but is not implemented in the installed version of opencv. I've done this by pip install opencv-contrib-python. Don't know what's wrong with it though. Is there anything to test it? opencv.__version__ yields 4.1.1.

@szk1509
Copy link
Contributor Author

szk1509 commented Oct 11, 2019

@hamddan4 well, i use (and recommend you) the official git repo... it includes some tests and tutorials :)
https://docs.opencv.org/

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.

4 participants