Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork56.4k
Fix intrinsics processing in case USAC parameters#21166
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
alalek left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thank you!
Please add simple regression tests case based on code from the issue.
modules/calib3d/src/usac/utils.cpp Outdated
| voidUtils::calibratePoints (const Mat &K1,const Mat &K2,const Mat &points, Mat &calib_points) { | ||
| constauto *const points_ = (float *) points.data; | ||
| constauto *const k1 =(double *) K1.data; | ||
| constauto *const k1 =K1.isContinuous() ? (double *) K1.data : (double *) K1.clone().data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
K1.clone().data
Don't use dangling pointers.
This expression creates temporary object which is disposed before the next line.
To declare assumptions of this code we should have several checks:
CV_Assert(K1.isContinuous());CV_CheckDepthEQ(K1.depth(), CV_32F, "");due to assumption(double *) K1.dataCV_CheckEQ(K1.total(), (size_t)9, "");- and similar for other types.
To avoid such "void" interface checks it is better to useconst Matx33d& K1 parameters in internal API.
.clone() should be done by caller function of this function (or convert toMatx33d).
alalek left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Well done! Thank you 👍
Fix intrinsics processing in case USAC parameters* fixed USAC intrinsics processing* change mat to matx33d, added test
Uh oh!
There was an error while loading.Please reload this page.
fixes#21105
relates#17683
Pull Request Readiness Checklist
See details athttps://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.