cudaoptflow: disable Optical Flow SDK when CUDA version is insufficient #2824

tomoaki0705 · 2021-01-18T12:52:55Z

relates #2807

Just modifying this line will let the build pass for both CUDA 8.0 and CUDA 10.0

/cc @vchiluka5
I understand that Optical flow API only supports CUDA 10.0 and later. Thank you for the comment.
Though, I'd like to propose this fix as a quick fix of build.

Eventually, I can add to disable Optical Flow API when CUDA is less than 10.0

Before

[ 81%] Building NVCC (Device) object modules/cudaoptflow/CMakeFiles/cuda_compile_1.dir/src/cuda/cuda_compile_1_generated_nvidiaOpticalFlow.cu.o
/opencv_contrib/modules/cudaoptflow/src/cuda/nvidiaOpticalFlow.cu(76): error: no instance of overloaded function "surf2Dwrite" matches the argument list
            argument types are: (short2, cudaSurfaceObject_t, unsigned long, int, cudaSurfaceBoundaryMode)

1 error detected in the compilation of "/tmp/tmpxft_00007a2a_00000000-7_nvidiaOpticalFlow.cpp1.ii".
CMake Error at cuda_compile_1_generated_nvidiaOpticalFlow.cu.o.Release.cmake:279 (message):
  Error generating file
  /opencv/build/modules/cudaoptflow/CMakeFiles/cuda_compile_1.dir/src/cuda/./cuda_compile_1_generated_nvidiaOpticalFlow.cu.o

After

[ 82%] Building NVCC (Device) object modules/cudaoptflow/CMakeFiles/cuda_compile_1.dir/src/cuda/cuda_compile_1_generated_nvidiaOpticalFlow.cu.o
 : 
[ 86%] Built target opencv_cudaoptflow

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
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

force_builders=Custom
buildworker:Custom=linux-4
build_image:Custom=ubuntu-cuda:16.04

vchiluka5 · 2021-01-18T13:49:10Z

Sure @tomoaki0705.
Thanks for the quick fix. I'll need to verify this fix with Optical flow outputs.
Il get back once i verify and let you know if this is a valid fix.
Please DO NOT SUBMIT till then. ETA 1/2 days

vchiluka5 · 2021-01-19T15:03:48Z

Sure @tomoaki0705.
Thanks for the quick fix. I'll need to verify this fix with Optical flow outputs.
Il get back once i verify and let you know if this is a valid fix.
Please DO NOT SUBMIT till then. ETA 1/2 days

Hi @tomoaki0705
The change you suggested breaks the code. Optical flow vector outputs are empty with your change.
surf2dWrite requires to be called with short2 type so that proper memory allocation is done and final flow vectors are generated.

Please DO NOT SUBMIT this change instead
you can upgrade to cuda 10 FROM CUDA 8
OR
Add the execption to not include NVIDIA_OPTFLOW because anyway code wont run with CUDA 8.

Thanks,
Vishal

alalek · 2021-01-19T19:22:45Z

only supports CUDA 10.0 and later

@vchiluka5 Perhaps this should be handled in CMake scripts through CUDA SDK version check:

disable v2.0 for CUDA<10.0
or fallback on previous v1.0

tomoaki0705 · 2021-01-21T12:46:21Z

Thank you @alalek @vchiluka5
I understand that this PR is helpless, so I want to shift the point of this PR to @alalek point

only supports CUDA 10.0 and later

@vchiluka5 Perhaps this should be handled in CMake scripts through CUDA SDK version check:
* disable v2.0 for CUDA<10.0

* or fallback on previous v1.0

I'd like to propose to fallback to v1.0 in master branch (4.5 series)
To migrate totally to NVIDIA Optical Flow SDK v2.0, this should happen in next branch (5.0 series), at least in my opinion.
For falling back to v1.0, I'm working on this on another branch.

@vchiluka5 , what I'd like to double check is that is Optical Flow SDK v1.0 totally deprecated ?
Does falling back to v1.0 makes sense to you ?

vchiluka5 · 2021-01-21T13:05:08Z

Thank you @alalek @vchiluka5
I understand that this PR is helpless, so I want to shift the point of this PR to @alalek point
only supports CUDA 10.0 and later

@vchiluka5 Perhaps this should be handled in CMake scripts through CUDA SDK version check:
* disable v2.0 for CUDA<10.0

* or fallback on previous v1.0
I'd like to propose to fallback to v1.0 in master branch (4.5 series)
To migrate totally to NVIDIA Optical Flow SDK v2.0, this should happen in next branch (5.0 series), at least in my opinion.
For falling back to v1.0, I'm working on this on another branch.

@vchiluka5 , what I'd like to double check is that is Optical Flow SDK v1.0 totally deprecated ?
Does falling back to v1.0 makes sense to you ?

il suggest to disable the v2.0 for CUDA < 10.0. Because CUDA 8 doesnt have proper compatibility with turing architecture and optical flow sdk 1.0 works from turing architecture and above.
Optical flow sdk 1.0 is NOT deprecated. Users can use both sdk's but not with CUDA 8. So better we disable both 2.0 and 1.0 SDK if its CUDA <10.
@tomoaki0705 if you can work on those changes then Great since i am not aware about cmake changes.
Otherwise @alalek please guide me on making those changes

tomoaki0705 · 2021-01-21T13:17:07Z

Thank you @vchiluka5 !
Your explanation makes things clear.
For CUDA less than 10.0, it makes sense totally disabling optical flow sdk.
Let me work on that cmake part.

vchiluka5 · 2021-01-21T13:23:10Z

Thank you @vchiluka5 !
Your explanation makes things clear.
For CUDA less than 10.0, it makes sense totally disabling optical flow sdk.
Let me work on that cmake part.

Thanks @tomoaki0705 . That will be of great help :-)

alalek

Expected changes in CMake (looks good) and src/nvidiaOpticalFlow.cpp (just add guard for HAVE_NVIDIA_OPTFLOW, no code removal is expected)

alalek · 2021-01-23T01:40:16Z

modules/cudaoptflow/samples/nvidia_optical_flow.cpp

@@ -198,6 +198,7 @@ bool parseROI(std::string ROIFileName, std::vector<Rect>& roiData)

 int main(int argc, char **argv)
 {
+#if defined HAVE_NVIDIA_OPTFLOW


Samples should not depend on such defines.
API calls will throw an exception.

Please revert compilation guards from samples.

Removed. thank you

alalek · 2021-01-23T01:41:28Z

modules/cudaoptflow/perf/perf_optflow.cpp

@@ -326,56 +326,7 @@ PERF_TEST_P(ImagePair, OpticalFlowDual_TVL1,
    }
 }

-//////////////////////////////////////////////////////
-// NvidiaOpticalFlow_1_0


cv::cuda::NvidiaOpticalFlow_1_0 is not removed from OpenCV public API, it is just disabled (throws exceptions).

Please restore this test.

Restored. Thank you

alalek · 2021-01-23T01:45:03Z

modules/cudaoptflow/src/nvidiaOpticalFlow.cpp

@@ -249,437 +239,6 @@ class LoadNvidiaModules
    PFNNvOFAPICreateInstanceCuda GetOFLibraryFunctionPtr() { return m_NvOFAPICreateInstanceCuda; }
 };

-class NvidiaOpticalFlowImpl : public cv::cuda::NvidiaOpticalFlow_1_0


I believe 1_0 implementation can work with 2.0 SDK.
Perhaps there is no reason to remove that.

Yes, you were right.
I could restore the 1_0 implementation.

There was another build failure but that was due to the wrong function call in the disabled part.

tomoaki0705 · 2021-01-23T10:34:54Z

Thanks for the review @alalek.
Current my recognition is that 1.0 and 2.0 doesn't have compatibility, and that's why I dropped the 1.0 implementation.
Let me get back by checking the compatibility between 1.0 and 2.0 carefully.

tomoaki0705 · 2021-01-23T14:23:17Z

So, I've restored the 1_0 implementation.
Basic modification is cmake part only.
I also fixed some wrong function calls.

vchiluka5 · 2021-01-24T05:56:22Z

Just for clarification. SDK 2.0 works with 1.0 as well and hence we have 2 classes exposed.
we pull SDK 2.0 headers which are compatible with SDK 1.0.
Thanks @alalek for reviewing.
Thanks @tomoaki0705, Final PR looks Good to me.

alalek · 2021-01-24T15:17:04Z

@tomoaki0705 Build is failing on .cu file compilation (check "Custom" builder with CUDA 8.0).
Could you please add #ifdef HAVE_NVIDIA_OPTFLOW guard there?

* follow the review comment * add missing constructors when not covered * add definition in cu file

tomoaki0705 force-pushed the fixBuildCudaOptFlow branch from d085506 to 5795d8d Compare January 22, 2021 23:39

alalek reviewed Jan 23, 2021

View reviewed changes

tomoaki0705 force-pushed the fixBuildCudaOptFlow branch from 5795d8d to a821659 Compare January 23, 2021 14:15

tomoaki0705 changed the title ~~cudaoptflow: fix build failure on CUDA 8.0~~ cudaoptflow: disable Optical Flow SDK when CUDA version is insufficient Jan 23, 2021

disable NVIDIA Optical Flow SDK when CUDA < 10.0

6e52be3

* follow the review comment * add missing constructors when not covered * add definition in cu file

tomoaki0705 force-pushed the fixBuildCudaOptFlow branch from a821659 to 6e52be3 Compare January 24, 2021 23:40

alalek approved these changes Jan 25, 2021

View reviewed changes

opencv-pushbot merged commit 0b8aecd into opencv:master Jan 25, 2021

tomoaki0705 deleted the fixBuildCudaOptFlow branch January 25, 2021 07:46

alalek mentioned this pull request Jan 25, 2021

Unable to build cudaoptflow #2834

Closed

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #2919

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cudaoptflow: disable Optical Flow SDK when CUDA version is insufficient #2824

cudaoptflow: disable Optical Flow SDK when CUDA version is insufficient #2824

tomoaki0705 commented Jan 18, 2021 •

edited by alalek

Loading

vchiluka5 commented Jan 18, 2021

vchiluka5 commented Jan 19, 2021

alalek commented Jan 19, 2021

tomoaki0705 commented Jan 21, 2021

vchiluka5 commented Jan 21, 2021

tomoaki0705 commented Jan 21, 2021

vchiluka5 commented Jan 21, 2021

alalek left a comment

alalek Jan 23, 2021

tomoaki0705 Jan 23, 2021

alalek Jan 23, 2021

tomoaki0705 Jan 23, 2021

alalek Jan 23, 2021

tomoaki0705 Jan 23, 2021

tomoaki0705 commented Jan 23, 2021

tomoaki0705 commented Jan 23, 2021

vchiluka5 commented Jan 24, 2021

alalek commented Jan 24, 2021

cudaoptflow: disable Optical Flow SDK when CUDA version is insufficient #2824

cudaoptflow: disable Optical Flow SDK when CUDA version is insufficient #2824

Conversation

tomoaki0705 commented Jan 18, 2021 • edited by alalek Loading

Pull Request Readiness Checklist

vchiluka5 commented Jan 18, 2021

vchiluka5 commented Jan 19, 2021

alalek commented Jan 19, 2021

tomoaki0705 commented Jan 21, 2021

vchiluka5 commented Jan 21, 2021

tomoaki0705 commented Jan 21, 2021

vchiluka5 commented Jan 21, 2021

alalek left a comment

Choose a reason for hiding this comment

alalek Jan 23, 2021

Choose a reason for hiding this comment

tomoaki0705 Jan 23, 2021

Choose a reason for hiding this comment

alalek Jan 23, 2021

Choose a reason for hiding this comment

tomoaki0705 Jan 23, 2021

Choose a reason for hiding this comment

alalek Jan 23, 2021

Choose a reason for hiding this comment

tomoaki0705 Jan 23, 2021

Choose a reason for hiding this comment

tomoaki0705 commented Jan 23, 2021

tomoaki0705 commented Jan 23, 2021

vchiluka5 commented Jan 24, 2021

alalek commented Jan 24, 2021

tomoaki0705 commented Jan 18, 2021 •

edited by alalek

Loading