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

[rolling] image_publisher: add field of view parameter #985

Merged

Conversation

Kotochleb
Copy link
Contributor

@Kotochleb Kotochleb commented May 26, 2024

Currently, the default value for focal length when no camera info is provided defaults to 1.0 rendering whole approximate intrinsics and projection matrices useless. Based on this article, I propose a better approximation of the focal length based on the field of view of the camera.

For most of the use cases, users will either know the field of view of the camera the used, or they already calibrated it ahead of time.

If there is some documentation to fill. please let me know.

This PR should be straightforward to port it to Humble, Iron and Jazzy.

@Kotochleb Kotochleb changed the title Humble: Add field of view parameter Humble: image_publisher, add field of view parameter May 26, 2024
@Kotochleb Kotochleb changed the title Humble: image_publisher, add field of view parameter [humble] image_publisher: add field of view parameter May 26, 2024
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Do you mind to target rolling ? then we can backport this to the other distros.

image_publisher/src/image_publisher.cpp Outdated Show resolved Hide resolved
Kotochleb and others added 2 commits May 27, 2024 12:29
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@Kotochleb Kotochleb force-pushed the feature/image-publisher-add-fov branch from c9cc228 to 4762cac Compare May 27, 2024 10:35
@Kotochleb Kotochleb changed the base branch from humble to rolling May 27, 2024 10:35
@Kotochleb Kotochleb changed the title [humble] image_publisher: add field of view parameter [rolling] image_publisher: add field of view parameter May 27, 2024
@Kotochleb
Copy link
Contributor Author

@ahcorde the changes were retargeted to rolling as requested, plus I added documentation for the new parameter since this branch accounts for that.

@Kotochleb Kotochleb requested a review from ahcorde May 27, 2024 10:49
camera_info_.k = {1, 0, static_cast<float>(camera_info_.width / 2), 0, 1,

// Based on https://learnopencv.com/approximate-focal-length-for-webcams-and-cell-phone-cameras/
double f_approx = (camera_info_.width / 2) / std::tan((field_of_view_ * M_PI / 180) / 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

is the default value 50 returning 1 ? as the current behavior ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't, and I forgot to account for breaking changes. I changed it now so when FOV is 0 or less, it disables the whole feature and uses 1.0 for focal length instead. The default vale is now zero as well, so now the whole thing is not breaking anything.

@Kotochleb Kotochleb requested a review from ahcorde May 28, 2024 09:14
@ahcorde ahcorde merged commit 78d80f7 into ros-perception:rolling May 28, 2024
3 checks passed
@ahcorde
Copy link
Contributor

ahcorde commented May 28, 2024

https://github.com/Mergifyio backport jazzy humble iron

Copy link

mergify bot commented May 28, 2024

backport jazzy humble iron

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request May 28, 2024
Currently, the default value for focal length when no camera info is
provided defaults to `1.0` rendering whole approximate intrinsics and
projection matrices useless. Based on [this
article](https://learnopencv.com/approximate-focal-length-for-webcams-and-cell-phone-cameras/),
I propose a better approximation of the focal length based on the field
of view of the camera.

For most of the use cases, users will either know the field of view of
the camera the used, or they already calibrated it ahead of time.

If there is some documentation to fill. please let me know.

This PR should be straightforward to port it to `Humble`, `Iron` and
`Jazzy`.

---------

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
(cherry picked from commit 78d80f7)
mergify bot pushed a commit that referenced this pull request May 28, 2024
Currently, the default value for focal length when no camera info is
provided defaults to `1.0` rendering whole approximate intrinsics and
projection matrices useless. Based on [this
article](https://learnopencv.com/approximate-focal-length-for-webcams-and-cell-phone-cameras/),
I propose a better approximation of the focal length based on the field
of view of the camera.

For most of the use cases, users will either know the field of view of
the camera the used, or they already calibrated it ahead of time.

If there is some documentation to fill. please let me know.

This PR should be straightforward to port it to `Humble`, `Iron` and
`Jazzy`.

---------

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
(cherry picked from commit 78d80f7)

# Conflicts:
#	image_publisher/doc/components.rst
mergify bot pushed a commit that referenced this pull request May 28, 2024
Currently, the default value for focal length when no camera info is
provided defaults to `1.0` rendering whole approximate intrinsics and
projection matrices useless. Based on [this
article](https://learnopencv.com/approximate-focal-length-for-webcams-and-cell-phone-cameras/),
I propose a better approximation of the focal length based on the field
of view of the camera.

For most of the use cases, users will either know the field of view of
the camera the used, or they already calibrated it ahead of time.

If there is some documentation to fill. please let me know.

This PR should be straightforward to port it to `Humble`, `Iron` and
`Jazzy`.

---------

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
(cherry picked from commit 78d80f7)

# Conflicts:
#	image_publisher/doc/components.rst
ahcorde pushed a commit that referenced this pull request May 28, 2024
Currently, the default value for focal length when no camera info is
provided defaults to `1.0` rendering whole approximate intrinsics and
projection matrices useless. Based on [this
article](https://learnopencv.com/approximate-focal-length-for-webcams-and-cell-phone-cameras/),
I propose a better approximation of the focal length based on the field
of view of the camera.

For most of the use cases, users will either know the field of view of
the camera the used, or they already calibrated it ahead of time.

If there is some documentation to fill. please let me know.

This PR should be straightforward to port it to `Humble`, `Iron` and
`Jazzy`.
<hr>This is an automatic backport of pull request #985 done by
[Mergify](https://mergify.com).

Co-authored-by: Krzysztof Wojciechowski <49921081+Kotochleb@users.noreply.github.com>
ahcorde pushed a commit that referenced this pull request May 28, 2024
Currently, the default value for focal length when no camera info is
provided defaults to `1.0` rendering whole approximate intrinsics and
projection matrices useless. Based on [this
article](https://learnopencv.com/approximate-focal-length-for-webcams-and-cell-phone-cameras/),
I propose a better approximation of the focal length based on the field
of view of the camera.

For most of the use cases, users will either know the field of view of
the camera the used, or they already calibrated it ahead of time.

If there is some documentation to fill. please let me know.

This PR should be straightforward to port it to `Humble`, `Iron` and
`Jazzy`.
<hr>This is an automatic backport of pull request #985 done by
[Mergify](https://mergify.com).

Co-authored-by: Krzysztof Wojciechowski <49921081+Kotochleb@users.noreply.github.com>
mikeferguson pushed a commit that referenced this pull request Jun 10, 2024
…992)

Currently, the default value for focal length when no camera info is
provided defaults to `1.0` rendering whole approximate intrinsics and
projection matrices useless. Based on [this
article](https://learnopencv.com/approximate-focal-length-for-webcams-and-cell-phone-cameras/),
I propose a better approximation of the focal length based on the field
of view of the camera.

For most of the use cases, users will either know the field of view of
the camera the used, or they already calibrated it ahead of time.

If there is some documentation to fill. please let me know.

This PR should be straightforward to port it to `Humble`, `Iron` and
`Jazzy`.
<hr>This is an automatic backport of pull request #985 done by
[Mergify](https://mergify.com).

Co-authored-by: Krzysztof Wojciechowski <49921081+Kotochleb@users.noreply.github.com>
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.

2 participants