Sanity check tutorial_charuco_create_detect.cpp::readCameraParameters() #2950
Conversation
There was a problem hiding this comment.
Thank you for contribution!
This patch should go into 3.4 branch first.
We will merge changes from 3.4 into master regularly (weekly/bi-weekly).
Please:
- change "base" branch of this PR: master => 3.4 (use "Edit" button near PR title)
- rebase your commits from master onto 3.4 branch. For example:
git rebase -i --onto upstream/3.4 upstream/master
(check list of your commits, save and quit (Esc + "wq" + Enter)
whereupstreamis configured by following this GitHub guide and fetched (git fetch upstream). - push rebased commits into source branch of your fork (with
--forceoption)
Note: no needs to re-open PR, apply changes "inplace".
| @@ -21,7 +21,7 @@ static bool readCameraParameters(std::string filename, cv::Mat& camMatrix, cv::M | |||
| return false; | |||
| fs["camera_matrix"] >> camMatrix; | |||
| fs["distortion_coefficients"] >> distCoeffs; | |||
| return true; | |||
| return (camMatrix.size().height == 3 && distCoeffs.size().width == 5); | |||
There was a problem hiding this comment.
Maybe (camMatrix.size() == cv::Size(3, 3) && distCoeffs.size() == cv::Size(5,1))
|
I think I mistyped some of that git magic and the push --force seemed to close this. I think I might have corrected https://github.com/drf5n/opencv_contrib/tree/patch-1 |
| @@ -21,7 +21,7 @@ static bool readCameraParameters(std::string filename, cv::Mat& camMatrix, cv::M | |||
| return false; | |||
| fs["camera_matrix"] >> camMatrix; | |||
| fs["distortion_coefficients"] >> distCoeffs; | |||
| return true; | |||
| return (camMatrix.size() == cv::Size(3,3) && distCoeffs.size() == cv::Size(5,1)); | |||
There was a problem hiding this comment.
distCoeffs can be different (contains different number of elements).
I believe we should check this instead: !distCoeffs.empty() or even remove that check and keep camMatrix validation only.
There was a problem hiding this comment.
distCoeffs.empty() compiles and functions. I don't know anything about well formed distCoeffs.
If one edits the calib.txt file to an ill-formed 1x3 matrix, you get an informative error like:
libc++abi.dylib: terminating with uncaught exception of type cv::Exception: OpenCV(3.4.2) /opt/concourse/worker/volumes/live/9523d527-1b9e-48e0-7ed0-a36adde286f0/volume/opencv-suite_1535558719691/work/modules/imgproc/src/undistort.cpp:400: error: (-215:Assertion failed) (((_distCoeffs) != __null && (((const CvMat*)(_distCoeffs))->type & 0xFFFF0000) == 0x42420000 && ((const CvMat*)(_distCoeffs))->cols > 0 && ((const CvMat*)(_distCoeffs))->rows > 0) && ((const CvMat*)(_distCoeffs))->data.ptr != __null) && (_distCoeffs->rows == 1 || _distCoeffs->cols == 1) && (_distCoeffs->rows*_distCoeffs->cols == 4 || _distCoeffs->rows*_distCoeffs->cols == 5 || _distCoeffs->rows*_distCoeffs->cols == 8 || _distCoeffs->rows*_distCoeffs->cols == 12 || _distCoeffs->rows*_distCoeffs->cols == 14) in function 'cvUndistortPointsInternal'
If you edit the calib.txt file to eliminate the distortion_coefficients field, then !distCoeffs.empty() would test true, but without the test the program still is able to function and do the detection and pose estimation.
camMatrix validation only is good enough.
|
Thank you. |
Check for successful read of
camera_matrixanddistortion_coefficientsparameters in readCameraParameters()If the filename exists, but the values are not read, the routine used to return true with empty matricies. This changes to check if each matrix has a reasonable dimension.
The tutorial file should be robust against bad inputs and help learners recognize and identify problems.
This should solve #2949
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.