Fix sampling for version multiplying factor#22025
Conversation
855f948 to
09e0eb7
Compare
modules/objdetect/src/qrcode.cpp
Outdated
| for (int i = 0, r = 0; i < version_size; i++ && r < postIntermediate.rows, r+= delta_rows) | ||
| { | ||
| for (int c = 0; c < postIntermediate.cols; c += delta_cols) | ||
| for (int j = 0, c = 0; j < version_size; j++ && c < postIntermediate.cols, c+= delta_cols) |
There was a problem hiding this comment.
Overcomplicated (need to rewrite) or just invalid "for" loops.
80e437f to
15b05c3
Compare
15b05c3 to
9b02b93
Compare
| INSTANTIATE_TEST_CASE_P(/*nothing*/, Perf_Objdetect_QRCode, | ||
| ::testing::Values( | ||
| "version_1_down.jpg", "version_1_left.jpg", "version_1_right.jpg", "version_1_up.jpg", "version_1_top.jpg", | ||
| "version_5_down.jpg", "version_5_left.jpg", "version_5_right.jpg", "version_5_up.jpg", "version_5_top.jpg", |
There was a problem hiding this comment.
We could use C++ comments here: /* "version_5_right.jpg", */ instead of whitespace
modules/objdetect/src/qrcode.cpp
Outdated
| (version == 3) ? 1.5 : | ||
| version * (version + 1); | ||
| (version < 6) ? version*(version+1) : | ||
| version; |
There was a problem hiding this comment.
fixing the tail processing process allows to reduce multiplyingFactor to 1 and skip strange resize() step
I want to add a few more tests and skip resize() step or reduce the multiplyingFactor to 2, for example.
There was a problem hiding this comment.
now multiplyingFactor:
const double multiplyingFactor = (version < 3) ? 1. :
(version == 3) ? 2. :
3.;
modules/objdetect/src/qrcode.cpp
Outdated
| deltas_rows[(i*version_size)/abs(skipped_rows)+(version_size/abs(skipped_cols)-1)/2] | ||
| += skipped_rows > 0 ? 1 : -1; |
There was a problem hiding this comment.
Please separate index calculation to simplify code.
There was a problem hiding this comment.
for (int i = 0; i < abs(skipped_rows); i++) {
// fix deltas_rows at each skip_step
const double skip_step = static_cast<double>(version_size)/abs(skipped_rows);
const int corrected_index = static_cast<int>(i*skip_step + skip_step/2);
deltas_rows[corrected_index] += skipped_rows > 0 ? 1 : -1;
}
| += skipped_cols > 0 ? 1 : -1; | ||
| } | ||
|
|
||
| const double totalFrequencyElem = countNonZero(postIntermediate) / static_cast<double>(postIntermediate.total()); |
There was a problem hiding this comment.
Looks like postIntermediate is not a binary (0/255) image. So there is no sense to use countNonZero() here.
Perhaps we want to use norm(postIntermediate, NORM_L1 | NORM_RELATIVE); here.
Do we have 0.5 value here?
There was a problem hiding this comment.
actually postIntermediate is binary (0/255) image in this case:
no_border_intermediateis binarypostIntermediate=resize(no_border_intermediate, newFactorSize, INTER_AREA)newFactorSizeis integer and InterpolationFlags isINTER_AREA, sopostIntermediateis binary (0/255) image
I checked this in tests. Also totalFrequencyElem must be around 0.5, otherwise most of the tests will fail.
27e28c2 to
98a95d6
Compare
|
@alalek, fixed the issues |
Merge with extra: opencv/opencv_extra#976
fixes #21287
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.