search

microsoft / vcpkg

C++ Library Manager for Windows, Linux, and MacOS
MIT License
22.98k stars 6.34k forks source link

[opencv3] patch to vcpkg build of 3.4.7-2 #11875

Closed pwm1234 closed 3 years ago

pwm1234 commented 4 years ago

Library name: opencv3[contrib]

Thanks for working on vcpkg and the opencv port! (I hope to soon be moving to opencv4, but until then ...)

The opencv3[contrib] feature will use hdf5 when it is available. But the opencv3 contrib /modules/hdf/CMakeLists.txt is broken on windows. It has some hand-crafted logic for finding hdf5, and that seems to work for the release build. But it does not work for the vcpkg debug build. Besides it is not necessary because the vpkg build of hdf5 generates a correct config script on windows. So the find package logic for hdf5 on windows should be the same as non-windows, which is simply find_package(HDF5).

The attached zip file has 0010-win32-hdf5.patch and with an updated portfile.cmake that uses the patch. opencv3-patch.zip

cenit commented 4 years ago

I can try to include it in #11130

JackBoosY commented 4 years ago

Hi @pwm1234 , since #11130 merged, does this issue still exist?

pwm1234 commented 4 years ago

No. My patch is for the opencv contrib hdf CMakeLists.txt file https://github.com/opencv/opencv_contrib/blob/master/modules/hdf/CMakeLists.txt. The error in the opencv find logic (at least on windows with opencv 3.4, which is all that I am using right now), is that there should not be special find logic for WIN32. What is there does not work with a vcpkg build of hdf5. A simple find_package(HDF5) does work.

Since I do not see a patch in this merge that addresses this, I believe the problem will still exist. (I have not, however, pulled the HEAD of vcpkg to confirm. Would you like me to?)

cenit commented 4 years ago

In case you still require a patch (I think I tested hdf5 features, but I might be wrong), please do it both for opencv3 and opencv4, thanks (they have very similar build toolchain)

NancyLi1013 commented 4 years ago

@pwm1234 Please let me know if this is still a problem for you.

Thanks.

pwm1234 commented 4 years ago

Thanks for double-checking. This is still a problem. As I mentioned above (in my comment on Jul 29) a simple find_package hdf5 is what is now correct for windows. The special logic workaround for windows leads to the debug build of opencv incorrectly using the release lib or hdf5. The build will still complete, but running the debug will lead to a crash because you are mixing debug and release code. You can see this by looking at the CMakeCache.txt file for the debug configuration build of opencv3. Mine shows

CMAKE_BUILD_TYPE:STRING=Debug
HDF5_C_LIBRARY:FILEPATH=C:/temp/vcpkg/installed/x64-windows/lib/hdf5.lib

While HDF5_C_LIBRARY should be C:/temp/vcpkg/installed/x64-windows/debug/lib/hdf5.lib

pwm1234 commented 3 years ago

@NancyLi1013 and @JackBoosY the opencv contrib modules/hdf/CMakeLists.txt still has the incorrect logic for win32. The special logic does not properly link the debug hdf5 libraries. It still builds, but it does not correctly link against the debug libraries.

JackBoosY commented 3 years ago

cc @cenit

pwm1234-sri commented 3 years ago

@JackBoosY @cenit I just wanted to check on the status of this issue. I am hoping it gets fixed before the next vcpkg release (which I also hope is coming soon!)

cenit commented 3 years ago

Sorry still not fixed. I will keep it on my radar for the next ocv4.5.1 update

pwm1234-sri commented 3 years ago

That would be great. Any idea when the 4.5.1 update will be? (A week, month, 6 months, ...?) Of course I am just looking for a ballpark estimate. I appreciate your help. (Also, any idea when the next vcpkg release will be?)

cenit commented 3 years ago

A week

pwm1234-sri commented 3 years ago

THANK YOU!