opencv / opencv

Open Source Computer Vision Library
https://opencv.org
Apache License 2.0
75.95k stars 55.62k forks source link

Added New Sample: Stereo Camera Calibration in Real Time #3306

Closed upperwal closed 8 years ago

upperwal commented 9 years ago

Captures images in real time by detecting chessboard corners and calibrate the stereo camera after that.

Keyboard Shortcuts:

Initial Mode: Detecting (Chessboard Corners) 'c': Capturing (Start capturing the stereo images) 'p' Calibrate (Once all the images are captured, calibration can start by pressing 'p')

StevenPuttemans commented 9 years ago

:+1: nice tutorial! However afaik you will not be allowed to use any other license than the official OpenCV license.

upperwal commented 9 years ago

Thanks Steven. I was going through opencv coding style guide and found this 'Every source file, except for the samples, starts with BSD-compatible license, which template can be found here.'. So do I use the official BSD licence in the samples as well or not?

Link: http://code.opencv.org/projects/opencv/wiki/Coding_Style_Guide

StevenPuttemans commented 9 years ago

Actually I think that in the near future all files and all samples should point to a single license which can be found here (http://opencv.org/license.html). Not sure on how this is going to be done.

upperwal commented 9 years ago

OK. Got that. Will replace the licence and commit the changes and is it allowed to give a brief description of the sample code and mention the name of the author?

StevenPuttemans commented 9 years ago

Ofcourse that is ! both is encouraged btw !

upperwal commented 9 years ago

Thanks Steven. I updated the sample code with new license.

upperwal commented 9 years ago

Hey! How this PR will be merged in the master branch?

StevenPuttemans commented 9 years ago

Well you need to wait untill the assignee has completely reviewed it and approved, then it will be merged automatically. Keep in mind that all this people do this in their spare time, so it could take a while.

upperwal commented 9 years ago

Ok! Got that! On 11-Oct-2014 6:23 pm, "Steven Puttemans" notifications@github.com wrote:

Well you need to wait untill the assignee has completely reviewed it and approved, then it will be merged automatically. Keep in mind that all this people do this in their spare time, so it could take a while.

— Reply to this email directly or view it on GitHub https://github.com/Itseez/opencv/pull/3306#issuecomment-58748526.

upperwal commented 9 years ago

@vpisarev Merge Please...

StevenPuttemans commented 9 years ago

I guess first thing to do is to merge your commit history. Just do a

 git rebase -i HEAD~9

First replace all entries except the first to squash Then replace to a general commit message And offcourse push back to here your rebased repo

If not, this will never get mergd since it clutters the commit history.

upperwal commented 9 years ago

Thanks Steven :). Merged all my commits. I can understand that my commits could have created a mess.

Cheers

StevenPuttemans commented 9 years ago

:+1:

but it seems you introduced another file .gitignore Can you remove it from your commit. It should not be updated.

upperwal commented 9 years ago

yup... Repo Updated with only one file. Thanks @StevenPuttemans

upperwal commented 9 years ago

@vpisarev Kindly review and approve the code.

StevenPuttemans commented 9 years ago

There was some waiting time due to the release of the 3.0 beta. I guess one of the following days they will have time to check your sample out.

upperwal commented 9 years ago

Thanks @StevenPuttemans. Hoping to get the code reviewed in the coming days.

upperwal commented 9 years ago

@vpisarev should I open a request to push this sample in some other version of opencv and not the master branch?

upperwal commented 9 years ago

hey @vpisarev any update??

upperwal commented 9 years ago

@vpisarev hey! Any update? I have been eagerly waiting for this pr to get merged.

StevenPuttemans commented 9 years ago

@upperwal it seemed that @vpisarev was on holidays until 1 or 2 days ago. He has been back and is graduatly merging pull requests. Just wait a little bit more! I am sure it will get merged since it is a nice addition.

StevenPuttemans commented 9 years ago

Also, please merge both commits, else this will not get accepted!

upperwal commented 9 years ago

Thanks for replying @StevenPuttemans :) and I will definitely merge both the commits asap. :D

vpisarev commented 9 years ago

Hello @upperwal, terribly sorry for long response. I would prefer if this sample will be combined with the offline stereo_calib. We are trying to cleanup OpenCV samples, remove duplicates or close variations, so it would be nice if we have a single calibration sample. This may be difficult, then I'd prefer that at least monocular camera calibration and stereo calibration are 2 samples, not 3 or more.

upperwal commented 9 years ago

