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
G-API: Advanced device selection for ONNX DirectML Execution Provider #24060
G-API: Advanced device selection for ONNX DirectML Execution Provider #24060
Conversation
954113d
to
0e60ecc
Compare
#ifdef HAVE_DXCORE | ||
|
||
// FIXME: Fix warning | ||
#define WINVER 0x0A00 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OpenCV defines Windows 7
there:
https://github.com/opencv/opencv/blob/4.x/cmake/OpenCVCompilerOptions.cmake#L477-L478
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I propose to set
WINVER
globally in CMake. - Cmake should print this during configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already defined in cmake: https://github.com/opencv/opencv/blob/4.x/cmake/platforms/OpenCV-WinRT.cmake#L18
This is how videoio
handles that: https://github.com/opencv/opencv/blob/4.x/modules/videoio/src/cap_msmf.cpp#L13-L17
modules/gapi/CMakeLists.txt
Outdated
@@ -369,6 +369,8 @@ if(HAVE_ONNX) | |||
ocv_target_compile_definitions(${the_module} PRIVATE HAVE_ONNX=1) | |||
if(HAVE_ONNX_DML) | |||
ocv_target_compile_definitions(${the_module} PRIVATE HAVE_ONNX_DML=1) | |||
# FIXME: It shouldn't be here... | |||
ocv_target_compile_definitions(${the_module} PRIVATE HAVE_DXCORE=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@opencv-alalek Could you advice how dxcore
, d3d12
and directml
can be detected by opencv?
Should we use the similar approach as there:
https://github.com/opencv/opencv/blob/4.x/cmake/OpenCVDetectDirectX.cmake ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we need to have something like OpenCVDetectDirectML.cmake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to all open self-questions regarding managing the DX components and Windows version properly...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks OK but you need to resolve DXCORE and Windows version dependencies
@TolyaTalamanov also, can we add some tests or a sample on that feature? |
@TolyaTalamanov @dmatveev Friendly reminder. |
1 similar comment
@TolyaTalamanov @dmatveev Friendly reminder. |
@opencv-alalek @asmorkalov Hi folks! Could you have a look at this one more time, please? |
Hi @asmorkalov can we proceed with merge here? |
@asmorkalov Hi! Let me know if there is anything else left |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Cool - thanks @asmorkalov ! |
…e-selection-onnxrt-directml G-API: Advanced device selection for ONNX DirectML Execution Provider opencv#24060 ### Overview Extend `cv::gapi::onnx::ep::DirectML` to accept `adapter name` as `ctor` parameter in order to select execution device by `name`. E.g: ``` pp.cfgAddExecutionProvider(cv::gapi::onnx::ep::DirectML("Intel Graphics")); ``` ### 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
…e-selection-onnxrt-directml G-API: Advanced device selection for ONNX DirectML Execution Provider opencv#24060 ### Overview Extend `cv::gapi::onnx::ep::DirectML` to accept `adapter name` as `ctor` parameter in order to select execution device by `name`. E.g: ``` pp.cfgAddExecutionProvider(cv::gapi::onnx::ep::DirectML("Intel Graphics")); ``` ### 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
Overview
Extend
cv::gapi::onnx::ep::DirectML
to acceptadapter name
asctor
parameter in order to select execution device byname
.E.g:
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.