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

Not sure if binning should be considered in IPM, or be converted to image coordinates prior #16

Closed
ijnek opened this issue Jul 21, 2022 · 3 comments

Comments

@ijnek
Copy link
Member

ijnek commented Jul 21, 2022

Currently, binning is considered within IPM such that the input point can be a "binned" pixel coordinate (I'm not sure what the correct terminology here is).

I'm wondering if it is necessary to consider the binning inside IPM, or outside IPM by ensuring pixels are converted to those that match the original width x height of the image.

@Flova
Copy link
Contributor

Flova commented Jul 21, 2022

We are using this feature at Bit-Bots. The official image proc software binning node does it like that, and there is also support for that in the camera_info message. It only changes the binning params in the camera_info keeping it otherwise the same. I think the idea is that roi, binning, etc don't change the original camera calibration matrix and instead provide an additional transformation that needs to be applied. This makes it a bit harder for us, as we need to consider this, but it also makes the camera_info cleaner, as the same camera always has the same camera matrix. I think we should use the default way of handling this instead of e.g. creating an additional node that creates "unbinned" camera_infos.

Or did you mean "outside" of the projection, but still inside the ipm libary? Because I think we need to handle the binning param in the camera_info ether way. You could also scale the pixels instead of the camera_matrix, but this is the exact same thing only the other way around, and I see no benefits in changing the implementation. The binning param is available ether way at this stage due to the CameraInfo message being passed to the projection function. The presence of the complete CameraInfo msg type will also be helpful in the future if we consider the distortion matrix in the projection.

@ijnek
Copy link
Member Author

ijnek commented Jul 21, 2022

I see, thinking twice I think we should handle binning_x and binning_y inside IPM, and project_point() should be called with "binned" pixels like you currently are doing.

For consistency, would it make sense to handle ROIs from CameraInfo aswell? (Related to #25)

@Flova
Copy link
Contributor

Flova commented Jul 22, 2022

It makes sense to handle ROIs but their implementation is currently not my to priory. We should open an issue for this.

@ijnek ijnek mentioned this issue Jul 22, 2022
@ijnek ijnek closed this as not planned Won't fix, can't repro, duplicate, stale Jul 22, 2022
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

No branches or pull requests

2 participants