Hey @vpisarev, thanks for replying and don't be sorry, I know you guys are very busy. OpenCV samples contain 3 type of calibration files 3calibration.cpp, calibration.cpp and stereo_calib.cpp. Are you suggesting to merge all these samples into one? As my sample is an extension to stereo_calib.cpp (which calibrate using pre saved stereo images, where as my code works on the live video feed) Will it be fine to merge these two samples and let the user decide which method to use?

vpisarev commented 9 years ago

@upperwal, yes, I meant merging your sample and stereo_calib.cpp. 3calibration.cpp can probably be removed and we plan to extend calibration.cpp. So, there is no need to merge everything together, just retaining a single stereo calibration sample would be good enough solution for now.

upperwal commented 9 years ago

@vpisarev :+1: got that. Will try to merge my sample with stereo_calib.cpp and then I will get back to you. Thanks :)

upperwal commented 9 years ago

@vpisarev @StevenPuttemans What from the following is advisable, to load stereo images in the program:

  1. Maintain a images name file (such as images.yml) from which name of an image is read and that string is used in imread().
  2. Ask the user to give a folder, image prefix (Ex: "imageleft", "imageright"), image post fix (Ex: "jpg") and no of images and iterate using image prefix+i+image post fix to read the images. Given image are image_left_1.jpg, image_right_1.jpg etc.

I am sorry if this is a lame question, but I came across the first method in almost all samples and according to me the second one is more efficient to do. (user does not need to create a file for the name of the images). Please correct me, if I am wrong. Thanks

StevenPuttemans commented 9 years ago

@upperwal it kind of depends I guess. Since you are building the sample, you can choose the method you like. The reason why people use a name file with locations, is because reading files is universal in different OS. Reading from a folder gives different sequences since ordering is different on several OS.

But hey, you choose what fits you the best!

upperwal commented 9 years ago

Thanks @StevenPuttemans :+1: I used the second method for the time being. To have a single sample for stereo calibration I have merged stereo_calib with my sample. The sample can now work on already saved stereo images or click the images on the go.

StevenPuttemans commented 9 years ago

Great :+1:

StevenPuttemans commented 9 years ago

Could you also use your PR to delete the double samples?

upperwal commented 9 years ago

@StevenPuttemans As stereo_calib is updated, there are two calibration samples left. 1. "3calibration.cpp" and 2. "calibration.cpp". So "3calibration.cpp" should be deleted as suggested by @vpisarev as "calibration.cpp" is used for single camera calibration. Correct?

upperwal commented 9 years ago

@vpisarev hi. Can you please review the code?

StevenPuttemans commented 9 years ago

@upperwal I know it is taking some time, but please be patient, I have the idea that some of the devs are on holidays or taking a break :)

upperwal commented 9 years ago

:+1: @StevenPuttemans

upperwal commented 9 years ago

Thanks @StevenPuttemans. You are a great support :+1: :)

askerpro commented 8 years ago

got an error with imread cv::imwrite could not find a writer for the specified extension //std::ostringstream leftString, rightString; //leftString << dir << "/" << prefixLeft << pairIndex << postfix; //rightString << dir << "/" << prefixRight << pairIndex << postfix; std::string leftString = dir + "/" + prefixLeft + to_string(pairIndex) + "." + postfix; std::string rightString = dir + "/" + prefixRight + to_string(pairIndex) + "." + postfix; imwrite(leftString, leftImage); imwrite(rightString, rightImage); works with this

upperwal commented 8 years ago

@askerpro :+1: will fix it ASAP. Thanks :)

StevenPuttemans commented 8 years ago

@upperwal I do not think you should close it down. Just update this branch?

upperwal commented 8 years ago

Hey @StevenPuttemans This PR already have 9 commits, I was thinking of creating a new PR after resolving the merge conflict and incorporating the above suggestion. Will that be fine?

StevenPuttemans commented 8 years ago

@upperwal there is a very nice tool in git called git rebase -i HEAD~9 (change number accordingly) which allows you to concatenate commits and make it a clean PR again. No needc in making a new branch for the fixes!

upperwal commented 8 years ago

@StevenPuttemans Very well then. Let me try it and reopen this PR. Thanks Steven. :)

upperwal commented 8 years ago

Sorry @StevenPuttemans I tried using rebase but it was all messed up and then I did a force push to my repo. I think, now I need to create a new PR for this.

StevenPuttemans commented 8 years ago

Ok no problem at least you tried :D

upperwal commented 8 years ago

:) Thanks @StevenPuttemans