Add FISHEYE model definition

[DavidTorresOcana]

Add FISHEYE model definition to be used in image_geometry model definition.

[tfoote]

This seems reasonable. Can you please add documentation for exactly what model this is implementing? We need to make sure that the model is specifically clarified.

[DavidTorresOcana]

This seems reasonable. Can you please add documentation for exactly what model this is implementing? We need to make sure that the model is specifically clarified.

The model is described in this OpenCV library.

It corresponds to a specific case of the radial distortion Kannala-Brandt model

[DavidTorresOcana]

This seems reasonable. Can you please add documentation for exactly what model this is implementing? We need to make sure that the model is specifically clarified.

@tfoote Is there any more doubts? There are several PRs awaiting for this to be passed.

As off today Melodic has a fisheye calibration tool but not a way to rectify with that calibration.

Please let us move this.

[dirk-thomas]

@DavidTorresOcana Please do not request reviews from as many people as you can think off (same for ros-perception/image_common#146 and ros-perception/vision_opencv#306). A simple comment on the ticket asking for attention is sufficient and usually makes sure the maintainer of the repository get a notification.

[tfoote]

As I mentioned in my review please add documentation to the code with a comment explaining it and a link. A comment here in this PR will be hard for people to find in the future.

[tfoote]

Formally entering the review status. Please see my comments with requests for documentation.

[DavidTorresOcana]

As I mentioned in my review please add documentation to the code with a comment explaining it and a link. A comment here in this PR will be hard for people to find in the future.

Agree: See 06c96c3
Would that be ok?

[mintar]

Please do not merge this PR. The "fisheye" distortion model proposed in this PR is the same as the "equidistant" distortion model that we already have! The model is based on the following publication:

J. Kannala and S. Brandt (2006). A Generic Camera Model and Calibration Method for Conventional, Wide-Angle, and Fish-Eye Lenses, IEEE Transactions on Pattern Analysis and Machine Intelligence, vol. 28, no. 8, pp. 1335-1340

The "equidistant" model in sensor_msgs is exactly that: #109 (comment)

It's unfortunate that there are so many different names for the same distortion model:

• Kannala and Brandt call it "Generic Camera Model", so that didn't help. Sometimes they also call it "equidistance projection model".
• Kalibr calls it "equidistant": https://github.com/ethz-asl/kalibr/wiki/supported-models
• Camodocal calls it "kannala-brandt": https://github.com/hengli/camodocal
• OpenCV calls it "fisheye": https://stackoverflow.com/a/34309644/3036576

But all of these are just different names for the same model. Therefore, I suggest we keep the existing name "equidistant". Maybe add some comment to the source code that mentions all the different names for it.

[DavidTorresOcana]

Please do not merge this PR. The "fisheye" distortion model proposed in this PR is the same as the "equidistant" distortion model that we already have! The model is based on the following publication:

J. Kannala and S. Brandt (2006). A Generic Camera Model and Calibration Method for Conventional, Wide-Angle, and Fish-Eye Lenses, IEEE Transactions on Pattern Analysis and Machine Intelligence, vol. 28, no. 8, pp. 1335-1340

The "equidistant" model in sensor_msgs is exactly that: #109 (comment)

It's unfortunate that there are so many different names for the same distortion model:

• Kannala and Brandt call it "Generic Camera Model", so that didn't help. Sometimes they also call it "equidistance projection model".
• Kalibr calls it "equidistant": https://github.com/ethz-asl/kalibr/wiki/supported-models
• Camodocal calls it "kannala-brandt": https://github.com/hengli/camodocal
• OpenCV calls it "fisheye": https://stackoverflow.com/a/34309644/3036576

But all of these are just different names for the same model. Therefore, I suggest we keep the existing name "equidistant". Maybe add some comment to the source code that mentions all the different names for it.

Thank you for feedback. I agree camera model naming in CV field is chaotic. At the end the mathematical description is the best help and, as you pointed out, equidistant can do the job.

I agree to use EQUIDISTANT instead, so will update the pipeline accordingly.

The problem I see is that I developed the entire pipeline including the calibration tool ros-perception/image_pipeline#440 using FISHEYE which is being merged ros-perception/image_pipeline#542 We will have to update the calibration tool in those too

[DavidTorresOcana]

Closing this as the intended functionality was already available.