Skip to content
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

Fixed calibration save, and two matrix issues within the multiple undistort variants. #178

Closed
wants to merge 0 commits into from

Conversation

xionluhnis
Copy link

Fixed a few things related to issues 175 and 176.

  1. Changed the calibration save method to correctly export the vector of vector of points (otherwise it crashes when we try to reload it).
  2. Changed the undistort functions to correctly handle the output parameter (by passing it by reference instead of by value, for edge case where it's currently empty) and using the undistorted camera matrix as newMatrix when undistorting points.

@xionluhnis
Copy link
Author

Sorry for the double pull request, I messed up some of the logs and removed some changes that were there for debug. I cleaned the history so it only looks like one commit (no extra commits).

Also, I removed the #ifdef USING_OPENCV_2_3 guard because I am personally using circle targets (with opencv 2.4.8), and I don't really see the point of not having the rest available anyway since if it worked before, given that it works now, let's not remove that part of the code.

@xionluhnis
Copy link
Author

Since many people are asking for the solution to #175, do you want me to redo the PR with only that fixed?
I assume you did not merge yet because I fixed more than that and the rest (#176 as well as USING_OPENCV_2_3) are not accepted as issues. Plus it needs to be rebased to the latest master.
I can do a new PR if that's what you want.

@xionluhnis
Copy link
Author

I rewrote things on my fork, sorry if that messes up other people, but it will be cleaner for merging the pressing #175 issue.

The old stuff is now on my Issue175+176 branch.

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.

1 participant