Skip to content
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

Fix windows build #68

Merged
merged 2 commits into from
Sep 29, 2020
Merged

Fix windows build #68

merged 2 commits into from
Sep 29, 2020

Conversation

shagren
Copy link
Contributor

@shagren shagren commented Sep 21, 2020

I cannot build module on windows via command from README.md: like as --global-option=... not passed to setup.py.
But I can install module from source via python setup.py build_ext -I....
Option --no-use-pep517 is easiest workaround. Let's fix README.

Maybe after #39 some additional fix will be required to build system.

@@ -32,7 +32,7 @@ Make sure you replace the paths in the following instructions with your own k4a
It is important to replace `1.2.0` with your installed version of the SDK.

```
pip install pyk4a --global-option=build_ext --global-option="-IC:\Program Files\Azure Kinect SDK v1.2.0\sdk\include" --global-option="-LC:\Program Files\Azure Kinect SDK v1.2.0\sdk\windows-desktop\amd64\release\lib"
pip install pyk4a --no-use-pep517 --global-option=build_ext --global-option="-IC:\Program Files\Azure Kinect SDK v1.4.1\sdk\include" --global-option="-LC:\Program Files\Azure Kinect SDK v1.4.1\sdk\windows-desktop\amd64\release\lib"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shagren shagren marked this pull request as ready for review September 22, 2020 12:04
@shagren
Copy link
Contributor Author

shagren commented Sep 22, 2020

@lpasselin ^

@shagren shagren mentioned this pull request Sep 28, 2020
@lpasselin lpasselin merged commit f3d1814 into etiennedub:master Sep 29, 2020
@lpasselin
Copy link
Collaborator

lpasselin commented Sep 29, 2020

@shagren Would you mind trying to install similarly to how they do it here?

warning: I am not sure how to handle spaces in paths on windows.

C:\> set LIB=C:\Program Files\Azure Kinect SDK v1.4.1\sdk\windows-desktop\amd64\release\lib;%LIB%  
C:\> set INCLUDE=C:\Program Files\Azure Kinect SDK v1.4.1\sdk\include;%INCLUDE%  
C:\> pip install pyk4a

Then try example/viewer.py to make sure it can be imported.

@shagren
Copy link
Contributor Author

shagren commented Sep 30, 2020

@lpasselin ,

C:> set LIB=C:\Program Files\Azure Kinect SDK v1.4.1\sdk\windows-desktop\amd64\release\lib;%LIB%
C:> set INCLUDE=C:\Program Files\Azure Kinect SDK v1.4.1\sdk\include;%INCLUDE%
C:> pip install pyk4a

It's helps with pip. Module compiled and installed. This solution is better then my.

But viewer didn't work:

Traceback (most recent call last):
  File "viewer.py", line 3, in <module>
    import pyk4a
  File "c:\tmp\venv-pyk4a\lib\site-packages\pyk4a\__init__.py", line 1, in <module>
    from .calibration import Calibration, CalibrationType
  File "c:\tmp\venv-pyk4a\lib\site-packages\pyk4a\calibration.py", line 4, in <module>
    import k4a_module
ImportError: DLL load failed while importing k4a_module: The specified module could not be found.

I have tried to add C:\Program Files\Azure Kinect SDK v1.4.1\sdk\windows-desktop\amd64\release\bin to %PATH% but without success.

Next snippet added before import pyk4a helps:

from ctypes import c_wchar_p, windll  
from ctypes.wintypes import DWORD

AddDllDirectory = windll.kernel32.AddDllDirectory
AddDllDirectory.restype = DWORD
AddDllDirectory.argtypes = [c_wchar_p]
AddDllDirectory(r"C:\Program Files\Azure Kinect SDK v1.4.1\sdk\windows-desktop\amd64\release\bin")

@lpasselin
Copy link
Collaborator

I have tried to add C:\Program Files\Azure Kinect SDK v1.4.1\sdk\windows-desktop\amd64\release\bin to %PATH% but without success.

I think the problem is only from python 3.8, as detailed here: https://docs.python.org/3/whatsnew/3.8.html

DLL dependencies for extension modules and DLLs loaded with ctypes on Windows are now resolved more securely. Only the system paths, the directory containing the DLL or PYD file, and directories added with add_dll_directory() are searched for load-time dependencies. Specifically, PATH and the current working directory are no longer used, and modifications to these will no longer have any effect on normal DLL resolution.

So if I understand (didn't test), before python 3.8, we could load a DLL by simply including the directory to the PATH environment variable.

At runtime, import pyk4a doesn't work because it cannot find k4a.dll. We can fix this by adding some code to pyk4a.__init__.py

Your code works, probably for python versions 3.8 and before.

from ctypes import c_wchar_p, windll  
from ctypes.wintypes import DWORD

AddDllDirectory = windll.kernel32.AddDllDirectory
AddDllDirectory.restype = DWORD
AddDllDirectory.argtypes = [c_wchar_p]
AddDllDirectory(r"C:\Program Files\Azure Kinect SDK v1.4.1\sdk\windows-desktop\amd64\release\bin")

In python 3.8, we can now use os.add_dll_directory https://docs.python.org/3/library/os.html#os.add_dll_directory

os.add_dll_directory(r"C:\Program Files\Azure Kinect SDK v1.4.1\sdk\windows-desktop\amd64\release\bin")
  1. My problem with both solutions is that we need to find the path of the directory containing the DLL. Is there a way to input that during the install like pip install pyk4a --k4a-root="C:\Program Files\Azure Kinect SDK v1.4.1\sdk\"? We would guess for LIB and INCLUDE and remember this k4a-root path to add the DLL at runtime?

  2. An other solution is to use github actions to publish precompiled wheels to pypi for windows which would not require installing the k4a SDK by the users. These should include k4a.lib k4arecord.lib k4a.dll k4arecord.dll and depthengine_2_0.dll for every combination of versions {python, windows,k4a¹}. Total filesize for all the required files is around 3MB. This is acceptable if it saves users a lot of time. The solution would probably fix most windows noob problems but take us a while to setup. See example in this project https://github.com/sqlalchemy/sqlalchemy/blob/master/.github/workflows/create-wheels.yaml. Also this approach could make development on windows harder.

What do you think about this? Am I getting something wrong?

¹: we could also force the most recent k4a SDK version and ask people to upgrade their kinect firmware if they want to use pyk4a

@shagren shagren deleted the fix-windows-build branch March 1, 2021 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants