emilianavt / OpenSeeFace

Robust realtime face and facial landmark tracking on CPU with Unity integration
BSD 2-Clause "Simplified" License
1.46k stars 152 forks source link

add build workflow for windows, mac, and linux #47

Closed you-win closed 2 years ago

you-win commented 2 years ago

Adds a GitHub workflow to automatically build OpenSeeFace when:

  1. A new tag is pushed (assuming the tag is prefixed with a "v" e.g. v1.1.1)
  2. Manually triggering a build

Bash is used for all OSs to create the builds. I've tested this to work on Windows 10 and MacOS Monterey. My test binaries can be found here

Improvements

  1. Do you want this to be manually triggered only?
  2. What should the resulting files be called?
    • Currently they are named windows-latest.zip, ubuntu-latest.zip, and macos-latest.zip. These names are taken from the GitHub workflow OS names
  3. This does not create a new GitHub release. It only creates artifacts that are stored for 90 days. This functionality could be added if desired

Issues

The above exception was the direct cause of the following exception:

Traceback (most recent call last): File "input_reader.py", line 196, in init File "input_reader.py", line 63, in init File "dshowcapture.py", line 47, in init File "ctypes__init.py", line 452, in LoadLibrary File "PyInstaller\loader\pyimod03_ctypes.py", line 55, in init pyimod03_ctypes.install..PyInstallerImportError: Failed to load dynlib/dll 'C:\Users\theaz\Downloads\windows-latest (1)\dshowcapture\dshowcapture_x64.dll'. Most likely this dynlib/dll was not found when the application was frozen. Exception ignored in: <function DShowCapture.del at 0x0000021F24C9BDC0> Traceback (most recent call last): File "dshowcapture.py", line 96, in del AttributeError: 'DShowCapture' object has no attribute 'buffer' DShowCapture failed. Falling back to escapi for device . Escapi exception: Traceback (most recent call last): File "PyInstaller\loader\pyimod03_ctypes.py", line 53, in init File "ctypes\init.py", line 374, in init__ FileNotFoundError: Could not find module 'C:\Users\theaz\Downloads\windows-latest (1)\escapi\escapi_x64.dll' (or one of its dependencies). Try using the full path with constructor syntax.

The above exception was the direct cause of the following exception:

Traceback (most recent call last): File "input_reader.py", line 212, in init File "escapi.py", line 65, in init File "ctypes__init__.py", line 452, in LoadLibrary File "PyInstaller\loader\pyimod03_ctypes.py", line 55, in init pyimod03_ctypes.install..PyInstallerImportError: Failed to load dynlib/dll 'C:\Users\theaz\Downloads\windows-latest (1)\escapi\escapi_x64.dll'. Most likely this dynlib/dll was not found when the application was frozen. Escapi failed. Falling back to OpenCV. If this fails, please change your camera settings.

orowith2os commented 2 years ago

Alternative arch builds, if possible, would be appreciated (mainly just one for aarch64, so I can have the package be multiarch over on Flathub). AFAIK Ubuntu provides OCI images for aarch64 as well.

you-win commented 2 years ago

@orowith2os Unfortunately that's not how GitHub actions work. According to this answer, all machines provided by GitHub actions are x86_64 (aka the most common desktop architecture). A self-hosted aarch64 runner could be used, but who would pay for it?

It does look like it's possible to create a custom docker image for Ubuntu aarch64 and run it on Ubuntu x86_64, but I've never tried doing this.

Perhaps you could put together a Docker image + Dockerfile and verify that running the make_exe.sh script works in that container?

orowith2os commented 2 years ago

Docker supports using qemu to emulate other architectures, so maybe it could work? I'll give your script a try on my aarch64 machine and see how well that works, worst case.

emilianavt commented 2 years ago

The missing binary errors with make_exe.bat are likely due to the VC++ runtime DLLs, which are not part of the repository. You would have to copy them in first. They (and the dshowcapture and escapi DLLs, which are in the repository) also seem to be missing from the example windows build, which is what causes the facetracker to fall back to opencv. It's a pretty big issue.

I also noticed that the produced artifacts are in effect tar bombs, that unpack to the current directory rather than creating a subfolder with everything inside.

you-win commented 2 years ago

Updated to pull the missing VC++ DLLs into the build. GitHub's artifact uploader unpacks folders, so a workaround has been added as well. So the most recent build here

Tracking seems to work as expected in my application.

Now the only remaining error is:

2022-09-10 17:39:55.4725446 [W:onnxruntime:Default, onnxruntime_pybind_state.cc:1622 onnxruntime::python::CreateInferencePybindStateModule] Init provider bridge failed.
Camera configuration: 640x360 416666 400
Final camera configuration: 640x360 24
Format: 0 Internal format: 400
Camera: "HD Pro Webcam C920" Capability ID: None Resolution: 640x360 Frame rate: 24 Colorspace: 400 Internal: 400 Flipped: False
Got frame
... omitted ...
emilianavt commented 2 years ago

