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

Add detect qr with aruco#23264

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

@AleksandrPanov
Copy link
Contributor

@AleksandrPanovAleksandrPanov commentedFeb 17, 2023•
edited
Loading

Using Aruco to detect finder patterns to search QR codes.

TODO (in next PR):

  • add single QR detect (updatedetect() anddetectAndDecode())
  • need reduce full enumeration of finder patterns
  • need add finder pattern info todecode step
  • need to merge the pipeline of the old and new algorithm

Current results:
+20% total detect, +8% total decode in OpenCVQR benchmark

res1

78.4% detect, 58.7% decode vs 58.5 detect, 50.5% decode in default

main.py.txt

res2

add new info togoogle docs

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

elejke reacted with rocket emoji
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.

Please add:

  • The table with benchmark results for 4.7 and new version;
  • Performance test (parameterize existing) and it's result.

@AleksandrPanovAleksandrPanovforce-pushed theadd_detect_qr_with_aruco branch 3 times, most recently frombdbe5de to339e8a1CompareMarch 23, 2023 22:40
@AleksandrPanovAleksandrPanov marked this pull request as ready for reviewMarch 29, 2023 13:12
@AleksandrPanovAleksandrPanovforce-pushed theadd_detect_qr_with_aruco branch 3 times, most recently from8602d7e to04785daCompareApril 5, 2023 15:41
@asmorkalovasmorkalov added this to the4.8.0 milestoneApr 7, 2023
@asmorkalov
Copy link
Contributor

/build/precommit_linux64/4.x/opencv/modules/objdetect/perf/perf_qrcode_pipeline.cpp:101:111: warning: passing 'cv::QRDetectMethod' chooses 'int' over 'unsigned int' [-Wsign-promo]/build/precommit_linux64/4.x/opencv/modules/objdetect/perf/perf_qrcode_pipeline.cpp:101:111: warning:   in call to 'std::__cxx11::string std::__cxx11::to_string(int)' [-Wsign-promo]/build/precommit_linux64/4.x/opencv/modules/objdetect/test/test_qrcode.cpp:371:111: warning: passing 'cv::QRDetectMethod' chooses 'int' over 'unsigned int' [-Wsign-promo]/build/precommit_linux64/4.x/opencv/modules/objdetect/test/test_qrcode.cpp:371:111: warning:   in call to 'std::__cxx11::string std::__cxx11::to_string(int)' [-Wsign-promo]

@asmorkalov
Copy link
Contributor

General notes:

  • Only::detectMulti is extended with the new algorithm.::detect,detectAndDecode,detectAndDecodeCurved are not covered.
  • It makes sense to decouple the old algorithm and new one as 2 internal classes with own virtual detect, detectMulti, etc. The only check point for algo type is impl pointer initialization.

@AleksandrPanovAleksandrPanovforce-pushed theadd_detect_qr_with_aruco branch 4 times, most recently fromcb7a30b to0d6eddfCompareApril 14, 2023 13:10
@asmorkalovasmorkalovforce-pushed theadd_detect_qr_with_aruco branch from3d9c6e5 tofca3934CompareMay 17, 2023 14:51
@asmorkalov
Copy link
Contributor

@opencv-alalek Could you update ABI-complience-checker settings to cover API changes.

@asmorkalov
Copy link
Contributor

@opencv-alalek Friendly reminder.

Comment on lines 907 to 908
/** @brief Aruco detector parameters are used to search for the finder patterns.*/
CV_PROP_RW aruco::DetectorParameters arucoParams;
Copy link
Contributor

Choose a reason for hiding this comment

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

broken indentation

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

fixed

ASSERT_FALSE(src.empty()) <<"Can't read image:" << image_path;
#ifdef HAVE_QUIRC
QRCodeDetector qrcode;
Ptr<QRCodeDetectorBase> qrcode;
Copy link
Contributor

Choose a reason for hiding this comment

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

PImpl doesn't needPtr<>. They are already smart pointers.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

removed Ptr

