Skip to content

Conversation

@protobits
Copy link

For now only "match_input" and "auto_generated" are supported.

@ethzasl-jenkins
Copy link

Can one of the admins verify this patch?

For now only "match_input" and "auto_generated" are supported.
Copy link
Contributor

@ZacharyTaylor ZacharyTaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks like it would work, but I think this approach might result in images that are poorly suited for dense stereo.


private:
bool generateRectificationParameters();
bool generateRectificationParameters(void);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why add void? From memory this style of syntax, while valid only makes a difference in old C code

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no reason, just a habbit. you can discard this change if you want

// in the center of the image, R=I and only x-translation is preserved.
// Resolution is set to the largest area that contains no empty pixels. The
// size of the output can also be modified with the "scale" parameter
// "match_input": intrinsic values are taken from the input camera parameters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is taking the exact input intrinsics desirable for a stereo pair? At the very least I think you will have to ensure that both cameras are rectified using the same focal length or you will probably get pretty poor stereo depth. One possible solution could be to arbitrarily pick one camera and use its intrinsics to set the output for both cameras.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a copy-paste of the documentation of the parameter from the other header. the step which obtains an equal fov from both cameras and so on is still performed. my change mostly sets the resolution the same as the input. otherwise, the current code produces an image which can become much larger (particulary, wider) than the input, due to how the undistortion works. this leads to wasted computation if this bigger image is processed. this is specially important for embedded image processing. moreover, this produces an output similar to what you obtain with opencv and is what most users expect (see #50).

in any case, I've only added an option and the default is the same, so I doesn't hurt.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just went back and checked, your using the function setOutputFromInput, which sets all intrinsic and extrinsic camera parameters to match the input, not just the image size.

The best approach is probably the creation of a new function that just fixes the focal length, updates image size and moves the principle to the center.

@protobits
Copy link
Author

Actually, after some further testing I'm getting a larger epipolar error when I use match_input with this change. So I guess there's something missing for this case. I will continue testing tomorrow.

@protobits
Copy link
Author

Hi,
I tried another approach, I've made a quick modification to setOptimalOutputCameraParameters() which skips the lines that compute an optimal resolution (the rest is the same). This appears to work as expected (resolution is same as input, focal length and principal point are set to their optimal values). What do you think about adding a parameter to this function which allows you to skip these lines? If so, I can close this and make another PR for that.

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

Successfully merging this pull request may close these issues.

3 participants