Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork56.4k
G-API: Advanced device selection for ONNX DirectML Execution Provider#24060
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 ourterms of service andprivacy 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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
954113d to0e60eccCompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| #ifdef HAVE_DXCORE | ||
| // FIXME: Fix warning | ||
| #defineWINVER0x0A00 |
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 definesWindows 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
WINVERglobally 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 howvideoio handles that:https://github.com/opencv/opencv/blob/4.x/modules/videoio/src/cap_msmf.cpp#L13-L17
Uh oh!
There was an error while loading.Please reload this page.
modules/gapi/CMakeLists.txt Outdated
| 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 howdxcore,d3d12 anddirectml 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 likeOpenCVDetectDirectML.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
dmatveev left a comment
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...
Uh oh!
There was an error while loading.Please reload this page.
smirnov-alexey left a comment
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
smirnov-alexey commentedAug 28, 2023
@TolyaTalamanov also, can we add some tests or a sample on that feature? |
asmorkalov commentedOct 20, 2023
@TolyaTalamanov@dmatveev Friendly reminder. |
1 similar comment
asmorkalov commentedNov 8, 2023
@TolyaTalamanov@dmatveev Friendly reminder. |
TolyaTalamanov commentedNov 9, 2023
@opencv-alalek@asmorkalov Hi folks! Could you have a look at this one more time, please? |
TolyaTalamanov commentedNov 13, 2023
Hi@asmorkalov can we proceed with merge here? |
TolyaTalamanov commentedNov 15, 2023
@asmorkalov Hi! Let me know if there is anything else left |
asmorkalov left a comment
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.
👍
dmatveev commentedNov 16, 2023
Cool - thanks@asmorkalov ! |
…e-selection-onnxrt-directmlG-API: Advanced device selection for ONNX DirectML Execution Provideropencv#24060### OverviewExtend `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 ChecklistSee details athttps://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-directmlG-API: Advanced device selection for ONNX DirectML Execution Provideropencv#24060### OverviewExtend `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 ChecklistSee details athttps://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-directmlG-API: Advanced device selection for ONNX DirectML Execution Provideropencv#24060### OverviewExtend `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 ChecklistSee details athttps://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

Uh oh!
There was an error while loading.Please reload this page.
Overview
Extend
cv::gapi::onnx::ep::DirectMLto acceptadapter nameasctorparameter in order to select execution device byname.E.g:
Pull Request Readiness Checklist
See details athttps://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.