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

toCvMat breaks SFM disparity output #21

Closed damianomal closed 5 years ago

damianomal commented 5 years ago

While testing current SFM code using an external laptop, I found that there has been a commit which probably breaks the functionalities of the module.

More precisely, the toCvMat function used here

https://github.com/robotology/stereo-vision/blob/23c45483ae03fc630a2ddf5a85c039f85905afc2/modules/SFM/SFM.cpp#L401-L421

causes the disparity map in output to be black.

We tried to visualize it with cv::imshow and the disparity itself is correctly computed and visualized, thus I believe there's something wrong with toCvMat.

The current branch used on iCubGenova01 is ok, we should be careful when updating the machines, before this issue will be fixed.

pattacini commented 5 years ago

Thanks @damianomal for spotting this.

The introduction of toCvMat/fromCvMat together with the need for updating the code to the use of cv::Mat has brought about quite devastating effects 😢

Honestly, I don't know offhand how to fix this. Perhaps, @Nicogene can suggest the correct workaround.

xEnVrE commented 5 years ago

@damianomal

Can you try to change the code to

if (!outputDm.empty()) 
{ 
    ImageOf<PixelMono> &outim = outDisp.prepare(); 
    Mat outimMat;
    if (doBLF) 
    { 
        Mat outputDfiltm; 
        cv_extend::bilateralFilter(outputDm,outputDfiltm, sigmaColorBLF, sigmaSpaceBLF); 
        outimMat = outputDfiltm; 
    } 
    else 
    { 
        outimMat = outputDm; 
    } 
    outim = fromCvMat<PixelMono>(outimMat);
    outDisp.write(); 
} 

?

pattacini commented 5 years ago

Good point @xEnVrE Your snippet could make it work.

Actually, operations like outimMat=outputDm in the first hand don't copy out the whole Mat but rather the header; that's probably why the original code isn't fine.

On the other hand, fromCvMat<> should elicit an entire copy inside 👍

damianomal commented 5 years ago

@xEnVrE's fix works :wink: thank you!

pattacini commented 5 years ago

Awesome! Could you then open up a PR @damianomal ?

Nicogene commented 5 years ago

Good job @xEnVrE , it is the right usage of YARP_cv :medal_military:

image

pattacini commented 5 years ago

Fix pushed to master @damianomal