Skip to content

QRCodeDetector::decodeMulti() fixed invalid usage fixedType()#19407

Merged
alalek merged 4 commits intoopencv:3.4from
rayonnant14:issue_19363
Feb 5, 2021
Merged

QRCodeDetector::decodeMulti() fixed invalid usage fixedType()#19407
alalek merged 4 commits intoopencv:3.4from
rayonnant14:issue_19363

Conversation

@rayonnant14
Copy link
Copy Markdown
Contributor

@rayonnant14 rayonnant14 commented Jan 27, 2021

Merge with extra: opencv/opencv_extra#845

fix #19363

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

vector<Mat> tmp_straight_qrcodes;
if (straight_qrcode.needed())
if (straight_qrcode.needed() &&
(straight_barcode.size() > 0))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

straight_barcode.size() > 0

if (straight_qrcode.needed() && straight_barcode.size() == 0)
{
    straight_qrcode.release()
}
else if (...)

((OutputArray)tmp_straight_qrcodes[i]).type() : CV_32FC2);
straight_barcode[i].convertTo(tmp_straight_qrcodes[i],
straight_qrcode.fixedType() ?
straight_qrcode.type() : CV_32FC2);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

CV_32FC2

What is stored here?

Documentation says:

The optional output vector of images containing rectified and binarized QR codes
The optional output image containing rectified and binarized QR code

What is about simple tests for checking output parameter types?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

binarized

https://en.wikipedia.org/wiki/Binary_image

Why do we need floating point type here? 8UC1 is enough.

@rayonnant14 rayonnant14 requested a review from alalek February 3, 2021 16:58
changed default barcode type to CV_8UC1
added tests
added assert in case multi channel straight barcode input
qrdec.getStraightBarcode().convertTo(straight_qrcode,
straight_qrcode.fixedType() ?
straight_qrcode.type() : CV_32FC2);
straight_qrcode.type() : CV_8UC1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps we should ignore this .fixedType() check and force 8UC1 only.
Exception would be raised in case of type mismatch (by convertTo).

The assertion check above about channels is not necessary too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@alalek convertTo isn't raise a exception in case type or channel number mismatch, but there will be exception later Mat::type() == traits::Type<_Tp>::value if we don't use .fixedType() check.
By this reason, it is better to continue use assertion check about channels number and .fixedType() check

}
straight_qrcode.createSameSize(tmp_straight_qrcodes, CV_32FC2);
straight_qrcode.create(OutputArray(tmp_straight_qrcodes).size(),
OutputArray(tmp_straight_qrcodes).type());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OutputArray(tmp_straight_qrcodes)

No need to wrap into OutputArray.
You already have all necessary values.

@alalek alalek merged commit 0be18f5 into opencv:3.4 Feb 5, 2021
@alalek alalek mentioned this pull request Feb 5, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

QRCodeDetector::decodeMulti() invalid usage of .fixedType()

2 participants