gazebosim / gazebo-classic

Gazebo classic. For the latest version, see https://github.com/gazebosim/gz-sim
http://classic.gazebosim.org/
Other
1.21k stars 484 forks source link

Camera: expose intrinsics parameters #3245

Closed deepanshubansal01 closed 2 years ago

deepanshubansal01 commented 2 years ago

This PR makes efforts to expose camera intrinsic parameters so that they can be accessed by gazebo_ros_camera plugin. Currently, the Camera API exposes a ProjectionMatrix() which is normalised based on Open GL graphics and whose values doesn't make sense to the user using the API.

The PR exposed ImageFocalLengthX(), ImageFocalLengthY(), ImageOpticalCentreX() and ImageOpticalCentreY() API's. These values are populated from the intrinsic tag in case intrinsic are provided in sdf file. If no intrinsic are provided then these values are decoupled from the ProjectionMatrix since the ProjectionMatrix is calculated by default based on fov, image size and we can decouple it to get the intrinsic.

Some related discussion here: https://github.com/ros-simulation/gazebo_ros_pkgs/pull/1407

Signed-off-by: deepanshu deepanshubansal01@gmail.com

deepanshubansal01 commented 2 years ago

Can we have this test case : We set the intrinsics as the default values, get the interinsics using FocalLengthX/Y(). Now don't set the intrinsics in sdf (for a separate camera), let the sensor use default values, and query the intrinsics again using the new methods. These values should be same as those obtained earlier, and the images should also be the same visually.

Yeah that’s a good test case to have. This test case should be in gazebo or gazebo_ros_pkgs?

deepanshubansal01 commented 2 years ago

Should we expose the API's for ImageFocalLengthX(), ImageOpticalCentreX() etc in CameraSensor similar to ImageWidth() and ImageHeight() API's? cc @adityapande-1995 @jacobperron https://github.com/osrf/gazebo/blob/127108a4137c70e59704281399465efe0e90fc19/gazebo/sensors/CameraSensor.hh#L97-L101

We can also get those values though from rendering::Camera object of CameraSensor https://github.com/osrf/gazebo/blob/127108a4137c70e59704281399465efe0e90fc19/gazebo/sensors/CameraSensor.hh#L135

jacobperron commented 2 years ago

Should we expose the API's for ImageFocalLengthX(), ImageOpticalCentreX() etc in CameraSensor similar to ImageWidth() and ImageHeight() API's?

Maybe, I'm not sure of the scenario where this might be required. I guess it's easy to add. This is probably a better question for @scpeters

scpeters commented 2 years ago

Should we expose the API's for ImageFocalLengthX(), ImageOpticalCentreX() etc in CameraSensor similar to ImageWidth() and ImageHeight() API's?

Maybe, I'm not sure of the scenario where this might be required. I guess it's easy to add. This is probably a better question for @scpeters

I think it would be simpler to just use the CameraSensor::Camera() API to get a rendering::Camera pointer and then use these APIs added in this PR from that object. So, I don't think additional changes are needed to the CameraSensor

https://github.com/osrf/gazebo/blob/gazebo11/gazebo/sensors/CameraSensor.hh#L93

scpeters commented 2 years ago

the new test is failing in macOS; I'll take a quick look and disable it if there's not an easy fix:

scpeters commented 2 years ago

the new test is failing in macOS; I'll take a quick look and disable it if there's not an easy fix:

disabled on macOS in https://github.com/osrf/gazebo/pull/3245/commits/71a7397cb894ea5a2845db401ddd39b7f28b573f