Implement cv::cuda::inRange (Fixes OpenCV #6295) by amiller27 · Pull Request #2803 · opencv/opencv_contrib

amiller27 · 2021-01-02T20:04:20Z

Merge with extra: opencv/opencv_extra#834

Implementation of cv::cuda::inRange, as requested in this issue. It's not a complete implementation of cv::inRange; my implementation supports cv::Scalar for the upper and lower bounds, which seems like the most common use case by far, but the CPU version does also support Mats for the bounds as well, which mine does not. It seemed like more of a challenge to support that as well, but if you don't want to merge this without full feature parity I might be able to spend more time on it to figure that out.

Some questions -

Wasn't sure about whether the base branch should be master or 3.4, happy to rebase to 3.4 if desired. I do use std::array, but I think that's the only C++11 feature I'm using and it'd be easily removed.
Wasn't sure if I need to do anything special for performance test data - I followed the instructions here to update the values for my new test only (PR in opencv_extra), but I didn't know how your performance test system deals with the fact that this should perform very differently depending on the GPU (I tested on a GeForce 750M).
I added Doxygen comments in the code; if there's more documentation or sample code needed I can do that as well
I had to use some recursive templates in functional.hpp to copy and compare CUDA vectors; if CUDA or OpenCV already has utilities somewhere to do this I can switch to make that simpler

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
The PR is proposed to proper branch - see comment below
There is reference to original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake - see comment below

force_builders=Custom
buildworker:Custom=linux-5
build_image:Custom=ubuntu-cuda:18.04

alalek

Thank you for contribution!

Please take a look on comments below.

modules/cudaarithm/src/cuda/in_range.cu

alalek · 2021-01-05T11:03:56Z

modules/cudaarithm/src/cuda/in_range.cu

+                                  inRangeImpl<double, 4>}};
+
+    const func_t func = funcs.at(channels - 1).at(depth);
+    func(src, lb, ub, dst, stream);


Please use CV_Check*() macro to validate channels / depth values.

src.depth()

where is validated?

Previously wasn't; I was trying to cover all depths. I've now realized that it doesn't work for float16, because CUDA doesn't have a float16 vector type. So I added a CV_Check that the depth is CV_64F or lower

alalek · 2021-01-05T11:08:00Z

modules/cudaarithm/include/opencv2/cudaarithm.hpp

+@param dst output array of the same size as src and CV_8U type.
+@param stream Stream for the asynchronous version.
+
+@sa inRange


There is self-reference loop.

Try this one instead:
@sa cv::inRange

alalek · 2021-01-05T11:10:36Z

modules/cudaarithm/include/opencv2/cudaarithm.hpp

+
+@sa inRange
+ */
+CV_EXPORTS_W void inRange(InputArray src, InputArray lowerb, InputArray upperb, OutputArray dst, Stream& stream = Stream::Null());


CV_EXPORTS_W

Did you try to call this from Python binding? Are lowerb / upperb handled properly?

I can make calls like

src = (np.random.random((1920, 1080, 4)) * 256).astype(np.uint8) lowerb = (0, 0, 0, 0) upperb = (255, 255, 255, 255) dst = cv2.cuda.inRange(src, lowerb, upperb)

with different types and numbers of channels for src (up to 4 channels). The CV_Check fires correctly if I pass the wrong shape for lowerb or upperb, or too many channels for src. It doesn't work if I pass a cv2.cuda_GpuMat (I get a TypeError: Expected Ptr<cv::UMat> for argument 'src') - I'm really not familiar with using OpenCV CUDA from Python so I don't know if that's the intended behavior.

I get a TypeError

Current messages from Python bindings are quite useless. You can set OPENCV_PYTHON_DEBUG=1 env variable to investigate conversion errors.
The "GPU" overload assumes that all inputs must be a GpuMat including lowerb/upperb parameters (because they a InputArray). But wrapping 4 scalars into GpuMat has significant overhead.

InputArray lowerb, InputArray upperb

Try to replace type to const Scalar& to eliminate unnecessary complexity.

Please add simple Python test into this file.

(please note, that public OpenCV CI doesn't run CUDA code/tests)

Done. I switched to const Scalar& for lowerb and upperb, and added a Python test to that file which passes on my machine, it now works if you pass a numpy array or a cuda_GpuMat

amiller27 · 2021-01-19T04:34:42Z

I think everything has been addressed, but the docs build is failing on CI with

From git://code.ocv/opencv/opencv-ci
 * branch            master     -> FETCH_HEAD
HEAD is now at 5258c2f CMAKE_CXX_FLAGS: removed duplicate -Winit-self
FATAL: Build image for 'docs--18.04' is missing on the current build worker
program finished with exit code 1

It passes on my machine and this seems unrelated to anything I've done, not sure if there's anything I can do to fix it

alalek · 2021-01-19T05:41:45Z

@amiller27 It is CI issue, I will fix that soon.

Custom (CUDA) builder is failed due to changes from #2807 (should be fixed soon in separate PR)

alalek

Well done! Thank you for contribution 👍

amiller27 mentioned this pull request Jan 2, 2021

Add performance data for cv::cuda::inRange opencv/opencv_extra#834

Merged

amiller27 force-pushed the inrange branch from 24d8013 to ae5a4ec Compare January 2, 2021 20:15

alalek reviewed Jan 5, 2021

View reviewed changes

amiller27 force-pushed the inrange branch 2 times, most recently from 4213ba3 to a102a0a Compare January 8, 2021 06:39

amiller27 force-pushed the inrange branch from a102a0a to c290cb1 Compare January 18, 2021 07:00

Implement cv::cuda::inRange (Fixes OpenCV #6295)

f1c0b5e

amiller27 force-pushed the inrange branch from c290cb1 to f1c0b5e Compare January 19, 2021 02:50

alalek approved these changes Jan 21, 2021

View reviewed changes

opencv-pushbot merged commit 59a9c88 into opencv:master Jan 21, 2021

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #2919

Merged

Conversation

amiller27 commented Jan 2, 2021 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amiller27 Jan 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amiller27 commented Jan 19, 2021

Uh oh!

alalek commented Jan 19, 2021

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

amiller27 commented Jan 2, 2021 •

edited by alalek

Loading

amiller27 Jan 18, 2021 •

edited

Loading