new algorithm Rapid Frequency Selective Reconstruction (FSR) added #2296
Conversation
There was a problem hiding this comment.
Thank you for great contribution!
May I ask you to add some simple test:
- code example
- lena.png image is available in opencv_extra repository:
cvtest::findDataFile("cv/shared/lena.png") - modify input image in test code (add mask)
- compare result with original unmodified image through PSNR metric.
modules/xphoto/src/inpainting.cpp
Outdated
|
|
||
|
|
||
| static void | ||
| icvBGR2YCbCr(const cv::Mat& bgr, cv::Mat& Y, cv::Mat& Cb, cv::Mat& Cr) |
There was a problem hiding this comment.
Please use cvtColor() + cv::split() instead of manual function.
There was a problem hiding this comment.
Thanks a lot for your fast review!
I added a simple test with existing test data following your advice and tried to take all your comments into account.
I changed that and use cvtColor() now.
The manual function was used to achieve the same value range for the YCbCr color space as in Matlab for a better comparison (the algorithm was ported from Matlab to C++ and the matlab color conversion covers a slighlty different value range as the result of cvtColor()).
So, the implementation might give slightly different results than those stated in the paper. I made a quick check for the Kodak image set, the resulting mean PSNR value differs by 0.04 dB.
modules/xphoto/src/inpainting.cpp
Outdated
| { | ||
| for (int x = 0; x < src.cols; ++x) | ||
| { | ||
| double curr_val = srcData[y*src.step1() + x]; |
There was a problem hiding this comment.
Please avoid manual calculation through steps:
CV_CheckTypeEQ(CV_64F, src.type(), "");
for (y...)
{
double curr_row = src.ptr<double>(y);
for (x...)
{
double curr_val = curr_row[x];
...
}
}
There was a problem hiding this comment.
I used src.at<double>(y,x) instead of the manual calculation. I hope that is fine as well.
modules/xphoto/src/inpainting.cpp
Outdated
| } | ||
| cv::Scalar tmp_stddev, tmp_mean; | ||
| cv::Mat mask8u = error_mask_2d*2; | ||
| mask8u.convertTo(mask8u, CV_8U); |
There was a problem hiding this comment.
*2
Why 2? What is values range?
mask8u.convertTo(mask8u, CV_8U);
There is third parameter which can help with multiplication.
BTW, Perhaps inplace mode doesn't work well here.
There was a problem hiding this comment.
error_mask_2d can have the values 0 (pixels that need to be reconstructed), 0.5 (already reconstructed) or 1 (valid, original pixels).
By multiplication with 2, the pixels with the value 0.5 won't be 0 after the conversion to CV_8U. With that, the already reconstructed pixels are used for the computation of the standard deviation.
Thanks for the remarks.
modules/xphoto/src/inpainting.cpp
Outdated
| // inital scan of distorted blocks | ||
| std::vector< std::tuple< int, int > > set_todo; | ||
| int blocks_column = cvCeil(static_cast<double>(img_height) / block_size); | ||
| int blocks_line = cvCeil(static_cast<double>(img_width) / block_size); |
There was a problem hiding this comment.
We can avoid floating-point computations here.
Use int divUp(int a, unsigned int b) from core/utility.hpp
modules/xphoto/src/inpainting.cpp
Outdated
| CV_Error(cv::Error::StsUnsupportedFormat, "Unsupported source image format!"); | ||
| break; | ||
| } | ||
| src.convertTo(src, CV_8UC3, 1 / 257.0); |
There was a problem hiding this comment.
BTW, .convertTo() does not care about number of channels. CV_8UC3 can be safely replaced by CV_8UC1, or CV_8U. Also this case code block can be merged with case CV_16UC1: (similar for others).
modules/xphoto/src/inpainting.cpp
Outdated
| { | ||
| for (int x = 0; x < width; ++x) | ||
| { | ||
| recData[y*dst.step1() + x] = cv::saturate_cast<uchar>(yData[y*y_reconstructed.step1() + x]); |
There was a problem hiding this comment.
Thank you for update!
I updated testing code:
- full test is very long - ~1min in release mode, ~8 min in debug
- separate tests for modes (full-sized "best" is disabled by default, can be tested through "--test_tag_enable=verylong,debug_verylong" parameter)
- added grayscale test with "reduced" input
| original.copyTo(im_distorted, mask_valid); | ||
|
|
||
| Mat reconstructed; | ||
| cv::xphoto::inpaint(im_distorted, mask_valid * (1/255.0), reconstructed, mode); |
There was a problem hiding this comment.
mask_valid * (1/255.0)
Looks like masks {0, 1} are accepted only. {0, 255} doesn't work good.
Please add mask pre-processing code into algorithm implementation, like documentation says:
@param mask mask (CV_8UC1), where non-zero pixels indicate valid image area, while zero pixels
indicate area to be inpainted indicate area to be inpainted
Try this pre-processing code:
threshold(mask_as_inputarray, mask_01, 0.0, 1.0, THRESH_BINARY)l
(after that please remove this fix from test code)
There was a problem hiding this comment.
Thank you for your work on the test code!
I fixed the mask pre-processing accordingly and removed your fix from the test code.
|
There is problem with sporadic tests failures:
Good builds from the same commit:
Both Perhaps some variable/matrix is not initialized. |
|
Vagrind catches errors: |
|
Thanks for checking. Maybe you have an idea on how to interpret this? |
|
You need "Debug" build (or add "-g1" C++ compiler option, "-O1" is also preferable instead of "-O2" or "-O3"). It blames on this line: I pushed fix here. Update: valgrind run is clean with fix |
Thanks for the info, I wasn't aware of these options and looked at the wrong lines. |
Submission
This pullrequest adds the implementation of the high-quality extrapolation algorithm Rapid Frequency Selective Reconstruction (FSR) with two quality profiles. As one of its fields of applications is inpainting, it was added to the existing inpainting interface in xphoto.
Please refer to
https://github.com/opencv/opencv_contrib/files/3730212/inpainting_comparison.pdf
for a detailed description of the algorithm and a comparison to existing inpainting methods in OpenCV.