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

Add fisheye rectification #441

Conversation

DavidTorresOcana
Copy link
Contributor

@DavidTorresOcana DavidTorresOcana commented Jul 31, 2019

This PR introduces changes to be able to rectify fisheye cameras calibrated with #440 #580 and #592 and aim to Fix #146

This PR depends on ros-perception/vision_opencv#306 and it is related to ros-perception/image_common#146

As this PR only intends to upgrade image_proc package with fisheye support, it was tested only with monocular cameras.

It introduces:

  • Rename class PinholeCameraModel to CameraModel and headers: PinholeCameraModel is misleading. Rename the class and file to a more general name.

    • The class CameraModel does use the distortion model specified in the camera_info, including pinhole, equidistant and rational_polynomial and is able to run all methods in image_proc, regardless of the model used

Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

You should use image transport, and this should probably be inside of image_proc package, otherwise LGTM when CI is passing and targeted to melodic

This reverts commit 1b83555.
PinholeCameraModel is misleading now that fisheye model (KB) was introduced. Hence, rename the class and file to a more general name.
Class CameraModel does use the distortion model specified in the camera_info, namely pinhole, fisheye and rationa_polynomial
@SteveMacenski
Copy link
Member

@DavidTorresOcana this cant be merged until the upstream things have been merged that you pointed to.

But given its just header changes, I have no issue merging once that time comes

@DavidTorresOcana
Copy link
Contributor Author

@DavidTorresOcana this cant be merged until the upstream things have been merged that you pointed to.

But given its just header changes, I have no issue merging once that time comes

It would be good if you could ask to push the upstream of PRs up

@SteveMacenski
Copy link
Member

@DavidTorresOcana I don't have control over those repos

@DavidTorresOcana
Copy link
Contributor Author

DavidTorresOcana commented Aug 2, 2020

Successfully tested in melodic with ros-perception/vision_opencv#306 , #580 and #581 on a monocular camera

@DavidTorresOcana
Copy link
Contributor Author

DavidTorresOcana commented Mar 1, 2021

Closing since ros-perception/vision_opencv#358 was finally merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support fisheye calibration using opencv 3.0
2 participants