-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[OpenCV] Add MSMF & MSMF-DXVA as a Option #15461
[OpenCV] Add MSMF & MSMF-DXVA as a Option #15461
Conversation
581b852
to
4ce22f0
Compare
I detected other pull requests that are modifying opencv/4.x recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
4ce22f0
to
b41bc51
Compare
This comment has been minimized.
This comment has been minimized.
recipes/opencv/4.x/conanfile.py
Outdated
@@ -25,6 +25,7 @@ class OpenCVConan(ConanFile): | |||
options = { | |||
"shared": [True, False], | |||
"fPIC": [True, False], | |||
"build_world":[True, False], |
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 not a very good name like all recipe options build_<something>
, I thinkworld
is a better name and consistent with other option dnn
to build opencv_dnn.
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.
Agree, I will changed it.
recipes/opencv/4.x/conanfile.py
Outdated
"with_msmf": False, | ||
"with_msmf_dxva": False, |
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.
these options should be removed if os is not Windows, and enabled by default.
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.
Agree, I have not possibility to test on other operation system.
recipes/opencv/4.x/conanfile.py
Outdated
if self.options.build_world and conan_component == "opencv_world": | ||
self.cpp_info.components[conan_component].libs = [lib_name] | ||
elif not self.options.build_world: | ||
self.cpp_info.components[conan_component].libs = [lib_name] | ||
|
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.
Are you sure that all other targets are defined in official OpenCV config file when world is built?
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've checked and other targets are created, but they actually depend on opencv_world
target, not the opposite. So logic proposed here is broken.
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.
That why I add this condition, opencv_world will combine all generated dll with only one dll which called opencv_world.dll
When world is disable, all dll will be created, tested on my side and working well.
I will recheck with the new version of the conanfile.py
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 know, but:
- all other components part of world should depend on opencv_world. They shouldn't have requires, system libs or frameworks themselves, everything should be aggregated in requires/system_libs/frameworks of opencv_world component
- 2 extra modules are never part of world: img_hash & sfm (and its static dependencies correspondence, multiview and numeric)
- ippiw is not part of world
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've updated #15193 to add world
option.
…efault Option removed if os is not windows
b41bc51
to
87258de
Compare
This comment has been minimized.
This comment has been minimized.
I remove opencv world which is added on your pull request now. |
Co-authored-by: SpaceIm <[email protected]>
Co-authored-by: SpaceIm <[email protected]>
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
@yekmen Thank you for your contribution, please, check the account |
4a45e51
to
8f3733b
Compare
This comment has been minimized.
This comment has been minimized.
|
CI is red, you can close/open to trigger it again. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
According to the CI error, don't know why my PR failed. Error : But ffmpeg is available on remote Any help is welcome |
Conan v1 pipeline ✔️All green in build 18 (
Conan v2 pipeline (informative, not required for merge) ❌
The v2 pipeline failed. Please, review the errors and note this will be required for pull requests to be merged in the near future. See details:Failure in build 22 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. |
Hooks produced the following warnings for commit a11b20copencv/4.5.1
|
Specify library name and version: opencv/4.5.5
Add as option to OpenCV: