Fix sampling for version multiplying factor #22025

AleksandrPanov · 2022-05-23T17:35:30Z

Merge with extra: opencv/opencv_extra#976

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 another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the 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

alalek · 2022-05-28T11:39:41Z

modules/objdetect/src/qrcode.cpp

+    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)


Overcomplicated (need to rewrite) or just invalid "for" loops.

alalek · 2022-06-03T10:18:19Z

modules/objdetect/perf/perf_qrcode_pipeline.cpp

@@ -106,10 +106,11 @@ PERF_TEST_P_(Perf_Objdetect_QRCode_Multi, decodeMulti)
 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",


We could use C++ comments here: /* "version_5_right.jpg", */ instead of whitespace

alalek · 2022-06-03T10:18:51Z

modules/objdetect/src/qrcode.cpp

@@ -2369,7 +2369,8 @@ bool QRDecode::samplingForVersion()
    CV_TRACE_FUNCTION();
    const double multiplyingFactor = (version < 3)  ? 1 :
                                     (version == 3) ? 1.5 :
-                                     version * (version + 1);
+                                     (version < 6) ? version*(version+1) :
+                                     version;


version;

version * 5 ?

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.

now multiplyingFactor:

const double multiplyingFactor = (version < 3) ? 1. : (version == 3) ? 2. : 3.;

alalek · 2022-06-03T10:21:06Z

modules/objdetect/src/qrcode.cpp

+        deltas_rows[(i*version_size)/abs(skipped_rows)+(version_size/abs(skipped_cols)-1)/2]
+        += skipped_rows > 0 ? 1 : -1;


Please separate index calculation to simplify code.

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; }

alalek · 2022-06-03T10:26:52Z

modules/objdetect/src/qrcode.cpp

+        += skipped_cols > 0 ? 1 : -1;
+    }
+
+    const double totalFrequencyElem = countNonZero(postIntermediate) / static_cast<double>(postIntermediate.total());


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?

actually postIntermediate is binary (0/255) image in this case:

no_border_intermediate is binary

postIntermediate = resize(no_border_intermediate, newFactorSize, INTER_AREA)

newFactorSize is integer and InterpolationFlags is INTER_AREA, so postIntermediate is binary (0/255) image

I checked this in tests. Also totalFrequencyElem must be around 0.5, otherwise most of the tests will fail.

…ression_version_25

AleksandrPanov · 2022-06-03T16:48:09Z

@alalek, fixed the issues

AleksandrPanov force-pushed the fix_samplingForVersion_multiplyingFactor branch from 855f948 to 09e0eb7 Compare May 27, 2022 13:44

Kumataro mentioned this pull request May 28, 2022

objdetect: Fix sampling QR code image #21949

Closed

6 tasks

alalek reviewed May 28, 2022

View reviewed changes

AleksandrPanov force-pushed the fix_samplingForVersion_multiplyingFactor branch 3 times, most recently from 80e437f to 15b05c3 Compare June 1, 2022 23:47

reduce experimentalFrequencyElem and listFrequencyElem

9b02b93

AleksandrPanov force-pushed the fix_samplingForVersion_multiplyingFactor branch from 15b05c3 to 9b02b93 Compare June 2, 2022 02:11

AleksandrPanov added 2 commits June 2, 2022 05:21

fix large resize

051d34d

fix tile in postIntermediate

311a12b

AleksandrPanov added the test label Jun 2, 2022

alalek reviewed Jun 3, 2022

View reviewed changes

AleksandrPanov added 2 commits June 3, 2022 18:20

add getMinSideLen(), add corrected_index

73b38b7

add test decode_regression_21929 author Kumataro, add test decode_reg…

98a95d6

…ression_version_25

AleksandrPanov force-pushed the fix_samplingForVersion_multiplyingFactor branch from 27e28c2 to 98a95d6 Compare June 3, 2022 16:13

AleksandrPanov marked this pull request as ready for review June 3, 2022 16:44

alalek approved these changes Jun 4, 2022

View reviewed changes

alalek merged commit 53eda42 into opencv:3.4 Jun 4, 2022

alalek mentioned this pull request Jun 4, 2022

(4.x) Merge 3.4 #22067

Merged

This was referenced Jun 8, 2022

Swiss QR-bills (ISO 20022) not decoded #21929

Closed

Doesn't correctly recognize some QR #21417

Open

QRCodeDetector fails randomly (+samples) #18372

Open

Unable to get result from QRCodeDetector.detectAndDecode() when encoded QR Code data is long #17152

Closed

alalek mentioned this pull request Aug 21, 2022

(5.x) Merge 4.x #22408

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix sampling for version multiplying factor #22025

Fix sampling for version multiplying factor #22025

Uh oh!

AleksandrPanov commented May 23, 2022 •

edited by alalek

Loading

Uh oh!

alalek May 28, 2022

Uh oh!

AleksandrPanov Jun 3, 2022

Uh oh!

alalek Jun 3, 2022

Uh oh!

AleksandrPanov Jun 3, 2022

Uh oh!

alalek Jun 3, 2022

Uh oh!

AleksandrPanov Jun 3, 2022 •

edited

Loading

Uh oh!

AleksandrPanov Jun 3, 2022

Uh oh!

alalek Jun 3, 2022

Uh oh!

AleksandrPanov Jun 3, 2022

Uh oh!

AleksandrPanov Jun 3, 2022

Uh oh!

alalek Jun 3, 2022

Uh oh!

AleksandrPanov Jun 3, 2022 •

edited

Loading

Uh oh!

AleksandrPanov commented Jun 3, 2022

Uh oh!

Uh oh!

		deltas_rows[(i*version_size)/abs(skipped_rows)+(version_size/abs(skipped_cols)-1)/2]
		+= skipped_rows > 0 ? 1 : -1;

Uh oh!

Fix sampling for version multiplying factor #22025

Fix sampling for version multiplying factor #22025

Uh oh!

Conversation

AleksandrPanov commented May 23, 2022 • edited by alalek 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!

Choose a reason for hiding this comment

Uh oh!

AleksandrPanov Jun 3, 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!

AleksandrPanov Jun 3, 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!

AleksandrPanov commented Jun 3, 2022

Uh oh!

Uh oh!

AleksandrPanov commented May 23, 2022 •

edited by alalek

Loading

AleksandrPanov Jun 3, 2022 •

edited

Loading

AleksandrPanov Jun 3, 2022 •

edited

Loading