Skip to content

backport C++ stereo/stereo_geom.cpp:5.x to calib3d/stereo_geom.cpp:4.x#26437

Merged
asmorkalov merged 2 commits intoopencv:4.xfrom
vrabaud:4x_calibration_base
Nov 11, 2024
Merged

backport C++ stereo/stereo_geom.cpp:5.x to calib3d/stereo_geom.cpp:4.x#26437
asmorkalov merged 2 commits intoopencv:4.xfrom
vrabaud:4x_calibration_base

Conversation

@vrabaud
Copy link
Copy Markdown
Contributor

@vrabaud vrabaud commented Nov 8, 2024

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

The corresponding C function dependencies aer removed when unused.
Note: cv::fisheye::stereoRectify is moved from fisheye.cpp to
stereo_geom.cpp.
@vrabaud vrabaud added the cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) label Nov 8, 2024
@vrabaud vrabaud requested a review from asmorkalov November 8, 2024 13:28
@vrabaud vrabaud marked this pull request as draft November 8, 2024 13:47
}
}

void cvUndistortPoints(const CvMat* _src, CvMat* _dst, const CvMat* _cameraMatrix,
Copy link
Copy Markdown
Contributor

@sturkmen72 sturkmen72 Nov 8, 2024

Choose a reason for hiding this comment

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

AFAIK these kind of functions were keeped in 4.x for compatibility as discussed in #26259

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Currently, this is only used internally, it is not part of the public C API (which does not have much in calib3d, cf https://github.com/opencv/opencv/blob/4.x/modules/calib3d/include/opencv2/calib3d/calib3d_c.h). This PR switches the C code that uses it to C++, so there is no need for that function anymore.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

for example https://github.com/shimat/opencvsharp uses cvUndistortPoints. shouldn't this be preserved?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

git grep cvUndistortPoints

dies not return anything in their repo, where do they use it? They use cv::undistortPoints which is the C++ API. modules/calib3d/src/calib3d_c_api.h is an internal header so modifying it fine, other projects cannot use the API it offers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry let me check again

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree, the C API is till used, but the API removed in this PR is INTERNAL to OpenCV, nobody outside of OpenCV is using it because they cannot use it: the header that declares it is not public, it is in src, not in include. cvUndistortPoints was removed from the public C API 6 years ago in 8f15a60#diff-7ae6742689a7ee66932275c29e27989687a7b9e6ca2549a765f5b27414c753daR53

The link you just sent me is from a different repo, the last commit on that file is "support 2.4.10" which is a very old OpenCV that is not supported anymore.

@vrabaud vrabaud marked this pull request as ready for review November 8, 2024 15:10
@asmorkalov asmorkalov self-assigned this Nov 11, 2024
@asmorkalov asmorkalov added this to the 4.11.0 milestone Nov 11, 2024
@asmorkalov asmorkalov merged commit 6f8c3b1 into opencv:4.x Nov 11, 2024
@asmorkalov asmorkalov mentioned this pull request Nov 13, 2024
thewoz pushed a commit to CobbsLab/OPENCV that referenced this pull request Feb 13, 2025
Backport C++ stereo/stereo_geom.cpp:5.x to calib3d/stereo_geom.cpp:4.x opencv#26437

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

Labels

cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants