search

opencv / opencv_contrib

Repository for OpenCV's extra modules
Apache License 2.0
9.42k stars 5.76k forks source link

Initial guide image in cv::ximgproc::rollingGuidanceFilter() is wrong #1738

Open dai1741 opened 6 years ago

dai1741 commented 6 years ago
System information (version)
Detailed Description

A guide image of rollingGuidanceFilter() is initialized as a copy of a src image, as shown in the following code. https://github.com/opencv/opencv_contrib/blob/5d3928e4f501aca9a9e6bf7791ecdf271f29c20f/modules/ximgproc/src/rolling_guidance_filter.cpp#L49

But the guide image must be initialized as a constant image (please see Algorithm 1 of the original paper).

This weakens the scale-aware property of the filter. So expected results cannot be obtained.

Example

Below is an input image used in Fig. 10 of the original paper. input

Below is the expected result of the filter, generated with the executable in the author's page with command RollingGuidanceFilter.exe -i input.png -sr 10 -ss 40 -iter 5. input png out

Below is the result of cv2.ximgproc.rollingGuidanceFilter(img, d=-1, sigmaColor=10, sigmaSpace=40, numOfIter=5). output

As shown in the above images, small squares in the input image should be removed, but actually none of the squares are removed.

LaurentBerger commented 6 years ago

@dai1741 My pr is ready but may be you can do a Pull Request yourself

dai1741 commented 6 years ago

@LaurentBerger Thank you for fixing the problem! I'm glad if you create a pull request, because I'm new to opencv_contrib and having trouble with running tests.

Please note, that a test set of the filter may also be fixed. A test set TypicalSet2/RollingGuidanceFilterTest_BilateralRef in test_rolling_guidance_filter.cpp checks that rolling guidance filter with one iteration gives similar results to bilateral filter. However, after fixing this issue, rolling guidance filter with one iteration will be equivalent to gaussian filter, not bilateral filter. So I think the reference filter should be replaced by gaussian.

LaurentBerger commented 6 years ago

@dai1741 I'm not sure there is a bug (except for iteration 0). Where do you copy image in figure 10? How do you get image size ?

dai1741 commented 6 years ago

@LaurentBerger There is no more bug in the implementation. But the bug fix causes a testcase of TypicalSet2/RollingGuidanceFilterTest_BilateralRef to fail. So the testcase should also be fixed.

I finally understood how to run tests properly (I forgot preparing the opencv_extra repo yesterday), so I'd like to try my own pull request with the test fix. Is it OK?

dai1741 commented 6 years ago

Where do you copy image in figure 10? How do you get image size ?

I copied the input image from the PDF of the paper. The image size is obtained from the copied image as well. Does that answer your question?

LaurentBerger commented 6 years ago

Finally we don't know real image size : we cannot simulate result given in paper. After reading original code, I changed my pr(https://github.com/LaurentBerger/opencv_contrib/blob/I1738/modules/ximgproc/src/rolling_guidance_filter.cpp#L47-L52) : it is a gaussian filter instead of image with 0 and i add a demo. There is no problem you can make your own PR

dai1741 commented 6 years ago

@LaurentBerger Ah, yes, we don't know the real size. Thank you for clarifying it!

I fixed the implementation and test in commit https://github.com/dai1741/opencv_contrib/commit/75b46f564abe79413bd6a68c998505f3b851c089. The implementation is based on your code (gaussian filter is used) but slightly changed to fix a small problem in numOfIter.

And I cherry-picked your very nice demo (commit https://github.com/dai1741/opencv_contrib/commit/f710dffb4f9a5450854a2dcfbb7134078a6b5257). Can I create pr including the demo? Or should I create pr without it?