Add QRCodeDetector to JavaScript Build by Ziachnix · Pull Request #19284 · opencv/opencv

Ziachnix · 2021-01-07T19:54:23Z

This PR adds javascript support for the class QRCodeDetector.
Tests for the proposed functions are derived from the equivalent python tests.
Because sample images are needed for running the tests, the build of the tests
requires now the opencv_extra repository.
(Alternatively i could copy some test images into the tutorial folder and use
them in the tests)
Due to emscripten limitations the functions decodeMulti and
detectAndDecodeMulti are excluded.
As far as i know it is not possible to use a function parameter (string array)
to return the decoded content of the QR codes.

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 proper branch
There is reference to 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

force_builders=Custom
build_image:Docs=docs-js
buildworker:Docs=linux-4,linux-6
build_image:Custom=javascript
buildworker:Custom=linux-4

alalek

Thank you for contribution!

Please take a look on comments below.

alalek · 2021-01-21T22:41:43Z

doc/js_tutorials/js_setup/js_setup/js_setup.markdown

    @endcode

+    @note
+    It requires the [opencv_extra repository](https://github.com/opencv/opencv.git) located in your development environment.


Lets keep OPENCV_TEST_DATA_PATH optional.

Prefer to use environment variable for OPENCV_TEST_DATA_PATH.

Did i get you right, that the tests for the QRCodeDetector should be skipped when the enviroment variable OPENCV_TEST_DATA_PATH is not set? Because the tests depend on two files from the opencv_extra repository.

Please drop OPENCV_TEST_DATA_PATH changes from the patch

alalek · 2021-01-21T22:44:08Z

modules/js/CMakeLists.txt

+list(APPEND opencv_test_js_file_deps "${test_data_haarcascade_path}" "${opencv_test_js_bin_dir}/${test_data_haarcascade}")
+# QR code
+set(test_data_qrcode_1 "link_ocv.jpg")
+set(test_data_qrcode_1_path "${OPENCV_TEST_DATA_PATH}/cv/qrcode/${test_data_qrcode_1}")


Copy data only if build_tests enabled.

alalek · 2021-01-21T22:45:57Z

modules/js/generator/embindgen.py

+    'string':'std::string', # Side effect
    'String': 'std::string',


Keep existed coding style.

Please remove the comment as it doesn't help and just confuse readers.

Thank you, i will remove the comment and fix the coding style.
If you swap the order of the entry 'string': 'std::string' (line 122) with 'String': 'std::string' (line 123) a double replacement happens and the code doesn't work anymore.
Should i add a comment there with a warning to not swap the order of the entries?

Thank you for information! I will take a look on this behavior

modules/js/test/test_objdetect.js

alalek · 2021-01-21T22:52:54Z

modules/js/test/test_objdetect.js

+        const imgSingleQrCode = new Image();
+        imgSingleQrCode.crossOrigin = 'anonymous';
+        imgSingleQrCode.onload = function () {
+            const canvas = document.createElement('canvas');
+            const ctx = canvas.getContext('2d');


Does it work in Node.JS ?

You are right, this is not working in Node.js.
In order to provide tests i have seen other JavaScript tests, where images are created from a hardcoded array.
In this test case it would probably be a huge array to create an image containing a QR-code.
Is there a way of loading images, which works both in the browser and in node.js? Otherwise i have to implement two different ways of loading the images.

Until we have good images (testdata) support in JS tests infrastructure, lets check the "bindings" existence only (without any attempt to perform accuracy checks).

Please use Mat.zeros(...) as input in tests for now. And check that there are no exceptions raised. Minimal smoke test.

For accuracy demo it would be nice to add tutorial page (examples).

I have now added tests that do not require any test images.
Therefore I have removed the copying of the test images again.
I will start writing a tutorial page. Should the tutorial include the QR code recognition algorithm? As far as I could understand QrCodeDetector::detect uses the algorithm described in this article.

Ziachnix · 2021-01-29T08:33:52Z

WIP: Sorry, something went wrong during the rebase

- cherry picked commit to solve rebase error

asmorkalov · 2021-02-16T10:40:22Z

@Ziachnix Do you have progress with the PR?

alalek

Thank you for update!

alalek · 2021-02-23T21:34:45Z

doc/js_tutorials/js_setup/js_setup/js_setup.markdown

    @endcode

+    @note
+    It requires the [opencv_extra repository](https://github.com/opencv/opencv.git) located in your development environment.


Please drop OPENCV_TEST_DATA_PATH changes from the patch

asmorkalov · 2021-03-12T08:53:26Z

@Ziachnix Friendly reminder.

Ziachnix · 2021-03-13T13:14:03Z

@Ziachnix Friendly reminder.

Thank you, i totally forgot that there is still some work to be done.
I will open a new pull request when the tutorial page is ready to be merged.

Ziachnix force-pushed the feature/js-qr-code-detector branch from 1021af6 to 5abb733 Compare January 13, 2021 08:30

asmorkalov added the category: javascript (js) label Jan 20, 2021

alalek reviewed Jan 21, 2021

View reviewed changes

Ziachnix force-pushed the feature/js-qr-code-detector branch from 3fb1088 to 14fbe63 Compare January 29, 2021 08:29

Ziachnix force-pushed the feature/js-qr-code-detector branch from 14fbe63 to d6bd688 Compare January 29, 2021 08:54

ADD: js support for qrCodeDetector

c22d409

- cherry picked commit to solve rebase error

Ziachnix force-pushed the feature/js-qr-code-detector branch from d6bd688 to c22d409 Compare February 1, 2021 11:30

CHG. Revert haarcascade path

064c37a

Ziachnix added 2 commits February 17, 2021 10:13

FIX: Tests without images

251693a

ADD: decodeCurved

8181f03

alalek reviewed Feb 23, 2021

View reviewed changes

js(docs): don't require OPENCV_TEST_DATA_PATH

da50980

alalek approved these changes Mar 13, 2021

View reviewed changes

alalek merged commit 960f501 into opencv:3.4 Mar 13, 2021

alalek mentioned this pull request Mar 13, 2021

(4.x) Merge 3.4 #19722

Merged

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #19885

Merged

		'string':'std::string', # Side effect
		'String': 'std::string',

Uh oh!

Conversation

Ziachnix commented Jan 7, 2021 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

alalek left a comment

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!

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!

Uh oh!

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!

Ziachnix commented Jan 29, 2021

Uh oh!

asmorkalov commented Feb 16, 2021

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmorkalov commented Mar 12, 2021

Uh oh!

Ziachnix commented Mar 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Ziachnix commented Jan 7, 2021 •

edited by alalek

Loading