8000 `cuda`: Add bindings to allow `GpuMat` and `Stream` objects to be initialized from memory initialized in other libraries by cudawarped · Pull Request #23371 · opencv/opencv · GitHub
[go: up one dir, main page]
Skip to content

cuda: Add bindings to allow GpuMat and Stream objects to be initialized from memory initialized in other libraries #23371

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 22, 2023
Merged

cuda: Add bindings to allow GpuMat and Stream objects to be initialized from memory initialized in other libraries #23371

merged 1 commit into from
May 22, 2023

Conversation

cudawarped
Copy link
Contributor
@cudawarped cudawarped commented Mar 17, 2023 •
edited
Loading

Added python bindings to enable GpuMat and Stream objects to be initialized from raw pointers + updated test cases.

This #16513 exposed the memory created by OpenCV GpuMat and Streams to other python libraries whereas this PR should allow OpenCV GpuMat and Stream objects to use raw pointers to memory created in other python libraries.

Related issues rapidsai/cucim#329 (comment) and opencv/opencv-python#818

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 another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the 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

@cudawarped cudawarped changed the title Add bindings to allow GpuMat and Stream objects to be initailized from memory initialized in other libraries Add bindings to allow GpuMat and Stream objects to be initialized from memory initialized in other libraries Mar 17, 2023
@cudawarped cudawarped force-pushed the cuda_add_futher_python_interop branch from 076edfb to 25cc357 Compare March 17, 2023 18:29
@manbehindthemadness
Copy link

Oh, this is a wonderful idea! This will make life so much easier when performing cross-library interop.

@asmorkalov asmorkalov self-requested a review March 24, 2023 10:48
@asmorkalov asmorkalov self-assigned this Mar 24, 2023
@asmorkalov
Copy link
Contributor

Thanks a lot for the contribution! Couple of general notes:

  • Please add CV_DEPRECATED_EXTERNAL macro and add documentation note that the functions are designed for bindings only, not for usage in C++.
  • Please rename createGpuMat to createGpuMatFromCudaMemory to stress that it's external pointed.

@cudawarped
Copy link
Contributor Author
  • Please add CV_DEPRECATED_EXTERNAL macro

I've tried adding this macro, for example

CV_DEPRECATED_EXTERNAL Stream wrapStream(uint64 cudaStreamMemoryAddress);

but it doesn't generate the python bindings, what is the required format for applying it to generate bindings and disable the export of the function to the shared library?

@cudawarped cudawarped force-pushed the cuda_add_futher_python_interop branch from 25cc357 to 4a6a441 Compare March 24, 2023 13:24
@asmorkalov
Copy link
Contributor

Looks like you need CV_EXPORTS_W together with CV_DEPRECATED_EXTERNAL. It generates public symbol for python, but mark the function as deprecated in C++ outside of OpenCV build tree. So external uses will get warning during build.

@cudawarped
Copy link
Contributor Author

If I combine the two, e.g.

CV_EXPORTS_W CV_DEPRECATED_EXTERNAL GpuMat inline createGpuMatFromCudaMemory(int rows, int cols, int type, uint64 cudaMemoryAddress, size_t step = Mat::AUTO_STEP)

then the automatically generated code for the bindings has an extra space and becomes

static PyObject* pyopencv_cv_cuda_GpuMat createGpuMatFromCudaMemory(PyObject* , PyObject* py_args, PyObject* kw)

instead of

static PyObject* pyopencv_cv_cuda_GpuMat_createGpuMatFromCudaMemory(PyObject* , PyObject* py_args, PyObject* kw)

resulting in a compilation error.

@cudawarped
Copy link
Contributor Author

@asmorkalov
What do you make of this error regarding unsigned long long on maxOS?

In file included from /Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/opencv/modules/python/src2/cv2.cpp:17:
/Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/opencv/modules/python/src2/cv2_convert.hpp:55:92: error: no member named 'to' in 'PyOpenCV_Converter<unsigned long long>'
bool pyopencv_to(PyObject* obj, T& p, const ArgInfo& info) { return PyOpenCV_Converter<T>::to(obj, p, info); }
                                                                    ~~~~~~~~~~~~~~~~~~~~~~~^
/Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/opencv/modules/python/src2/cv2_convert.hpp:29:16: note: in instantiation of function template specialization 'pyopencv_to<unsigned long long>' requested here
        return pyopencv_to(obj, value, info);
               ^
/Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/build/modules/python_bindings_generator/pyopencv_generated_funcs.h:25764:9: note: in instantiation of function template specialization 'pyopencv_to_safe<unsigned long long>' requested here
        pyopencv_to_safe(pyobj_cudaMemoryAddress, cudaMemoryAddress, ArgInfo("cudaMemoryAddress", 0)) &&
        ^
1 error generated.

@cudawarped cudawarped changed the title Add bindings to allow GpuMat and Stream objects to be initialized from memory initialized in other libraries cuda: Add bindings to allow GpuMat and Stream objects to be initialized from memory initialized in other libraries Apr 5, 2023
@opencv-alalek opencv-alalek added this to the 4.8.0 milestone Apr 6, 2023
asmorkalov
asmorkalov approved these changes Apr 20, 2023
View reviewed changes
Copy link
Contributor
@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

👍

@asmorkalov asmorkalov self-requested a review April 20, 2023 10:00
@cudawarped cudawarped force-pushed the cuda_add_futher_python_interop branch from c91a5c7 to f3c8a25 Compare April 20, 2023 11:13
@cudawarped
Copy link
Contributor Author

@asmorkalov I'm not sure what is causing the failure as I don't have a mac to test on. I suspect it could be the use of the optional uint64 datatype which I have now replaced everywhere with int_least64_t.

@asmorkalov
Copy link
Contributor

I'll take a look.

@cudawarped cudawarped mentioned this pull request May 15, 2023
Open
@alllexx88 alllexx88 mentioned this pull request May 16, 2023
Closed
opencv-alalek
opencv-alalek reviewed May 19, 2023
View reviewed changes
modules/python/test/test_cuda.py
cuMatFromPtrSz = cv.cuda.createGpuMatFromCudaMemory(cuMat.size(),cuMat.type(),cuMat.cudaPtr(), cuMat.step)
self.assertTrue(cuMat.cudaPtr() == cuMatFromPtrSz.cudaPtr())
cuMatFromPtrRc = cv.cuda.createGpuMatFromCudaMemory(cuMat.size()[1],cuMat.size()[0],cuMat.type(),cuMat.cudaPtr(), cuMat.step)
self.assertTrue(cuMat.cudaPtr() == cuMatFromPtrRc.cudaPtr())
Copy link
Contributor

Choose a reason for hiding this comment

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

@asmorkalov we don't have python tests (and build) on CI with CUDA: https://github.com/opencv/opencv/actions/runs/4753621211/jobs/8445779900?pr=23371 (there is dnn test only)

@asmorkalov
Copy link
Contributor

Tested manually with Ubuntu 18.04, Python 3.6, CUDA 10.2.

asmorkalov
asmorkalov approved these changes May 19, 2023
View reviewed changes
Copy link
Contributor
@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

👍

@cudawarped cudawarped force-pushed the cuda_add_futher_python_interop branch from f3c8a25 to 7539abe Compare May 22, 2023 08:38
@asmorkalov asmorkalov merged commit e3c5c09 into opencv:4.x May 22, 2023
@asmorkalov asmorkalov mentioned this pull request May 31, 2023
Merged
@kmaehashi kmaehashi mentioned this pull request Jun 2, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@opencv-alalek opencv-alalek opencv-alalek left review comments

@asmorkalov asmorkalov asmorkalov approved these changes

Assignees

@asmorkalov asmorkalov

Labels
category: gpu/cuda (contrib) OpenCV 4.0+: moved to opencv_contrib category: python bindings
Projects
None yet
Milestone
4.8.0
Development

Successfully merging this pull request may close these issues.
4 participants
@cudawarped @manbehindthemadness @asmorkalov @opencv-alalek
0