Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Bugfix/qrcode version estimator#24364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@bagelbytes61
Copy link
Contributor

@bagelbytes61bagelbytes61 commentedOct 5, 2023•
edited by asmorkalov
Loading

Fixes#24366

Pull Request Readiness Checklist

See details athttps://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

@bagelbytes61bagelbytes61 changed the base branch from4.x to4.8.xOctober 5, 2023 17:57
@bagelbytes61bagelbytes61 marked this pull request as draftOctober 5, 2023 18:11
@bagelbytes61bagelbytes61force-pushed thebugfix/qrcode-version-estimator branch 4 times, most recently from05fa58a tofd815d8CompareOctober 6, 2023 02:25
@bagelbytes61bagelbytes61 marked this pull request as ready for reviewOctober 6, 2023 02:25
@asmorkalovasmorkalov added this to the4.9.0 milestoneOct 6, 2023
@asmorkalovasmorkalov changed the base branch from4.8.x to4.xOctober 6, 2023 10:25
@dkurt
Copy link
Member

Please rebase source branch

@bagelbytes61bagelbytes61force-pushed thebugfix/qrcode-version-estimator branch fromfd815d8 to62102d3CompareOctober 6, 2023 14:10
@bagelbytes61
Copy link
ContributorAuthor

Please rebase source branch

Done!

@AleksandrPanov
Copy link
Contributor

AleksandrPanov commentedOct 7, 2023•
edited
Loading

@bagelbytes61, several tests have fallen:

Objdetect_QRCode_Encode_Decode.regression :unknown file: FailureC++ exception with description "OpenCV(4.8.0-dev) /build/precommit_linux64/4.x/opencv/modules/objdetect/src/qrcode_encoder.cpp:371: error: (-5:Bad argument) The given version is not suitable for the given input string length  in function 'generateQR'" thrown in the test body.[  FAILED  ] Objdetect_QRCode_Encode.regression/3, where GetParam() = "version2_mode1.png"[  FAILED  ] Objdetect_QRCode_Encode.regression/4, where GetParam() = "version2_mode2.png"[  FAILED  ] Objdetect_QRCode_Encode.regression/6, where GetParam() = "version3_mode2.png"

https://pullrequest.opencv.org/buildbot/builders/precommit_linux64/builds/104823

https://pullrequest.opencv.org/buildbot/builders/precommit_linux64/builds/104823/steps/test_objdetect/logs/tests%20summary

There were also warnings:

/build/precommit_linux64/4.x/opencv/modules/objdetect/src/qrcode_encoder.cpp:681:12: warning: enumeration value 'MODE_AUTO' not handled in switch [-Wswitch]/build/precommit_linux64/4.x/opencv/modules/objdetect/src/qrcode_encoder.cpp:681:12: warning: enumeration value 'MODE_ECI' not handled in switch [-Wswitch]/build/precommit_linux64/4.x/opencv/modules/objdetect/src/qrcode_encoder.cpp:681:12: warning: enumeration value 'MODE_KANJI' not handled in switch [-Wswitch]/build/precommit_linux64/4.x/opencv/modules/objdetect/src/qrcode_encoder.cpp:681:12: warning: enumeration value 'MODE_STRUCTURED_APPEND' not handled in switch [-Wswitch]

@AleksandrPanov
Copy link
Contributor

[  FAILED  ] 4 tests, listed below:[  FAILED  ] Objdetect_QRCode_Encode_Decode.regression[  FAILED  ] Objdetect_QRCode_Encode.regression/3, where GetParam() = "version2_mode1.png"[  FAILED  ] Objdetect_QRCode_Encode.regression/4, where GetParam() = "version2_mode2.png"[  FAILED  ] Objdetect_QRCode_Encode.regression/6, where GetParam() = "version3_mode2.png"

@asmorkalov
Copy link
Contributor

@bagelbytes61 Thanks for the patch. The fix changes version for several test cases, where version is set to auto (0). You need to create PR with the same branch name as this one to OpenCV extra and update reference images. Example:

[ RUN      ] Objdetect_QRCode_Encode.regression/3, where GetParam() = "version2_mode1.png"[DEBUG:0@0.152] global qrcode_encoder.cpp:370 generateQR QR code detected_version: 4 version_level: 0src resolution: 29x29result resolution: 37x37unknown file: FailureC++ exception with description "OpenCV(4.8.0-dev) /home/alexander/Projects/OpenCV/opencv-master/modules/core/src/arithm.cpp:652: error: (-209:Sizes of input arguments do not match) The operation is neither 'array op array' (where arrays have the same size and the same number of channels), nor 'array op scalar', nor 'scalar op array' in function 'arithm_op'" thrown in the test body.[  FAILED  ] Objdetect_QRCode_Encode.regression/3, where GetParam() = "version2_mode1.png" (76 ms)

@asmorkalov
Copy link
Contributor

@dkurt Could you take a look too? Loos like the PR overestimate minimal version of QR code for several test cases. The QR codes in our test data contains full (not truncated) sequence.

dkurt reacted with thumbs up emoji

@bagelbytes61
Copy link
ContributorAuthor

@bagelbytes61 Thanks for the patch. The fix changes version for several test cases, where version is set to auto (0). You need to create PR with the same branch name as this one to OpenCV extra and update reference images. Example:

[ RUN      ] Objdetect_QRCode_Encode.regression/3, where GetParam() = "version2_mode1.png"[DEBUG:0@0.152] global qrcode_encoder.cpp:370 generateQR QR code detected_version: 4 version_level: 0src resolution: 29x29result resolution: 37x37unknown file: FailureC++ exception with description "OpenCV(4.8.0-dev) /home/alexander/Projects/OpenCV/opencv-master/modules/core/src/arithm.cpp:652: error: (-209:Sizes of input arguments do not match) The operation is neither 'array op array' (where arrays have the same size and the same number of channels), nor 'array op scalar', nor 'scalar op array' in function 'arithm_op'" thrown in the test body.[  FAILED  ] Objdetect_QRCode_Encode.regression/3, where GetParam() = "version2_mode1.png" (76 ms)

Apologize for not getting back to you sooner. I addressed the test case failures except for the ECI case is still failing. Would you be able to provide any insight into the failure? I don't know much about QR code technology, so if there's a technical reason for the failures I am afraid that it may be a bit over my head...

@dkurtdkurt self-assigned thisNov 14, 2023
@dkurtdkurtforce-pushed thebugfix/qrcode-version-estimator branch from12e667a to64a5ee8CompareNovember 14, 2023 07:21
@dkurtdkurtforce-pushed thebugfix/qrcode-version-estimator branch from64a5ee8 tobdfa697CompareNovember 15, 2023 08:12
Copy link
Contributor

@asmorkalovasmorkalov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

@asmorkalovasmorkalov merged commitfad0dbb intoopencv:4.xNov 16, 2023
Copy link
Contributor

@AleksandrPanovAleksandrPanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

IskXCr pushed a commit to Haosonn/opencv that referenced this pull requestDec 20, 2023
…on-estimatorBugfix/qrcode version estimatoropencv#24364Fixesopencv#24366### Pull Request Readiness ChecklistSee details athttps://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request- [x] I agree to contribute to the project under Apache 2 License.- [x] 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- [x] The PR is proposed to the proper branch- [x] There is a reference to the original bug report and related work- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable      Patch to opencv_extra has the same branch name.- [x] The feature is well documented and sample code can be built with the project CMake
thewoz pushed a commit to thewoz/opencv that referenced this pull requestJan 4, 2024
…on-estimatorBugfix/qrcode version estimatoropencv#24364Fixesopencv#24366### Pull Request Readiness ChecklistSee details athttps://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request- [x] I agree to contribute to the project under Apache 2 License.- [x] 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- [x] The PR is proposed to the proper branch- [x] There is a reference to the original bug report and related work- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable      Patch to opencv_extra has the same branch name.- [x] The feature is well documented and sample code can be built with the project CMake
@asmorkalovasmorkalov mentioned this pull requestJan 19, 2024
thewoz pushed a commit to thewoz/opencv that referenced this pull requestMay 29, 2024
…on-estimatorBugfix/qrcode version estimatoropencv#24364Fixesopencv#24366### Pull Request Readiness ChecklistSee details athttps://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request- [x] I agree to contribute to the project under Apache 2 License.- [x] 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- [x] The PR is proposed to the proper branch- [x] There is a reference to the original bug report and related work- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable      Patch to opencv_extra has the same branch name.- [x] The feature is well documented and sample code can be built with the project CMake
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@AleksandrPanovAleksandrPanovAleksandrPanov left review comments

@dkurtdkurtdkurt left review comments

@asmorkalovasmorkalovasmorkalov approved these changes

Assignees

@dkurtdkurt

Projects

None yet

Milestone

4.9.0

Development

Successfully merging this pull request may close these issues.

QRCodeEncoderImpl::estimateVersion does not properly estimate version for certain input length ranges

4 participants

@bagelbytes61@dkurt@AleksandrPanov@asmorkalov
[8]ページ先頭
©2009-2025 Movatter.jp