Implementation questions

[HansBrende]

Question: is this line:

lens-distortion-rectification/groupLineSegments.m

Line 107 in 9a79efe

error1 = min(dist1, dist2);

supposed to be:

error1 = min(abs(dist1), abs(dist2));

as I am assuming the distance calculation here can return signed numbers?

Second question: was this line:

lens-distortion-rectification/groupLineSegments.m

Line 62 in 9a79efe

angDiff = getDifferenceOfAngles(ang1, ang3);

supposed to be:

angDiff = getDifferenceOfAngles(ang2, ang3)

for symmetry with the preceding logic of ang1, ang0, or was ang1 intended here?

Third question: regarding the following two lines:

lens-distortion-rectification/groupLineSegments.m

Line 29 in 9a79efe

errors(seedSeg(1)) = Inf;

and

lens-distortion-rectification/groupLineSegments.m

Line 65 in 9a79efe

errors(indLineSeg3) = Inf;

it seems like one of the following should be true: either (1), the first line is amended to:

errors(seedSeg(1)) = Inf;
errors(seedSeg(2)) = Inf;

OR (2), the second line is amended to:

errors(seedSeg(2)) = Inf;

for consistency (although I suspect the latter case is the correct one, if the intent was to mark all but the last index as Inf). Was one of the above intended?

Fourth question:
The following line appears to disagree with the published paper (equation 3):

lens-distortion-rectification/getDistParamError.m

Line 66 in 9a79efe

for k = j+1:noOfLines

Should that not be:

if j < noOfLines
    k = j + 1

(no loop)?

[bbenligiray]

Hey @HansBrende

Q1: Distance is always positive, so no need for abs there. You can refer to (f) from https://homepages.inf.ed.ac.uk/rbf/CVonline/LOCAL_COPIES/BEARDSLEY/node2.html

Q2: Yes, that's most likely the case.

Q3: Agreed, your second fix looks good. Retrospectively, I would depend on usedLines to avoid using line segments multiple times rather than setting errors to Inf.

Q4: Exactly, I'm surprised that this works so well as is. (On second thought, it's not that surprising, considering that there is an implicit assumption that the chain of line segments all come from the same linear structure on the scene. It's definitely not intended though.)

[HansBrende]

@bbenligiray thanks for the response!

Regarding Q1, I'm not so sure that's the case, as the dot product can be signed. Take for example, the following line segments:

lineSeg1 = (0, 0, 1, 1)
lineSeg2 = (1, 0, 0, 0)

In this case, dist1 as calculated in that code snippet will be signed, with a value of -1/sqrt(2). I believe the "dist" values, as calculated, represent the displacement (under the convention of counter-clockwise unsigned, clockwise signed), not the distance per se.

---

One other somewhat philosophical question, regarding calculating the inverse distortions here: any reason you did it manually instead of using OpenCV's built-in undistortPoints method? Or did you test out cv.undistortPoints and find it insufficient for the use-case?

[bbenligiray]

@HansBrende Looking at the source (pg.22) the actual equation seems to be d² = (l⋅p)² (and I worked through your example and also saw this). So yeah, that's wrong too. I shouldn't have taken a website at face value.

About the second point, I vaguely remember knowing about undistortPoints. However, I probably found it too much of a hassle to bind it to Matlab, and I was probably already working on other code that I could copy-paste over. This is entirely speculation as it has been too long, but the main point is that I doubt that there is anything wrong with using OpenCV here.