Add support thread-local directx (OpenCL interop) initialization#18593
Add support thread-local directx (OpenCL interop) initialization#18593alalek merged 5 commits intoopencv:masterfrom
Conversation
alalek
left a comment
There was a problem hiding this comment.
Thank you for contribution!
BTW, did you try to validate this patch on configuration with multiple GPUs on some example?
modules/core/src/directx.cpp
Outdated
| if (impl.initializedPlatform9 != platform) | ||
| { | ||
| clCreateFromDX9MediaSurfaceKHR = (clCreateFromDX9MediaSurfaceKHR_fn) | ||
| impl.clCreateFromDX9MediaSurfaceKHR = (clCreateFromDX9MediaSurfaceKHR_fn) |
There was a problem hiding this comment.
Perhaps such initialization should be moved to OpenCLDirectXImpl methods to keep code responsibilities clean.
There was a problem hiding this comment.
OK, move all the initialization to OpenCLDirectXImpl.
modules/core/src/directx.cpp
Outdated
| if (!platform) | ||
| { | ||
| cl_device_id current_device = (cl_device_id)ocl::Device::getDefault().ptr(); | ||
| CV_Assert(current_device); |
There was a problem hiding this comment.
Perhaps it is better to move this below (after platforms query, before bool found = false;)
There was a problem hiding this comment.
OK, I move this below.
(I'm changing it to use clDeviceInfo(), so it doesn't have much effect.)
| cl_platform_id getPlatform() | ||
| { | ||
| if (!platform) | ||
| { |
There was a problem hiding this comment.
It make sense to add this check here: CV_Assert(cv::ocl::haveOpenCV());
There was a problem hiding this comment.
Maybe haveOpenCL instead of haveOpenCV?
modules/core/src/ocl.cpp
Outdated
| namespace cv { namespace directx { | ||
| struct OpenCLDirectXImpl; | ||
| OpenCLDirectXImpl* createDirectXImpl(); | ||
| void deleteDirectXImpl(OpenCLDirectXImpl**); |
There was a problem hiding this comment.
Please move this into modules/core/src/directx.cpp and use it instead (here and in directx.hpp).
Consider using this namespace:
cv::directx::internal- or
cv::directx::detail
There was a problem hiding this comment.
OK, move this into modules/core/src/directx.cpp and using cv::directx::internal namespace.
| OpenCLDirectXImpl& getImpl() | ||
| { | ||
| OpenCLDirectXImpl* i = getDirectXImpl(ocl::Context::getDefault()); | ||
| CV_Assert(i); | ||
| return *i; | ||
| } |
There was a problem hiding this comment.
static call of OpenCLDirectXImpl?
modules/core/src/directx.cpp
Outdated
| if (current_device == devices[j]) | ||
| { | ||
| platform = platforms[i]; | ||
| found = true; |
There was a problem hiding this comment.
Perhaps you want to use clGetDeviceInfo() call with CL_DEVICE_PLATFORM.
There was a problem hiding this comment.
Oh! Thank you!
Certainly the platform can be obtained using clGetDeviceInfo(). Change it to use this.
|
It has been verified in my multi-GPU environment. |
|
The error in the above comment occurs when using an NVIDIA GPU. |
Add support thread-local directx (OpenCL interop) initialization * support thread-local directx (OpenCL interop) initialization * reflect reviews * Remove verbose function prototype declarations * Countermeasures for VC warnings. (declaration of 'platform' hides class member) * core(directx): remove internal stuff from public headers
Add support thread-local directx (OpenCL interop) initialization * support thread-local directx (OpenCL interop) initialization * reflect reviews * Remove verbose function prototype declarations * Countermeasures for VC warnings. (declaration of 'platform' hides class member) * core(directx): remove internal stuff from public headers
Add support for DirectX-OpenCL interop thread-local initialization.
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.