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

Fix python bindings for setCharucoParameters #23436

Merged
merged 3 commits into from Apr 17, 2023
Merged

Conversation

siilats
Copy link
Contributor

@siilats siilats commented Mar 29, 2023

setcharucoparameters fails in python

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

This hack works meanwhile:
image

setcharucoparameters fails in python
@dkurt
Copy link
Member

dkurt commented Mar 30, 2023

void CharucoDetector::setCharucoParameters(CharucoParameters &charucoParameters) {
should be changed too

@mostafarohani
Copy link

@dkurt thank you for the reference, and thank you @siilats for the workaround, however I think there is still a bug.

When I do a charuco find markers without using the intrinsics and distortions (i.e. a factory CharucoParamaters) the calibration/poses derived are reasonable but not great.

However when i feed in the ground truth intrinsics/distortions using CharucoParameters, the result is terrible/very wrong.

I have only noticed this because we have upgraded our repo from opencv 4.1 to 4.7; in the old code everything worked completely fine.

@asmorkalov asmorkalov added the pr: needs test New functionality requires minimal tests set label Apr 1, 2023
@asmorkalov asmorkalov self-assigned this Apr 1, 2023
@asmorkalov asmorkalov self-requested a review April 1, 2023 11:55
@dkurt
Copy link
Member

dkurt commented Apr 10, 2023

@mostafarohani, I've added a test in Python, can you please take a look and if your example is different, please provide a reproducer.

Update: I were able to reproduce your issue, sorry. This is only when either cameraMatrix or distCoeffs is not initialized.

@asmorkalov asmorkalov removed the pr: needs test New functionality requires minimal tests set label Apr 10, 2023
@asmorkalov asmorkalov added this to the 4.8.0 milestone Apr 10, 2023
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.

It is possible to add more clear names instead of test_charuco_no_segfault_1, test_charuco_no_segfault_2. The tests are small and there are comments, so it's ok for me.

@dkurt
Copy link
Member

dkurt commented Apr 17, 2023

@AleksandrPanov, sure, done!

@asmorkalov asmorkalov removed their request for review April 17, 2023 06:32
@asmorkalov asmorkalov changed the title Update charuco_detector.hpp Fix python bindings for setCharucoParameters Apr 17, 2023
@asmorkalov asmorkalov merged commit 8512deb into opencv:4.x Apr 17, 2023
18 of 19 checks passed
@asmorkalov asmorkalov mentioned this pull request May 31, 2023
thewoz pushed a commit to thewoz/opencv that referenced this pull request Jan 4, 2024
Fix python bindings for setCharucoParameters opencv#23436

setCharucoParameters fails in python
Fixes: opencv#23440

### Pull Request Readiness Checklist

See details at https://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 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.

setCharucoParameters results in segfault when using the python opencv
5 participants