Skip to content
This repository has been archived by the owner on Feb 3, 2025. It is now read-only.

Camera: expose intrinsics parameters #3245

Merged
merged 15 commits into from
Sep 8, 2022
Merged

Conversation

deepanshubansal01
Copy link
Contributor

@deepanshubansal01 deepanshubansal01 commented Jul 27, 2022

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: ros-simulation/gazebo_ros_pkgs#1407

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

Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01 deepanshubansal01 self-assigned this Jul 27, 2022
@deepanshubansal01 deepanshubansal01 marked this pull request as draft July 27, 2022 19:47
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Copy link
Contributor

@adityapande-1995 adityapande-1995 left a comment

Choose a reason for hiding this comment

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

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.

@deepanshubansal01
Copy link
Contributor Author

deepanshubansal01 commented Jul 28, 2022

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?

Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
Copy link
Contributor Author

deepanshubansal01 commented Aug 1, 2022

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

Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@jacobperron
Copy link

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

Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@scpeters scpeters changed the title Expose camera intrinsics for gazebo_ros_camera plugin Camera: expose intrinsics parameters Aug 18, 2022
@scpeters
Copy link
Member

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

Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Copy link

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM

I'm not sure if we should expect CI to be completely green. I'll leave that up to @scpeters

Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Copy link
Contributor

@adityapande-1995 adityapande-1995 left a comment

Choose a reason for hiding this comment

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

Looks good to me !

@scpeters
Copy link
Member

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

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Copy link
Member

scpeters commented Sep 8, 2022

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 71a7397

@scpeters scpeters merged commit bf205dd into gazebo11 Sep 8, 2022
@scpeters scpeters deleted the deepanshu/optical-centre branch September 8, 2022 05:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants