Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change binarization in findChessboardCorners() #24564

Open
wants to merge 1 commit into
base: 4.x
Choose a base branch
from

Conversation

thewoz
Copy link
Contributor

@thewoz thewoz commented Nov 20, 2023

In testing to solve problem #23558 I noticed that on certain images CALIB_CB_FAST_CHECK fails.
The problem arises in the binarization of the pictures which is done by a "classic" method.
This causes images like the following to be binarized badly and the check fails.

chess

chess_T

What I did was to eliminate icvBinarizationHistogramBased() and instead use adaptiveThreshold()

After the change I tried the function on other nine different checkerboard images and had no problems.

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 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

@thewoz thewoz changed the title Update calibinit.cpp change binarization in findChessboardCorners() Nov 20, 2023
@s-trinh
Copy link
Contributor

s-trinh commented Nov 20, 2023

Maybe:

  • run the new method adaptiveThreshold()
  • if later the corners detection fails, fall back to the old approach?
  • or add a parameter to choose/customize the binarization method to use?

@thewoz
Copy link
Contributor Author

thewoz commented Nov 20, 2023

perhaps it might make sense to tie it to the CALIB_CB_ADAPTIVE_THRESH parameter.
if it is defined the adaptive threshold is used if not the old method is used.
what do you think?

@s-trinh
Copy link
Contributor

s-trinh commented Nov 20, 2023

Yes, using a flag looks good to me.

@thewoz
Copy link
Contributor Author

thewoz commented Nov 20, 2023

Hi,
I have test the pull request against the images:
BoardStereoL1-6.png BoardStereoR1-6.png and chess1-6.png

in

https://github.com/opencv/opencv_extra/tree/4.x/testdata/cv/cameracalibration

and the chessboard was found in each of them.

@asmorkalov asmorkalov self-assigned this Nov 21, 2023
@asmorkalov asmorkalov added this to the 4.9.0 milestone Nov 21, 2023
@AleksandrPanov
Copy link
Contributor

[ RUN      ] Calib3d_StereoCalibrate_CPP.regression
/home/ci/opencv/modules/ts/src/ts.cpp:618: Failure
Failed

	failure reason: Bad accuracy
	test case #-1
	seed: 00000000000c5a5d
-----------------------------------
	LOG:
The distance between y coordinates is too big(=4.24615) (used uncalibrated stereo), testcase 1

@thewoz, you have regressions in accuracy.

@thewoz
Copy link
Contributor Author

thewoz commented Dec 1, 2023

Hi I have been trying to figure out what is wrong.
What happens is that there are only 2 corners in one image, out of 28, that have an error of 4.24615px after rectifying the image on the y when the maximum is set to 4px. This causes the test to fail.

Investigating you more what comes out is that the problem comes from how findChessboardCorners() found the corners.
Since the binarization is different corners found come out slightly different.
findChessboardCorners() inside it calls cornerSubPix() with a very small mask (2x2) which in fact in practice is like not calling it.

In fact by calling cornerSubPix() externally from findChessboardCorners(), with a mask slightly larger than 6 pixels, as also suggested by the documentation of findChessboardCorners(), the problem is solved.

@thewoz
Copy link
Contributor Author

thewoz commented Dec 14, 2023

hello sorry to bother you all the time.
is there anything i can do?

@thewoz
Copy link
Contributor Author

thewoz commented Dec 14, 2023

now should work!

@asmorkalov asmorkalov modified the milestones: 4.9.0, 4.10.0 Dec 20, 2023
@thewoz
Copy link
Contributor Author

thewoz commented Jan 4, 2024

Hi @asmorkalov, I should have resolved the conflict.

@thewoz
Copy link
Contributor Author

thewoz commented May 9, 2024

hi,
is there anything I can do?

@AleksandrPanov AleksandrPanov modified the milestones: 4.10.0, 4.11.0 May 29, 2024
Copy link
Contributor

@AleksandrPanov AleksandrPanov left a comment

Choose a reason for hiding this comment

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

Tested with benchmark --configuration=generate_run --board_x=7 --board_y=6 --path=res_chessboard --synthetic_object=chessboard:

Before PR:
6x7

After PR:
6x7v2

There are more improvements than regressions, but additional testing needs to be done.

Comment on lines 511 to 656
if(!is_plain) {
if(flags & CALIB_CB_ADAPTIVE_THRESH)
{
// Assume that the checkerboard occupies 5% of the image
int min_size = cvRound((img.cols * img.rows * 0.05) / ((pattern_size.width+1) * (pattern_size.height+1)));
if(min_size%2==0) min_size += 1;
adaptiveThreshold(img, thresh_img_new, 255, ADAPTIVE_THRESH_MEAN_C, THRESH_BINARY, min_size, 0);
}
else
{
icvBinarizationHistogramBased(thresh_img_new); // process image in-place
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

fix the tabs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 1399 to 1412
goto finalize;
goto finalize;
Copy link
Contributor

Choose a reason for hiding this comment

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

does not apply to this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Comment on lines 1403 to 1412

Mat leftGrey = left;
if(left.channels() != 1) cv::cvtColor(left, leftGrey, cv::COLOR_BGR2GRAY);

Mat rightGrey = right;
if(right.channels() != 1) cv::cvtColor(right, rightGrey, cv::COLOR_BGR2GRAY);

cv::cornerSubPix(leftGrey, imgpt1[i], cv::Size(6,6), cv::Size(-1,-1), cv::TermCriteria(cv::TermCriteria::EPS + cv::TermCriteria::COUNT, 15, 0.1));
cv::cornerSubPix(rightGrey, imgpt2[i], cv::Size(6,6), cv::Size(-1,-1), cv::TermCriteria(cv::TermCriteria::EPS + cv::TermCriteria::COUNT, 15, 0.1));

Copy link
Contributor

Choose a reason for hiding this comment

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

Delete these changes and make a rebase from 4.x (cornerSubPix has already been added to 4.x)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@thewoz thewoz force-pushed the adaptive-findChessboardCorners branch from c7d8e1d to 314208f Compare May 29, 2024 09:17
@AleksandrPanov
Copy link
Contributor

@thewoz, rebase needs to be fixed, you have 622 commits now)
Will you do it or do you need help?

@thewoz
Copy link
Contributor Author

thewoz commented May 29, 2024

hi @AleksandrPanov,
maybe I'm doing the procedure wrong.
i did the rebase with the default branch 4.x
should i do it with another one? with upstream 4.x?

@AleksandrPanov
Copy link
Contributor

AleksandrPanov commented May 29, 2024

@thewoz, update your local repo to main opencv repo and make rebase

git checkout 4.x
git pull https://github.com/opencv/opencv
git push https://github.com/thewoz/opencv
git checkout adaptive-findChessboardCorners
git rebase 4.x

@thewoz
Copy link
Contributor Author

thewoz commented May 29, 2024

hi @AleksandrPanov
Sorry I don't want to make a mistake.
it asks me this.
how should i configure it?

git pull https://github.com/opencv/opencv
From https://github.com/opencv/opencv
 * branch                  HEAD       -> FETCH_HEAD
hint: You have divergent branches and need to specify how to reconcile them.
hint: You can do so by running one of the following commands sometime before
hint: your next pull:
hint: 
hint:   git config pull.rebase false  # merge
hint:   git config pull.rebase true   # rebase
hint:   git config pull.ff only       # fast-forward only
hint: 
hint: You can replace "git config" with "git config --global" to set a default
hint: preference for all repositories. You can also pass --rebase, --no-rebase,
hint: or --ff-only on the command line to override the configured default per
hint: invocation.
fatal: Need to specify how to reconcile divergent branches.

@AleksandrPanov
Copy link
Contributor

@thewoz

hint: You have divergent branches and need to specify how to reconcile them.
hint: You can do so by running one of the following commands sometime before

Did you make your commits in your local 4.x?
Roll back your 4.x branch to the default state.

image

It is better to make changes to your branch and not to "main/master"

@thewoz thewoz force-pushed the adaptive-findChessboardCorners branch from 549ec68 to 774efd1 Compare May 30, 2024 09:20
@thewoz
Copy link
Contributor Author

thewoz commented May 30, 2024

Hi @AleksandrPanov,
I hope I was able to do everything you asked me to do.
I try to repeat the tests I had done last time?
The banchmarks I never figured out how to run them.

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.

None yet

4 participants