static
voidrunQR(
QRCodeDetector& qrcode,const Mat& input,
Ptr<QRCodeDetectorBase> qrcode,const Mat& input,
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. We don't needPtr for PImpl which is already smart ptr.
  2. Ptrs must be passed as "const reference" be default.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

removed Ptr, added ref

@param epsX Epsilon neighborhood, which allows you to determine the horizontal pattern
of the scheme 1:1:3:1:1 according to QR code standard.
*/
CV_WRAPvoidsetEpsX(double epsX);
Copy link
Contributor

Choose a reason for hiding this comment

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

All "set" methods should return*this reference to allow construction chains.

Copy link
ContributorAuthor

@AleksandrPanovAleksandrPanovMay 26, 2023•
edited
Loading

Choose a reason for hiding this comment

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

these setters drop CI (Python bindings)
Maybe update the setters later?

Copy link
Contributor

Choose a reason for hiding this comment

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

In DNNpyopencv_cv_dnn_dnn_ClassificationModel_setEnableSoftmaxPostProcessing has been properly generated.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

done.
The directiveCV_EXPORTS_W_SIMPLE instead ofCV_EXPORTS_W directive helped to add setter

opencv-alalek reacted with thumbs up emoji
Copy link
ContributorAuthor

@AleksandrPanovAleksandrPanovJun 1, 2023•
edited
Loading

Choose a reason for hiding this comment

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

first was addedQRCodeDetectorAruco& setDetectorParameters
now were addedQRCodeDetector& setEpsX,QRCodeDetector& setEpsY,QRCodeDetector& setUseAlignmentMarkers

now it's done)

@mshabuninmshabunin mentioned this pull requestMay 26, 2023
6 tasks
classCV_EXPORTS_W QRCodeDetector
{
classCV_EXPORTS_W QRCodeDetectorBase {
public:
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned on team meetings, we have to followdnn:Model approach.

Constructors / destructors / assignment / move operators should be defined explicitly.
Also we should not have public default constructor (but we have issue with bindings - deprecation and comment should be on-place).

Copy link
ContributorAuthor

@AleksandrPanovAleksandrPanovMay 31, 2023•
edited
Loading

Choose a reason for hiding this comment

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

added withdnn::Model style:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

//CV_DEPRECATED_EXTERNAL

Why is it commented?

AleksandrPanov reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

removed the comments

virtualbooldetectAndDecodeMulti(InputArray img, std::vector<std::string>& decoded_info, OutputArray points,
OutputArrayOfArrays straight_qrcode)const = 0;

virtual~Impl() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

It is better to declare virtual destructor first.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

done

};

boolQRCodeDetectorBase::detect(InputArray img, OutputArray points)const {
return p->detect(img, points);
Copy link
Contributor

Choose a reason for hiding this comment

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

->

All deference cases must be pre-checked for NULL values.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

addedCV_Assert(p != nullptr);

Copy link
Contributor

Choose a reason for hiding this comment

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

JustCV_Assert(p) as we don't really want to retrieve the pointer itself.

It is likecontainer.size() == 0 vscontainer.empty().

AleksandrPanov reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

added justCV_Assert(p)


QRCodeDetector::QRCodeDetector() : p(new Impl) {}
QRCodeDetector::QRCodeDetector() {
p =newImplContour();
Copy link
Contributor

Choose a reason for hiding this comment

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

new

makePtr

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

replaced

CV_CheckGT(contourArea(src_points),0.0,"Invalid QR code source points");

QRDecodeqrdec(p->useAlignmentMarkers);
QRDecodeqrdec((std::static_pointer_cast<ImplContour>)(p)->useAlignmentMarkers);
Copy link
Contributor

Choose a reason for hiding this comment

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

PImpl pattern classes should not have large method implementations.
All logic must go toImpl.

Copy link
ContributorAuthor

@AleksandrPanovAleksandrPanovMay 31, 2023•
edited
Loading

Choose a reason for hiding this comment

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

I propose to make this fix in another PR. This fix will erase the history and increase the diff.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I moveddecodeCurved anddetectAndDecodeCurved toImplContour. I think I've fixed everything you asked for. Is there anything else that needs to be fixed?

@param epsX Epsilon neighborhood, which allows you to determine the horizontal pattern
of the scheme 1:1:3:1:1 according to QR code standard.
*/
CV_WRAPvoidsetEpsX(double epsX);
Copy link
Contributor

Choose a reason for hiding this comment

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

In DNNpyopencv_cv_dnn_dnn_ClassificationModel_setEnableSoftmaxPostProcessing has been properly generated.

@AleksandrPanovAleksandrPanovforce-pushed theadd_detect_qr_with_aruco branch 2 times, most recently frome633c4c tof860430CompareMay 30, 2023 21:29
Copy link
Contributor

@opencv-alalekopencv-alalek left a comment

Choose a reason for hiding this comment

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

LGTM 👍

AleksandrPanov reacted with thumbs up emoji
classCV_EXPORTS_W QRCodeDetector
{
classCV_EXPORTS_W QRCodeDetectorBase {
public:
Copy link
Contributor

Choose a reason for hiding this comment

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

//CV_DEPRECATED_EXTERNAL

Why is it commented?

AleksandrPanov reacted with thumbs up emoji
};

boolQRCodeDetectorBase::detect(InputArray img, OutputArray points)const {
return p->detect(img, points);
Copy link
Contributor

Choose a reason for hiding this comment

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

JustCV_Assert(p) as we don't really want to retrieve the pointer itself.

It is likecontainer.size() == 0 vscontainer.empty().

AleksandrPanov reacted with thumbs up emoji
vector<vector<Point2f>> alignmentMarkers;
vector<Point2f> updateQrCorners;
mutablevector<vector<Point2f>> alignmentMarkers;
mutablevector<Point2f> updateQrCorners;
Copy link
Contributor

Choose a reason for hiding this comment

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

suspicious code, need to be revised later.

AleksandrPanov reacted with eyes emoji

QRCodeDetector::~QRCodeDetector() {}
QRCodeDetector&QRCodeDetector::setEpsX(double epsX) {
(std::static_pointer_cast<ImplContour>)(p)->epsX = epsX;
Copy link
Contributor

Choose a reason for hiding this comment

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

no need braces here:

std::static_pointer_cast<ImplContour>(p)

checkeddynamic_pointer_cast is more secure.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

addeddynamic_pointer_cast insteadstatic_pointer_cast

scaleTimingPatternScore =0.9f;
}

structFinderPatternInfo {
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, it makes sense to hide local internal classes (especially with some generic names) into inner anonymous namespace.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

addedFinderPatternInfo andQRCode into anonymous namespace

static
voidrunQR(
QRCodeDetector& qrcode,const Mat& input,
QRCodeDetectorBase& qrcode,const Mat& input,
Copy link
Contributor

Choose a reason for hiding this comment

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

Useconst reference for inputs

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

addedconst reference and added const todecode methods

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@asmorkalovasmorkalovasmorkalov approved these changes

@opencv-alalekopencv-alalekopencv-alalek approved these changes

Assignees

@asmorkalovasmorkalov

Projects

None yet

Milestone

4.8.0

Development

Successfully merging this pull request may close these issues.

5 participants

@AleksandrPanov@asmorkalov@opencv-alalek@VadimLevin@komakai
[8]ページ先頭
©2009-2025 Movatter.jp