-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
Improve QR code version estimation #22695
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Improve QR code version estimation #22695
Conversation
3240ebc
to
53964a3
Compare
82a247a
to
0a8c040
Compare
0a8c040
to
8566b76
Compare
8550665
to
ea53a7b
Compare
a6d055e
to
091f3e6
Compare
2ea256c
to
79c63c6
Compare
79c63c6
to
4b41946
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
3d0a706
to
ed3810f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contribution!
Just comments, feel free to ignore.
EDIT: welp, I'm late
* findPatternsVerticesPoints() may be erroneous, so they are checked. | ||
*/ | ||
static inline std::pair<int, int> matchPatternPoints(const vector<Point> &finderPattern, | ||
const vector<Point2f> cornerPointsQR) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it should be a reference
sqrt(normL2Sqr<float>(cornerPointsQR[0] - cornerPointsQR[3]))*0.5f; | ||
const float maxRelativeDistance = .1f; | ||
|
||
if (distanceToOrig/originalQrSide > maxRelativeDistance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, looks suspicious: squared distance is divided by mean of distances
vector<vector<Point>> finderPatterns; | ||
double numModulesX = 0., numModulesY = 0.; | ||
bool flag = findPatternsVerticesPoints(finderPatterns); | ||
if (flag) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: negate the condition to reduce nesting.
double numModulesX = 0., numModulesY = 0.; | ||
bool flag = findPatternsVerticesPoints(finderPatterns); | ||
if (flag) { | ||
vector<double> pattern_distance(4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this a vector?
if (flag) { | ||
vector<double> pattern_distance(4); | ||
for (auto& pattern : finderPatterns) { | ||
auto indexes = matchPatternPoints(pattern, original_points); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: name both elements of the pair so that's easier to read.
if (roundingError < thresholdFinderPattern) | ||
useFinderPattern = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useFinderPattern = (roundingError < thresholdFinderPattern);
bool useCode = false; | ||
int versionByCode = 7; | ||
if (cvRound(versionByFinderPattern) >= 7 || versionByTransition >= 7) { | ||
vector<std::pair<double, int>> versionAndDistances; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this? Should be pair bestVersion
instead, then push_back
-> bestVersion = std::min(bestVersion, ...)
;
version_size = 21 + (version - 1) * 4; | ||
CV_LOG_INFO(NULL, "QR corners: " << original_points[0] << " " << original_points[1] << " " << original_points[2] << | ||
" " << original_points[3]); | ||
if ( !(0 < version && version <= 40) ) { return false; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: if ( version <= 0 || version > 40) { return false; }
looks far more readable
if (1.f - minSide / maxSide > patternMaxRelativeLen) | ||
return false; | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return (1.f - minSide / maxSide <= patternMaxRelativeLen);
QR codes with version 7 or greater store a special 18 bit code with the version number of the code:
https://www.thonky.com/qr-code-tutorial/format-version-information
https://www.thonky.com/qr-code-tutorial/format-version-tables
This PR reads this QR information to determine the version.
Also, to determine the version of QR codes < 7, added method with
getNumModules()
function.To check the correctness, a benchmark was launched:
Logging from benchmark:
new_version.txt
base.txt
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.