-
Notifications
You must be signed in to change notification settings - Fork 30
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
Updating distortion model description #139
Changes from 9 commits
71d0152
e1e0be5
0cb706d
1b68b9d
a0b815e
e827dde
a5136bc
585d39f
eff2d06
36f0675
9305630
ac5693c
1648532
7b8fc29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd drop the reference to
Brown-Conrady
– I don't see that anywhere in OpenCV? In fact I believe OpenCV may have come up with their own implementation for certain things – don't quite remember where I learned that so I can't cite a source, maybe Brown–Conrady is accurate.Also nit: I don't think we say "Foxglove supports…" in other places, we just say "supported values":
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the ROS link you might want to link to a newer version, like https://github.com/ros-perception/vision_opencv/blob/iron/image_geometry/src/pinhole_camera_model.cpp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the language to use "Supported values..." language.
Hmm interesting point w/ the model. Its a gripe of mine that they don't reference it well anywhere as far as I can tell. I know the "plumb bob" model is in fact a sub-set of the brown-conrady model but poking around it looks like you're right the rational polynomial is a deviation from it that the internet seems confused on exactly who to reference on it... I'll remove the reference and let people look at the sources.
For the ROS link, I intentionally pointed to that particular model because I believe its the one we implement. The updated one you're pointing at is what we should support when have time because it has the equidistant model as well for fisheye.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though none of this matters if I can never get the build system and auto-gen files to be happy lol