WeChat QRCode module open source. #2821

dddzg · 2021-01-15T08:29:14Z

Merge with 3rdparty: opencv/opencv_3rdparty#59

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 the proper branch
The feature is well documented and sample code can be built with the project CMake

There is an open-source version of the WeChat QR code detector.

Check #2809 for more information.

force_builders=ARMv7,Custom
build_image:Custom=centos:7
buildworker:Custom=linux-1

build_image:Docs=docs-js

allow_multiple_commits=1

modules/wechat_qrcode/CMakeLists.txt

dddzg · 2021-01-16T14:47:14Z

Hi @alalek, sorry for bothering you again. Now, the code successfully supports different platforms. However, there are two issues that we meet:

As we can see, the patch size exceeds 1MB, could we ignore this warning?
during debugging on iOS, we find that we can't name the API class as QRCodeDetector because there is a QRCodeDetector in OpenCV objdetect module. Even though they are under different modules (one is under objdetect and the other is under wechat_qrcode, check logs below), there is still an error. Maybe it is a feature, not a bug for iOS. So we rename the QRCodeDetector class as WeChatQRCode temporarily. What are your ideas?

logs:
error: Multiple commands produce '/Volumes/build-storage/build/precommit-contrib_ios/build/build_ios_contrib/build/build-armv7-iphoneos/lib/Release/opencv2.framework/Headers/QRCodeDetector.h':

Target 'opencv2' (project 'opencv2') has copy command from '/Volumes/build-storage/build/precommit-contrib_ios/build/build_ios_contrib/build/build-armv7-iphoneos/modules/objc_bindings_generator/ios/gen/objc/objdetect/QRCodeDetector.h' to '/Volumes/build-storage/build/precommit-contrib_ios/build/build_ios_contrib/build/build-armv7-iphoneos/lib/Release/opencv2.framework/Headers/QRCodeDetector.h' `
Target 'opencv2' (project 'opencv2') has copy command from '/Volumes/build-storage/build/precommit-contrib_ios/build/build_ios_contrib/build/build-armv7-iphoneos/modules/objc_bindings_generator/ios/gen/objc/wechat_qrcode/QRCodeDetector.h' to '/Volumes/build-storage/build/precommit-contrib_ios/build/build_ios_contrib/build/build-armv7-iphoneos/lib/Release/opencv2.framework/Headers/QRCodeDetector.h'

alalek · 2021-01-16T18:33:32Z

This repository should store source code only. Models and other data blobs should be stored externally.
(There is an exception for images for samples / documentation only. Images for tests are stored in opencv_extra).

There are several options:

opencv_3rdparty repository with data on branches (example for face detector DNN data). Usually used with C++ code (downloaded by CMake scripts). This data is downloaded through https://raw.githubusercontent.com (AFAIK, this may cause issues for devs from China due to GFW)
GoogleDrive, DropBox, etc (need to check if CMake is able to download data from these locations (to use in C++ code) - there are several limitations). There are no OpenCV accounts for that. Authors use their own.

Take a look ocv_download() function in CMake scripts.

There is CV_WRAP_AS(WeChatQRCode) / CV_EXPORTS_AS(WeChatQRCode) macros. Need to check if Obj-C binding generator support that.

Headers/QRCodeDetector.h

In general binding generator should handle nested namespaces (target name may look like <namespace>_<className>). Perhaps we need to improve generator first. Consider disabling objc wrapper for now.

Just renaming to WeChatQRCode should be OK too.

dddzg · 2021-01-18T11:56:26Z

Hi, @alalek . I have added ocv_download related code. There is a new pull request for opencv_3rdparty opencv/opencv_3rdparty#59. Please check it out. I can't start a pull request to the wechat_qrcode branch, so I start a pull request to the readme branch for opencv_3rdparty. Could you help me?

BTW, there are still some issues:

I have moved the models with ocv_download. However, the code is too much. It still exceeds the 1MB patch limit. Could we ignore it?
For ocv_download, I didn't find a standard requirement about DESTINATION_DIR. Now, it is set to "${CMAKE_BINARY_DIR}/downloads/wechat_qrcode". What are your ideas?
As you mentioned <namespace>_<className>, it reminds me that, for the python binding generator, it also follows <namespace>_<className> template. So, it leads to a little weird API result likes cv2.wechat_qrcode_WeChatQRCode (can we adjust it to cv2.wechat_qrcode.WeChatQRCode or cv2.WeChatQRCode). For C++, it looks well for cv::wechat_qrcode::WeChatQRCode

alalek · 2021-01-18T17:13:26Z

Thank you for update!

I can't start a pull request to the wechat_qrcode branch

Please prepare 2 commits in this PR:

base branch is root (currently it is readme which is not correct), you can change target branch in proposed PR using the "Edit" button.
1st commit with readme file (I will create wechat_qrcode branch on this commit before the merge)
2nd commit with models itself (I will create wechat_qrcode_20210115 branch during the merge)

(no need to re-create PR, use --force push option)

However, the code is too much. It still exceeds the 1MB patch limit.
dddzg wants to merge 59 commits

Need to squash commits into one (all binaries are still in git history)

I didn't find a standard requirement about DESTINATION_DIR.
Now, it is set to "${CMAKE_BINARY_DIR}/downloads/wechat_qrcode". What are your ideas?

This is the right path.

it leads to a little weird API result likes cv2.wechat_qrcode_WeChatQRCode

Agreed, we need to take a look on this.

cv2.wechat_qrcode.WeChatQRCode

Submodules are not supported yet in Python bindings. However it makes sense (but we need to preserve existed Python API too to avoid breaking of Users code).

@vpisarev Any suggestions about bindings?

vpisarev · 2021-01-19T10:53:32Z

@dddzg, @alalek, in my opinion, the patch looks great. I suggest to merge it in and adjust Python bindings/fix ObjC bindings etc. later if needed. It's usable, it works really well and that's enough for now.

@dddzg, may I ask you to do 2 more things:

take the updated C++ example from here: https://github.com/vpisarev/opencv_contrib/blob/wechat_qrcode/modules/wechat_qrcode/samples/qrcode_example.cpp. It would be nice if the Python example is updated in a similar way.
make sure all the builders are green (except for the doc builder which will report warning about the patch size). Then squash commits into one:

$ git reset --soft HEAD~number_of_commits_in_PR  # now it would be 59, but after updating qrcode_example.cpp (and optionally qrcode.py the number will increase)
$ git commit
$ git push --force origin

after it the pull request will contain just 1 commit and the doc builder warning will gone, since the patch will not include model files anymore.

dddzg · 2021-01-19T15:18:58Z

Sorry for the delay.
Hi @alalek, I have updated the opencv_3rdparty with 2 commits, opencv/opencv_3rdparty#59.
Hi @vpisarev , thanks for your suggestions, I have updated the C++ and python examples and squashed commits into one.

alalek

I have updated the opencv_3rdparty with 2 commits

Update looks good. Thank you!

modules/wechat_qrcode/CMakeLists.txt

vpisarev · 2021-01-20T10:42:23Z

@alalek, all the tests pass now; in my opinion, PR is ready to be merged 👍

vpisarev · 2021-01-20T12:10:04Z

@alalek, if you do not have any objections, I suggest @dddzg to squash commits into one once again; then it can be merged using our automated tool

alalek

Great contribution!

modules/wechat_qrcode/LICENSE

modules/wechat_qrcode/src/binarizermgr.cpp

modules/wechat_qrcode/src/detector/align.hpp

modules/wechat_qrcode/samples/qrcode.py

alalek · 2021-01-20T12:20:59Z

modules/wechat_qrcode/src/zxing/common/kmeans.cpp

+ * maxepoches max iteration epochs
+ * minchanged min central change times
+ */
+vector<Cluster> k_means(vector<vector<double> > trainX, uint k, uint maxepoches, uint minchanged) {


Future TODO: try to reuse cv::kmeans()

alalek · 2021-01-20T12:21:28Z

modules/wechat_qrcode/src/zxing/common/mathutils.hpp

+     * Ends up being a bit faster than {@link Math#round(float)}. This merely
+     * rounds its argument to the nearest int, where x.5 rounds up to x+1.
+     */
+    static inline int round(double value) {


Future TODO: cvRound()

modules/wechat_qrcode/src/wechat_qrcode.cpp

alalek · 2021-01-20T22:34:44Z

modules/wechat_qrcode/src/wechat_qrcode.cpp

+    auto res_points = vector<Mat>();
+    auto ret = p->decode(input_img, candidate_points, res_points);


Are points fixed size? 4 points per instance?

if so, then it make sense to return 2D Mat instead of vector:

rows are instances

cols(=4) are corners points of CV_32FC2 elements (C2 means 2 channels for x/y components)

Yes, 4 points per instance, but the row size of points is not fixed. Because our detector supports detecting multiple QRcode in an image. Moreover, not all detected QRCode can be decoded. So, there are candidate_points detected by the detector. Then we try to decode them one by one. For the successfully decoded qrcode, we will append their points to the res_points.

So, I prefer to use the vector, which is more convenient to append. Could we keep it?
I am not very familiar with OpenCV 2D Mat, is it convenient to append new rows?

multiple QRcode in an image

So, what is represented by each vector element? 4 points corners?

This question is about external API representation (points output argument). Internal implementation can be any (we don't worry about that).

@alalek, I suggest to merge the current PR as-is.

@vpisarev Please make careful reviews. For example, .fixedType() check below doesn't make any sense. Are you going to fix that?

@alalek Yes, each Mat in vector represents 4 points corners, it can also be represented as Mat(4, 2, CV_32FC1). And the format is the same with OpenCV QRcodeDetector

Hi @alalek , the code fixedType() comes from https://github.com/opencv/opencv/blob/7a790d0d353bcbe48f1d62fb917c787ab4185846/modules/objdetect/src/qrcode.cpp#L49 (the opencv qrcode module) Because I try to convert vector<Mat> to OutputArrayOfArrays. Do you have any good ideas?

@dddzg
Hello, are there any steps for use and deployment? I want to try to use it in Golang, but I can't run the project of wechat_qrcode. Cmake is always running unsuccessfully, and I meet all kinds of errors, which makes me very upset!

Could you list the errors with more details?

alalek · 2021-01-21T07:49:44Z

@vpisarev Do you want to merge opencv_3rdparty?

Why do you violate existed rules? ("fixup" commits should not go to upstream repository)

alalek · 2021-01-21T08:17:32Z

modules/wechat_qrcode/src/wechat_qrcode.cpp

+    // opencv type convert
+    vector<Mat> tmp_points;
+    if (points.needed()) {
+        for (size_t i = 0; i < res_points.size(); i++) {
+            Mat tmp_point;
+            tmp_points.push_back(tmp_point);
+            res_points[i].convertTo(((OutputArray)tmp_points[i]),
+                                    ((OutputArray)tmp_points[i]).fixedType()
+                                        ? ((OutputArray)tmp_points[i]).type()
+                                        : CV_32FC2);


.fixedType() is used with external arguments only (mentioned file from objdetect follows this rule).
No sense to use it with "local" variables.

good ideas

Reuse conventions from external API of objdetect/QRCodeDetector
This code is expected here.

Update: file from objdetect is valid partially

valid / 2 / 3

invalid (code returns rectified and binarized images)

OK. I will remove it.

@alalek Could we reopen this PR or open a new PR?

vpisarev · 2021-01-21T10:07:53Z

@alalek, it looks like our tool does not merge opencv_3rdparty, and I do not have permissions to merge into it manually. So, could you please do it?

@dddzg, open the new PR, this one is merged already

dddzg mentioned this pull request Jan 15, 2021

WeChat QRCode module open source. #2809

Closed

4 tasks

alalek reviewed Jan 15, 2021

View reviewed changes

modules/wechat_qrcode/CMakeLists.txt Outdated Show resolved Hide resolved

vpisarev self-requested a review January 19, 2021 10:45

vpisarev self-assigned this Jan 19, 2021

dddzg force-pushed the wechat_qrcode branch from 0d550e5 to 0f3b83c Compare January 19, 2021 15:07

alalek reviewed Jan 19, 2021

View reviewed changes

modules/wechat_qrcode/CMakeLists.txt Outdated Show resolved Hide resolved

vpisarev approved these changes Jan 20, 2021

View reviewed changes

add wechat qrcode module

98acd62

dddzg force-pushed the wechat_qrcode branch from b78925c to 98acd62 Compare January 20, 2021 12:13

alalek reviewed Jan 20, 2021

View reviewed changes

dddzg added 3 commits January 20, 2021 20:48

remove useless license

af72ddd

remove licenses in samples

06f690f

add precomp

7b872c9

alalek reviewed Jan 20, 2021

View reviewed changes

dddzg added 2 commits January 21, 2021 11:46

use CV_Assert

9750e14

remove useless cv::

be6718e

opencv-pushbot merged commit 905994d into opencv:master Jan 21, 2021

alalek reviewed Jan 21, 2021

View reviewed changes

dddzg mentioned this pull request Feb 1, 2021

Wechat qrcode module refinement #2849

Merged

6 tasks

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #2919

Merged

alalek mentioned this pull request Nov 11, 2022

Link wechat_qrcode with libiconv on MinGW #2931

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WeChat QRCode module open source. #2821

WeChat QRCode module open source. #2821

dddzg commented Jan 15, 2021 •

edited by vpisarev

Loading

dddzg commented Jan 16, 2021 •

edited

Loading

alalek commented Jan 16, 2021

dddzg commented Jan 18, 2021 •

edited

Loading

alalek commented Jan 18, 2021 •

edited

Loading

vpisarev commented Jan 19, 2021 •

edited

Loading

dddzg commented Jan 19, 2021

alalek left a comment

vpisarev commented Jan 20, 2021

vpisarev commented Jan 20, 2021

alalek left a comment

alalek Jan 20, 2021

alalek Jan 20, 2021

alalek Jan 20, 2021

dddzg Jan 21, 2021 •

edited

Loading

alalek Jan 21, 2021

vpisarev Jan 21, 2021

alalek Jan 21, 2021

dddzg Jan 21, 2021 •

edited

Loading

dddzg Jan 21, 2021 •

edited

Loading

PublicWorld Feb 2, 2021

PublicWorld Feb 2, 2021

dddzg Feb 3, 2021

alalek commented Jan 21, 2021

alalek Jan 21, 2021 •

edited

Loading

dddzg Jan 21, 2021

dddzg Jan 21, 2021 •

edited

Loading

vpisarev commented Jan 21, 2021

		auto res_points = vector<Mat>();
		auto ret = p->decode(input_img, candidate_points, res_points);

WeChat QRCode module open source. #2821

WeChat QRCode module open source. #2821

Conversation

dddzg commented Jan 15, 2021 • edited by vpisarev Loading

Pull Request Readiness Checklist

dddzg commented Jan 16, 2021 • edited Loading

alalek commented Jan 16, 2021

dddzg commented Jan 18, 2021 • edited Loading

alalek commented Jan 18, 2021 • edited Loading

vpisarev commented Jan 19, 2021 • edited Loading

dddzg commented Jan 19, 2021

alalek left a comment

Choose a reason for hiding this comment

vpisarev commented Jan 20, 2021

vpisarev commented Jan 20, 2021

alalek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dddzg Jan 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dddzg Jan 21, 2021 • edited Loading

Choose a reason for hiding this comment

dddzg Jan 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alalek commented Jan 21, 2021

alalek Jan 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dddzg Jan 21, 2021 • edited Loading

Choose a reason for hiding this comment

vpisarev commented Jan 21, 2021

dddzg commented Jan 15, 2021 •

edited by vpisarev

Loading

dddzg commented Jan 16, 2021 •

edited

Loading

dddzg commented Jan 18, 2021 •

edited

Loading

alalek commented Jan 18, 2021 •

edited

Loading

vpisarev commented Jan 19, 2021 •

edited

Loading

dddzg Jan 21, 2021 •

edited

Loading

dddzg Jan 21, 2021 •

edited

Loading

dddzg Jan 21, 2021 •

edited

Loading

alalek Jan 21, 2021 •

edited

Loading

dddzg Jan 21, 2021 •

edited

Loading