Skip to content

Added stream support on hough circles, lines and segments #2801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 31, 2020

Conversation

Atlas42
Copy link
Contributor

@Atlas42 Atlas42 commented Dec 29, 2020

  • Passed the stream to the different cuda, OpenCV and thurst library calls
  • Replace all device by cuda synchronizes
  • Added extra synchronize calls after device to host transfers
  • Replaced the cuda globals by allocated values

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
Xbuild_image:Custom=ubuntu-cuda-cc52:18.04
build_image:Custom=ubuntu-cuda:16.04

- Passed the stream to the different cuda, OpenCV and thurst library calls
- Replace all device by cuda synchronizes
- Added extra synchronize calls after device to host transfers
- Replaced the cuda globals by allocated values
Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contribution!


totalCount = ::min(totalCount, maxSize);

if (doSort && totalCount > 0)
{
thrust::device_ptr<float2> outPtr(out);
thrust::device_ptr<int> votesPtr(votes);
thrust::sort_by_key(votesPtr, votesPtr + totalCount, outPtr, thrust::greater<int>());
thrust::sort_by_key(thrust::cuda::par.on(stream), votesPtr, votesPtr + totalCount, outPtr, thrust::greater<int>());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please take a look if there is an easy way to guard this code for CUDA 8.x?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand your question here.
Is there a problem specific to CUDA 8 ? I have checked the API and it seems fine.
As Cuda 8 is not supported anymore on Ubuntu 18.04 or Visual Studio 19 (my dev tools),
I took some time today to setup an Ubuntu 16.04/CUDA 8 and it builds properly with OpenCV 4.5.0 (master did not on some unrelated problem).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take a look on build issues of "Custom" CUDA builder here.

There are several ways:

  • try to fix the build using #if guards with CUDA version checks (requested above)
  • try to fix the build through disabling this algorithm for CUDA 8.0
  • open a discussion about deprecation of old CUDA versions in next OpenCV releases (will take some time)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be fixed on my last commit.
I managed to reproduce the build problem, the instruction on the NVIDIA website to install cuda 8 were wrong, I should have checked.
Turns out that the thrust::cuda::par did exist in CUDA 8, but was not included by default.

Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done! Thank you for contribution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants