This is a correction of the previously missleading documentation and a warning related to a common calibration failure described in issue #15992#15993
Merged
alalek merged 2 commits intoopencv:3.4from Jan 31, 2020
midjji:master
Merged
This is a correction of the previously missleading documentation and a warning related to a common calibration failure described in issue #15992#15993alalek merged 2 commits intoopencv:3.4from midjji:master
alalek merged 2 commits intoopencv:3.4from
midjji:master
Conversation
Contributor
Author
|
This is a documentation warning regarding a bug which has been around for... well since opencv 2.2 atleast. The bug is difficult to fix in general as a more complicated optimization framework is required to add the required constraints to the solvers. (integer programming, polynomial inequalities. ). The problem is common enough that the documentation actually tries to (incorrectly ) explain away the error. But while its trivial to identify the constraints in the case of only optimizing k1,k2,k3, even identifying the constraints when the additional parameters are added is quite difficult. However, because the problem generally goes away when more photoes with the chessboard spread better over the image, a documentation change seems like a good temporary solution. |
Contributor
Author
|
Mistakenly closed, |
Contributor
|
@midjji Please pay attention to CI bot report, the patch contains trailing white spaces: https://pullrequest.opencv.org/buildbot/builders/precommit_docs/builds/22799/steps/whitespace%20opencv/logs/stdio |
Contributor
|
This patch should go into 3.4 branch first. We will merge changes from 3.4 into master regularly (weekly/bi-weekly). So, please:
Note: no needs to re-open PR, apply changes "inplace". |
alalek
reviewed
Dec 15, 2019
Member
alalek
left a comment
There was a problem hiding this comment.
Thank you for contribution.
Please check live results for this patch:
- use proper formatting for formulas
- use
@notefor notes
| Note that (1 + k_1 r^2 + k_2 r^4 + k_3 r^6) is always monotonic for real lenses, | ||
| and if the estimator produces a non monotonic result, this should be considered a calibration failure. | ||
| Note that the result may look deceptively good near the image center but will work poorly in e.g. AR/SFM applications. | ||
| The optimization does not include this constraint as the framework does not support the required integer programming and polynomial inequalities. See issue #15992. |
Member
There was a problem hiding this comment.
See issue #15992
Please use full URL (issue numbers are not handled well in documentation):
Details: https://github.com/opencv/opencv/issues/15992
alalek
reviewed
Dec 15, 2019
Comment on lines
+121
to
+122
| The next figures show two common types of radial distortion: barrel distortion | ||
| (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically decreasing) and pincushion distortion (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically increasing). |
Member
There was a problem hiding this comment.
@catree Could you please take a look on this update? Thank you!
Contributor
There was a problem hiding this comment.
Yes, I think it is correct.
Most likely since k1 is an order of magnitude greater than the other terms, it is "sufficient" (or at least this is what can be read elsewhere) to look at the sign of k1 to determine quickly if the distortion is barrel or pincushion?
Looking at the equations above, is it not if (1 + k1.r^2 + k2.r^4 + k3.r^6)/(1 + k4.r^2 + k5.r^4 + k6.r^6) (ignoring the tangential distortion coefficients) is monotonically decreasing it produces a barrel distortion? Or it is more complicated when taking into account the denominator?
Contributor
Author
|
I'll fix it tomorrow
…On Sun, 15 Dec 2019 at 10:56, Alexander Alekhin ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In modules/calib3d/include/opencv2/calib3d.hpp
<#15993 (comment)>:
> +The next figures show two common types of radial distortion: barrel distortion
+(1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically decreasing) and pincushion distortion (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically increasing).
@catree <https://github.com/catree> Could you please take a look on this
update? Thank you!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15993?email_source=notifications&email_token=ABJQYJPBKGOKUURNELXBTA3QYX5K7A5CNFSM4JRODKM2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPG4VGI#pullrequestreview-332253849>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJQYJKSD5ZG3Q5IIAQRBB3QYX5K7ANCNFSM4JRODKMQ>
.
|
Contributor
Author
|
Yes its the full term which creates either barrel or pincushion which means
monotonic-+
In general all lenses for which this lens model is suitable are bijective
for rays. This must hold even if you add, e.g. tangential distortion etc.
(Not just inside the region of interest, though there may be a case to be
made for that). Opencv supports no lens model which does not strictly
assume(but fails to enforce) bijective for rays. Basically, unless you are
doing classic superresolution and estimating point spread, an astronomer or
your lens datasheet is thicker than the bible, dont worry about it.
Regarding just checking the sign of k1, guess again... Full monotonicity
check is required. But given that your first attempt should set fx==fy,
k2:6 =0, and tangential to zero, its true, and in the second attempt with
k2 != 0, its just (k_1*r^2 + k_2*r^4)'_r (>0, <0) \forall R implies
(2k_1*r + 4k_2*r^3)'_r ... implies (2k_1 + 4k_2*r^2) != 0 for r in
|R^1.
The derivatives are complicated enough everyone will do them wrong for the
full param set, also people will forget the K4:6\vr != 0 issue. So sure, it
would be good to provide them, but the code docs probably shouldnt contain
a full guide. There must be one somewere right, or is the shitty youtube
ones students use actually all there is?
…On Sun, 15 Dec 2019 at 22:17, catree ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In modules/calib3d/include/opencv2/calib3d.hpp
<#15993 (comment)>:
> +The next figures show two common types of radial distortion: barrel distortion
+(1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically decreasing) and pincushion distortion (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically increasing).
Yes, I think it is correct.
Most likely since k1 is an order of magnitude greater than the other
terms, it is "sufficient" (or at least this is what can be read elsewhere)
to look at the sign of k1 to determine quickly if the distortion is
barrel or pincushion?
------------------------------
@midjji <https://github.com/midjji>
Looking at the equations above, is it not if (1 + k1.r^2 + k2.r^4 +
k3.r^6)/(1 + k4.r^2 + k5.r^4 + k6.r^6) (ignoring the tangential
distortion coefficients) is monotonically decreasing it produces a barrel
distortion? Or it is more complicated when taking into account the
denominator?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15993?email_source=notifications&email_token=ABJQYJPMZM43OY4PZPCJAXTQY2NFJA5CNFSM4JRODKM2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPHE6HY#discussion_r358006519>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJQYJKSWNOPGCMI4OWGBFDQY2NFJANCNFSM4JRODKMQ>
.
|
Contributor
Author
|
Actually thinking on it, a very good feature would be automatically
testing all models and providing a few standard model complexity criteriums
along with a model complexity (which parameters are zero or equal another
fx==fy), recommendation. Almost everyone gets confused by the lower errors
the more complex models provides regardless of if the lower complexity
model is more accurate after all.
On Mon, 16 Dec 2019 at 09:09, Mikael Persson <mikael.p.persson@gmail.com>
wrote:
… Yes its the full term which creates either barrel or pincushion which
means monotonic-+
In general all lenses for which this lens model is suitable are bijective
for rays. This must hold even if you add, e.g. tangential distortion etc.
(Not just inside the region of interest, though there may be a case to be
made for that). Opencv supports no lens model which does not strictly
assume(but fails to enforce) bijective for rays. Basically, unless you are
doing classic superresolution and estimating point spread, an astronomer or
your lens datasheet is thicker than the bible, dont worry about it.
Regarding just checking the sign of k1, guess again... Full monotonicity
check is required. But given that your first attempt should set fx==fy,
k2:6 =0, and tangential to zero, its true, and in the second attempt with
k2 != 0, its just (k_1*r^2 + k_2*r^4)'_r (>0, <0) \forall R implies
(2k_1*r + 4k_2*r^3)'_r ... implies (2k_1 + 4k_2*r^2) != 0 for r in
|R^1.
The derivatives are complicated enough everyone will do them wrong for the
full param set, also people will forget the K4:6\vr != 0 issue. So sure, it
would be good to provide them, but the code docs probably shouldnt contain
a full guide. There must be one somewere right, or is the shitty youtube
ones students use actually all there is?
On Sun, 15 Dec 2019 at 22:17, catree ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In modules/calib3d/include/opencv2/calib3d.hpp
> <#15993 (comment)>:
>
> > +The next figures show two common types of radial distortion: barrel distortion
> +(1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically decreasing) and pincushion distortion (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically increasing).
>
> Yes, I think it is correct.
>
> Most likely since k1 is an order of magnitude greater than the other
> terms, it is "sufficient" (or at least this is what can be read elsewhere)
> to look at the sign of k1 to determine quickly if the distortion is
> barrel or pincushion?
> ------------------------------
>
> @midjji <https://github.com/midjji>
>
> Looking at the equations above, is it not if (1 + k1.r^2 + k2.r^4 +
> k3.r^6)/(1 + k4.r^2 + k5.r^4 + k6.r^6) (ignoring the tangential
> distortion coefficients) is monotonically decreasing it produces a barrel
> distortion? Or it is more complicated when taking into account the
> denominator?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#15993?email_source=notifications&email_token=ABJQYJPMZM43OY4PZPCJAXTQY2NFJA5CNFSM4JRODKM2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPHE6HY#discussion_r358006519>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABJQYJKSWNOPGCMI4OWGBFDQY2NFJANCNFSM4JRODKMQ>
> .
>
|
Contributor
|
@midjji Have you had a chance to finish the PR? Rebase to 3.4 and some formatting fixes are still required. |
Contributor
|
@midjji Have you had a chance to finish the PR? |
Contributor
Author
|
Sorry about the delay, I'll take care of it once I'm back in the office.
…On Tue, 14 Jan 2020, 08:44 Vadim Levin, ***@***.***> wrote:
@midjji <https://github.com/midjji> Have you had a chance to finish the
PR?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15993?email_source=notifications&email_token=ABJQYJNVXMA2O74DYDBO2R3Q5V3OXA5CNFSM4JRODKM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEI3Y6EY#issuecomment-574066451>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJQYJIKA2VSEMF3VMLUPYDQ5V3OXANCNFSM4JRODKMQ>
.
|
Contributor
Author
|
Assuming my git foo didnt fail I have fixed the issues and rebased to 3.4 according to instructions. One question, some of this discussion might be a good idea to add to the documentation somewhere. Specifically, tips to get camera calibration right, and how to tell if it went wrong. The existing guides are abysmal. |
Contributor
catree
reviewed
Jan 23, 2020
|
|
||
| The next figures show two common types of radial distortion: barrel distortion (typically \f$ k_1 < 0 \f$) and pincushion distortion (typically \f$ k_1 > 0 \f$). | ||
| The next figures show two common types of radial distortion: barrel distortion | ||
| (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically decreasing) |
Contributor
There was a problem hiding this comment.
Suggested change
| (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically decreasing) | |
| (\f$ 1 + k_1 r^2 + k_2 r^4 + k_3 r^6 \f$ monotonically decreasing) |
catree
reviewed
Jan 23, 2020
| The next figures show two common types of radial distortion: barrel distortion (typically \f$ k_1 < 0 \f$) and pincushion distortion (typically \f$ k_1 > 0 \f$). | ||
| The next figures show two common types of radial distortion: barrel distortion | ||
| (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically decreasing) | ||
| and pincushion distortion (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically increasing). |
Contributor
There was a problem hiding this comment.
Suggested change
| and pincushion distortion (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically increasing). | |
| and pincushion distortion (\f$ 1 + k_1 r^2 + k_2 r^4 + k_3 r^6 \f$ monotonically increasing). |
catree
reviewed
Jan 23, 2020
| A failed estimation result may look deceptively good near the image center | ||
| but will work poorly in e.g. AR/SFM applications. | ||
| The optimization does not include these constraints as the framework does not support | ||
| the required integer programming and polynomial inequalities. See issue #15992. |
Contributor
There was a problem hiding this comment.
Suggested change
| the required integer programming and polynomial inequalities. See issue #15992. | |
| the required integer programming and polynomial inequalities. See [issue 15992](https://github.com/opencv/opencv/issues/15992) for additional information. |
catree
reviewed
Jan 23, 2020
| More generally, radial distortion must be monotonic and the distortion function, must be bijective. | ||
| A failed estimation result may look deceptively good near the image center | ||
| but will work poorly in e.g. AR/SFM applications. | ||
| The optimization does not include these constraints as the framework does not support |
Contributor
There was a problem hiding this comment.
Suggested change
| The optimization does not include these constraints as the framework does not support | |
| The optimization method used in OpenCV camera calibration does not include these constraints as the framework does not support |
Contributor
There was a problem hiding this comment.
Regarding the suggestion, there isn't a opt framework in opencv which
supports it, so technically correct as is. Though it's unimportant. Shall I
fix it?
What do you mean? Do you want to change the framework word in the second part of the sentence?
If you feel that the suggestion is not relevant, feel free to ignore it.
At the end, it is just a minor suggestion.
catree
reviewed
Jan 23, 2020
| The next figures show two common types of radial distortion: barrel distortion | ||
| (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically decreasing) | ||
| and pincushion distortion (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically increasing). | ||
| Radial distortion is always monotonic for real lenses, |
Contributor
There was a problem hiding this comment.
Pedantic: left align the text to be consistent with the surrounding text.
Maybe too many break lines.
Contributor
Author
|
The text isnt not left aligned, it's the space after . Which couldn't be on
the end of the lines according to the previous complaint. Or did I
missunderstand? Newlines don't affect output formatting, but the absence of
a ' ' after a . Does.
Regarding the suggestion, there isn't a opt framework in opencv which
supports it, so technically correct as is. Though it's unimportant. Shall I
fix it?
…On Thu, 23 Jan 2020, 23:30 catree, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In modules/calib3d/include/opencv2/calib3d.hpp
<#15993 (comment)>:
> @@ -118,7 +118,17 @@ v = f_y \times y'' + c_y
tangential distortion coefficients. \f$s_1\f$, \f$s_2\f$, \f$s_3\f$, and \f$s_4\f$, are the thin prism distortion
coefficients. Higher-order coefficients are not considered in OpenCV.
-The next figures show two common types of radial distortion: barrel distortion (typically \f$ k_1 < 0 \f$) and pincushion distortion (typically \f$ k_1 > 0 \f$).
+The next figures show two common types of radial distortion: barrel distortion
+(1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically decreasing)
+and pincushion distortion (1 + k_1 r^2 + k_2 r^4 + k_3 r^6 monotonically increasing).
+ Radial distortion is always monotonic for real lenses,
Pedantic: left align the text to be consistent with the surrounding text.
Maybe too many break lines.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15993?email_source=notifications&email_token=ABJQYJPJSOZ44MXQZJF3WL3Q7IK77A5CNFSM4JRODKM2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCS4MC7Y#pullrequestreview-347652479>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJQYJLHHLQCNR2HJBUIN4LQ7IK77ANCNFSM4JRODKMQ>
.
|
Contributor
|
Yes, it will complain for trailing whitespace.
But there will be no effect for documentation rendering. |
Contributor
|
@midjji your turn |
Contributor
Author
|
Umm I'm not all that good att git, whats the next things I should DO?
…On Wed, 29 Jan 2020, 13:25 Alexander Smorkalov, ***@***.***> wrote:
@midjji <https://github.com/midjji> your turn
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15993?email_source=notifications&email_token=ABJQYJONQKO4JBB4VOY3R53RAFYTDA5CNFSM4JRODKM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKHAJSQ#issuecomment-579732682>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJQYJID4HMF6YEJ6ORDS23RAFYTDANCNFSM4JRODKMQ>
.
|
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pullrequest changes
#15992