robotology / stereo-vision

Repository containing apps for stereo vision
https://robotology.github.io/stereo-vision/
GNU General Public License v2.0
19 stars 19 forks source link

Now SFM should not exit anymore if LIBELAS cannot compute a valid disparity map for a frame couple. #8

Closed GiuliaP closed 8 years ago

GiuliaP commented 8 years ago

With this PR, the library LIBELAS does not exit anymore if, for a frame couple, it's not possible to compute the disparity map (because there are not enough points to compute the Delaunay triangulation). The expected behavior is instead the following:

I checked that in all the parts of the code that I found there is always a check on stereoCamera::Disparity and stereoCamera::Disparity16 before using them (as it should be). The other two options that I had in mind were either leaving the two matrices to the previous value or set them to all zeros (both of them are possible in case you prefer), but I thought that this method is the best because the user can realize that indeed the disparity is not being computed at all. I tried this SFM on the robot and it worked fine when the robot wasn't seeing anything, whereas the current SFM was exiting with the mentioned error message. Finally, I also checked other possible parts of the code in LIBELAS that might cause similar exit behaviors and I concluded that -as far as I know- this was the only 'bug' that needed to be faced in the perspective of using the disparity in a streaming pipeline. Indeed other reasons could be (i) failures in memory allocation or (ii) errors which the same authors of the code would like to be informed because, if they happen, they are considered bugs of their algorithms. Let me know if further issues arise.

Tobias-Fischer commented 8 years ago

Great, thanks very much for this! Looks all fine to me, except of the small comment above, as well as that the line indentation seems to be messed up with tabs/spaces. I'll give it a try on our robot in the next days.

Thanks! Tobias

pattacini commented 8 years ago

yep, @GiuliaP could you fix up the indentation? It's not just a stylistic comment in the end :smirk:

Tobias-Fischer commented 8 years ago

Hi @GiuliaP, as above, please ignore my first comment regarding the order of the loops and undistortPoints .. all good!

Regarding the indentation: I think the policy is having 4 spaces per indent rather than tabs as in your last commit.

Thanks again! Tobias

GiuliaP commented 8 years ago

Thanks @Tobias-Fischer, I should have fixed my indentation settings to use spaces only instead of tabs. I'm sorry for the inconvenience.

Tobias-Fischer commented 8 years ago

Looking good to me. Thanks!

pattacini commented 8 years ago

Great then, shall we merge this PR?

Tobias-Fischer commented 8 years ago

@pattacini Done. Thanks again @GiuliaP! I think this was a really important change so the libelas can be used reliably on the robots for the stereo matching.

Reminder @maxime-petit: Let's update our robot early next week and try the improved stereo!

pattacini commented 8 years ago

:+1:

vtikha commented 8 years ago

Excellent! @Tobias-Fischer let us know how things proceed

Tobias-Fischer commented 8 years ago

Hi @vtikha, @GiuliaP and @pattacini,

@maxime-petit gave this a try today and it seems to work all fine :)! Thanks @GiuliaP.

Best, Tobi

pattacini commented 8 years ago

Cool :+1:

vtikha commented 8 years ago

Excellent!