Thank you, that part looks good now! I just realized that my own release archive also suffers from being a tar bomb. Oops.

The Init provider bridge failed. sounds like loading the pyd part of onnxruntime failed somehow, but checking the dependencies, everything seems to be in place.

you-win commented 2 years ago

Looks like the 2 places where this could happen in onnxruntime are:

  1. orttraining_python_module.cc#L328
  2. onnxruntime_pybind_state.cc#L1619 <-- The error that currently occurs

Both cases fail when calling InitProvidersSharedLibrary(). Looks like there might be another DLL that's missing, but there's nothing else that I'm missing from make_exe.bat

emilianavt commented 2 years ago

Looking at this, it seems that a DLL that is part of the onnxruntime package might not be included properly by pyinstaller. Presumably, this additional file was introduced with some newer version of onnxruntime, so I didn't notice any issues with my older build.

you-win commented 2 years ago

@emilianavt Good catch, that indeed was the issue.

Problem

onnxruntime has a dll at onnxruntime/capi/onnxruntime_providers_shared.dll that isn't automatically picked up by pyinstaller.

Solution

I've modified make_exe.bat to add that specific binary and place it under the correct directory. That batch file also now uses a venv so the path to the dll is deterministic. Debug echos have also been added.

I've tested this to work locally and via GitHub workflow. Test runs can be found here


I still have not tested to Linux build but, since the MacOS build works, I'm confident enough that this works for Mac/Linux since they use the same build script.

you-win commented 2 years ago

Actually, hold off on merging. Looks like the Linux and Mac builds also need that shared library copied

you-win commented 2 years ago

@emilianavt Updated for Linux/Mac. Confirmed to run properly on all platforms with one slight note.


On MacOS, the executable bits are stripped out by the GitHub artifact uploader when there are files in subdirectories, which is every single file since we want to avoid a tarbomb.

I'm not sure why this happens and setting the bit in the pipeline doesn't make a difference. A manual fix is to run (assuming the files are extracted into a folder called OpenSeeFace) chmod +x OpenSeeFace/*.

emilianavt commented 2 years ago

That looks very good! Regarding the executable bit, I'm not seeing it in the Ubuntu build either. Perhaps the fact that the files are stored in the zip file rather than a tar archive is the cause? In any case, I believe it should only be set on the facetracker file.

you-win commented 2 years ago

Perhaps the fact that the files are stored in the zip file rather than a tar archive is the cause?

Github automatically packs the files into a zip file when downloading, so there's not much I can do there.

I guess I could pack everything into a tar, and then the user would have to unzip + untar the binary?

In any case, I believe it should only be set on the facetracker file.

Running pyinstaller on Mac/Linux sets the executable bit on every single file. You are correct, the executable bit should only be set on facetracker but I was trying to follow how pyinstaller works on mac/linux.

Should I change the logic to only set the executable bit on facetracker ?

emilianavt commented 2 years ago

Github automatically packs the files into a zip file when downloading, so there's not much I can do there.

I guess I could pack everything into a tar, and then the user would have to unzip + untar the binary?

That might actually be more intuitive for novice users than having to set the executable bit. This also seems to recommend making a tar to keep permissions of files.

Running pyinstaller on Mac/Linux sets the executable bit on every single file. You are correct, the executable bit should only be set on facetracker but I was trying to follow how pyinstaller works on mac/linux.

Should I change the logic to only set the executable bit on facetracker ?

That's somewhat unpleasant behaviour by pyinstaller so changing it would be nice. I think something like find OpenSeeFace/ -type f -exec chmod -x {} + should get rid of the flags on all files as a first step.

you-win commented 2 years ago

Updated to:

  1. Remove the unneeded execution bits and only make facetracker executable on Unix
  2. Pack files into a tar
  3. Split Windows and Unix builds since the logic has diverged

Resulting binaries. I've personally tested the Windows and Mac versions. Prior Linux versions also worked and the Linux version uses the same build scripts as the Mac version.


The only remaining items now are:

  1. Build triggers - Should this be built when a specific tag is pushed or only when manually triggered?
  2. Artifact names - Currently named windows-latest, ubuntu-latest, and macos-latest, but this can be changed if desired.
emilianavt commented 2 years ago

The ubuntu build also works nicely on my system.

Having the build trigger when pushing a tag would be handy. For the names, I think OpenSeeFace-windows-latest etc. would be good. Perhaps a timestamp (like date '+%Y%m%d%H%M%S') instead of latest? Although I guess it could look a little messy since all the artifacts would probably get a different timestamp, but it would make keeping them around after downloading easier.

you-win commented 2 years ago

Updated to add datetimes to each artifact. Resulting binaries here. I split the logic for adding datetimes out into a standalone action so it's a bit cleaner.

emilianavt commented 2 years ago

Looks good to me! I'd be happy to merge it now unless there is anything left you'd like to change?

you-win commented 2 years ago

Nothing left on my end, thanks for being patient and working through this with me!

emilianavt commented 2 years ago

Thank you very much for the contribution!