BriefDescriptorExtractor does not support monochannel 16 bit images.

[Cazadorro]

System information (version)

• OpenCV => 4.5
• Operating System / Platform => ALL
• Compiler => ALL

Detailed description

Currently xfeatures2D brief descriptor does not support single channel 16bit brief descriptors. I recently was put on a project where the solution people had was to literally copy the entire codebase inside xfeatures2D and just change breif.cpp to a 16 bit version. The biggest changes were really to the summation part, which doesn't work with 16 bit integers given it sums with 32S numbers. If every value was filled to the max for a 16 bit number, only 256x256 images would be guaranteed to successfully sum. The summation was replaced with double precision for this reason.

16bit grayscale images are actually pretty common, and down scaling to 8bit is not an option. The brief code that sits in the repo today just doesn't work with monochannel 16bit data.

An additional problem with the code was the integral function:

opencv_contrib/modules/xfeatures2d/src/brief.cpp

Line 246 in 0def473

integral( grayImage, sum, CV_32S);

The performance compared to the rest of our system (which was GPU accelerated) was so bad that it ended up taking up to 50% of the runtime of our application.

To fix this, and because we had access to a standalone copy of brief, we were able to just replace the code with cv::cuda::sum(...) which eliminated the problem.

This looked like this:

cv::Scalar sum; 
if(image.isGpuMat()){
    sum = cv::cuda::sum(grayImage);
}else{
    sum = cv::sum(grayImage);
}
...

The issue is that I don't really know if you can test for GPU mat in all OpenCV 4.x installations, even those with out cuda support. But regardless the only reason the code even needs the image itself is for this integral term.

Even the code itself seems to admit that it would be better to pass in this integral term separately:

opencv_contrib/modules/xfeatures2d/src/brief.cpp

Lines 241 to 244 in 0def473

	///TODO allow the user to pass in a precomputed integral image
	//if(image.type() == CV_32S)
	// sum = image;
	//else

I don't know if I have the expertise to actually submit a pull request, but at least can give some notion of what I believe the appropriate solution might look like.

I think there should be an overload to brief.cpp's compute function

opencv_contrib/modules/xfeatures2d/src/brief.cpp

Line 231 in 0def473

void BriefDescriptorExtractorImpl::compute(InputArray image,

Some how we need a signature that uses cv::Scalar sum or something, but also needs to send image size, something like this:

void BriefDescriptorExtractorImpl::compute(Size imageSize,
                                                                      Scalar sum,
                                                                      std::vector<KeyPoint>& keypoints,
                                                                      OutputArray descriptors)

The complete definition would then be:

void BriefDescriptorExtractorImpl::compute(Size imageSize,
                                                                      Scalar sum,
                                                                      std::vector<KeyPoint>& keypoints,
                                                                      OutputArray descriptors)
{
    // Construct integral image for fast smoothing (box filter)

    //Remove keypoints very close to the border
    KeyPointsFilter::runByImageBorder(keypoints, imageSize, PATCH_SIZE/2 + KERNEL_SIZE/2);

    descriptors.create((int)keypoints.size(), bytes_, CV_8U);
    descriptors.setTo(Scalar::all(0));
    test_fn_(sum, keypoints, descriptors, use_orientation_);
}

And with such a definition, our code base could use cuda to compute the sum, and the code would become agnostic to the actual type of the image data, removing the need for a breif16 class.

I believe if the inheritance chain supported this, it would work out of the box with no other code changes. However I don't really understand how the class hierarchy and indirection present in the brief class, I don't believe I can just "do" this? and have it work, since create is overwriting Featur2D compute() functions

https://github.com/opencv/opencv/blob/2a2e4e78bfdd49bc241c8a2f83d16aefb540766c/modules/features2d/include/opencv2/features2d.hpp#L144-L230

I'm not sure, for other types of features, if using the image is actually necessary in such a way we wouldn't want to not have said function, or that the sum+image size overload of compute are not really supported on other feature types besides BriefDescriptorExtractor

Maybe it would be possible to simply add a single function to the BriefDescriptorExtractor class which uses this impl overload?

With this change, I believe I could eliminate this copied code base entirely and use normal opencv again.