-
Notifications
You must be signed in to change notification settings - Fork 48
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
feat(ceres_intrinsic_camera_calibrator): add FOV regularization #228
base: tier4/universe
Are you sure you want to change the base?
feat(ceres_intrinsic_camera_calibrator): add FOV regularization #228
Conversation
Signed-off-by: Amadeusz Szymko <[email protected]>
…ortion and distortion Signed-off-by: Amadeusz Szymko <[email protected]>
Signed-off-by: Amadeusz Szymko <[email protected]>
@@ -0,0 +1,356 @@ | |||
// Copyright 2024 Tier IV, Inc. |
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.
2025 !
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.
Right! Addressed in 43ef5b7
Signed-off-by: Amadeusz Szymko <[email protected]>
const std::vector<T> shifts = {T(0.01), T(0.03), T(0.05), T(0.1), | ||
T(0.3), T(0.5), T(1.0), T(3.0)}; | ||
|
||
const T width_t = static_cast<T>(width_); |
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.
It seems you are mixing T()
and static_cast<T>()
In the past for some reason I though static cast would not work for some internal reason to ceres, but it seems I was wrong. Can you choose one style?
T xp = (u - cx) / fx; | ||
T yp = (v - cy) / fy; | ||
|
||
for (int i = 0; i < UNDIST_ITERS; i++) { |
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.
Just a comment for now (will check during the review)
Using a high number of iterations (I know you consider early stopping, but the threshold seems strict) will result in a very long chain rule (potential gradient issues)
intrinsics_placeholder_.data()); | ||
} | ||
|
||
if (fov_regularization_weight_ > 0.0) { |
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.
Can you remind me (to do the proper experiments), which coefficient, it set to a positive value, no matter how small, resulted in dominating the cost function?
#include <opencv2/calib3d.hpp> | ||
#include <opencv2/core.hpp> | ||
|
||
#include <Eigen/src/Core/Matrix.h> |
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.
Is this include Ok? seems quite weird
* Calculates radial distortion | ||
* | ||
* Approximation is applied if any rational distortion coefficient is negative. This approximation | ||
* follows Taylor series expansion of a function around x0=0, also known as a Maclaurin series. |
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.
How the approximation is explained, but not the why
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.
@amadeuszsz
Watching the code this time (instead of verbal or whiteboard explanations), I (think I) can see why the optimization got unstable,etc. There are many computations that would make the chain rule a complex one.
To summarize the motivation and proposed approach:
- We "can" calibrate the camera without this PR, obtaining good reprojection error,
- Problem is, points that should be projected outside the image are projected inside.
- However, k and d are valid for points that should be inside the image (i.e., reprojection error is good).
This would allow us to unproject the borders of the image, which you do with imageToCamera
, with z=1, which should be fixed
from here on.
The implicance: "points outside the fov of the image causes non zero fov residuals" should be true
Re calibrating, this time with the fov residual, should make the residual zero (the camera points with their shifts are constants during this optimization).
In summary, what I think should be performed is:
- Calibrate without this residual
- If the fov residual weight is more than zero (or if we detect an anomaly):
- Compute the camera points with your method (make them constants)
- Optimize with your residual. No approximations should be needed. No iterative algorithm should be needed inside the residual
- After calibration, check a sanity check to see if the model is "valid"
Description
This PR adds new residual block for Ceres optimizer. The block flow is:
u
,v
) using current camera intrinsic and transform resultant coordinate from image frame to camera frame (x
,y
).x
,y
) from camera frame to image frame and distort resultant pixel (u
,v
).x
,y
) toward "out of FOV" direction.u_shifted
,v_shifted
) is placed within the input image dimensions (camera FOV), apply penalization term.There are few (8) predefined shifts and few (8) input pixels - 4 image corners and 4 middle of image borders. During image projection & back-projection and fact that during optimization coefficient not necessary converged, numerical instability may occur. It happens for rational model when polynomial has real roots. Using trivial assumption (if any rational coefficient is negative), we applies polynomial approximation which fully cover this issue.
Related links
Tests performed
Notes for reviewers
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.