bastibe / python-soundfile

SoundFile is an audio library based on libsndfile, CFFI, and NumPy
BSD 3-Clause "New" or "Revised" License
717 stars 111 forks source link

Add Linux ARM64 (aarch64) wheel support #415

Open aoirint opened 1 year ago

aoirint commented 1 year ago

This pull request is taking over #382. Thanks @imWildCat ( https://github.com/bastibe/python-soundfile/pull/382#issuecomment-1774979298 )!

Add Linux ARM64 (aarch64) wheel to build_wheels.py and fix some platform dependent code.

Here is the main points of fixes.

With only this pull request, pytest is not passed due to an issue of libsndfile_arm64.so distributed in bastibe/libsndfile-binaries. Please see the GitHub Issue below.

Currently, this pull request does not update the submodule _soundfile_data to keep the reference for bastibe's repository. Updating submodule after https://github.com/bastibe/libsndfile-binaries/issues/29 fixed is required for working properly either before or after merging this pull request.

Here is an experimental ARM64 (aarch64) binary wheel build using my own libsndfile_arm64.so binary ( https://github.com/bastibe/libsndfile-binaries/issues/29 fixed one). This binary successfully passes pytest.

Full log of pytest using the experimental wheel (successfully passed) ```shell $ python -m pytest ================================================= test session starts ================================================== platform linux -- Python 3.11.6, pytest-7.4.2, pluggy-1.3.0 rootdir: /code collected 324 items tests/test_argspec.py .... [ 1%] tests/test_soundfile.py ........................................................................................ [ 28%] ................................................................................................................ [ 62%] ................................................................................................................ [ 97%] ........ [100%] =================================================== warnings summary =================================================== tests/test_soundfile.py::test_file_truthiness[obj] /home/user/.local/lib/python3.11/site-packages/_pytest/unraisableexception.py:78: PytestUnraisableExceptionWarning: Exception ignored from cffi callback .vio_write at 0x5518f63ec0>: None Traceback (most recent call last): File "/home/user/.local/lib/python3.11/site-packages/_pytest/fixtures.py", line 596, in _get_active_fixturedef return self._fixture_defs[argname] ~~~~~~~~~~~~~~~~~~^^^^^^^^^ KeyError: 'request' During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/user/.local/lib/python3.11/site-packages/_pytest/fixtures.py", line 599, in _get_active_fixturedef fixturedef = self._getnextfixturedef(argname) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/user/.local/lib/python3.11/site-packages/_pytest/fixtures.py", line 473, in _getnextfixturedef raise FixtureLookupError(argname, self) _pytest.fixtures.FixtureLookupError: ('request', >) During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/code/soundfile.py", line 1261, in vio_write written = file.write(data) ^^^^^^^^^^^^^^^^ ValueError: I/O operation on closed file warnings.warn(pytest.PytestUnraisableExceptionWarning(msg)) -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html ============================================ 324 passed, 1 warning in 3.49s ============================================ ```

If you are using Docker, you may test my ARM64 (aarch64) wheel build/fix on x86_64 machine with QEMU CPU emulation.

Commands - Docker Hub: [arm64v8/python](https://hub.docker.com/r/arm64v8/python) ```shell # On host machine git clone https://github.com/aoirint/python-soundfile.git cd python-soundfile git checkout 5f2f5b37cd210005f0510f3d11cd9e4e0efa06e4 git submodule update --init sudo docker run --rm -it --volume ".:/code" --platform "linux/arm64" arm64v8/python:3.11 bash ``` ```shell # As root user in Docker container useradd --uid 1000 --create-home user su -l user -s /bin/bash ``` ```shell # As general user in Docker container cd /code # If you want to build your own wheel python build_wheels.py # Please replace the wheel file name for your own wheel pip install ./dist/soundfile-0.12.2.dev2+aoirint.addlinuxarm64-py2.py3-none-manylinux_2_17_aarch64.whl pip install numpy pytest python -m pytest ```
bastibe commented 1 year ago

Awesome to see progress on this! The changes so far look entirely reasonable.

Are you working on the aarch64 build scripts over in libsndfile-binaries as well?

aoirint commented 1 year ago

Are you working on the aarch64 build scripts over in libsndfile-binaries as well?

We don't need any new build script for libsndfile-binaries. It has already been merged in the pull request below.

It seems you just need to run the GitHub Actions workflow manually, then upload/push the Linux ARM64 build artifact libsndfile.so as libsndfile_arm64.so into bastibe/libsndfile-binaries (overwrite existing libsndfile_arm64.so).

image_concat

image_005

bastibe commented 1 year ago

Thank you for the heads-up! I probably won't have time to look into this in the next few days, but next weekend or so looks promising.

bastibe commented 11 months ago

I have just updated libsndfile-binaries to the newest version of libsndfile, which should include support for Vorbis in ARM64 in #423.

However, in order to test these things, we'd need a test suite that runs uses the precompiled binaries, in addition to the platform-supplied libsndfile. Would you, by any chance, like to contribute such a test suite? I'm afraid I don't have enough time to do that in my spare time at the moment.

aoirint commented 11 months ago

Thanks for creating the updated shared libraries.

Would you, by any chance, like to contribute such a test suite?

OK. I will try work on it.

Also, python-soundfile is not currently tested for the ARM64 platform. We should test this if possible.

However, it is complicated to test ARM64 on GitHub Actions because there are only the x64-native CI runners. We might need to add a separated CI job instead of adding arm64 to strategy.matrix.architecture.

We have some options. If you don't mind, I prefer to use Docker because it is easier to implement for me.

Example implementation using Docker:

docker run --name "emulator" --volume ".:/code" --platform "linux/arm64" --detach arm64v8/python:3.11 tail -f
docker exec "emulator" useradd --create-home user
docker exec "emulator" apt-get update
docker exec "emulator" apt-get install -y libsndfile1
docker exec -u user "emulator" pip install numpy pytest cffi>=1.0
docker exec -u user "emulator" python soundfile_build.py
docker exec -u user -w /code "emulator" pip install --editable . --verbose
docker exec -u user -w /code "emulator" python -m pytest .
bastibe commented 11 months ago

OK. I will try work on it.

Thank you very much! This is highly appreciated!

Also, python-soundfile is not currently tested for the ARM64 platform. We should test this if possible.

Indeed, that is a problem. The use of docker for this is absolutely fine. Thank you for bringing this up, I wasn't aware of that option. Do you want to address this as part of this PR, or in a new PR?

Out of curiosity, does it run faster than the pguyot/arm-runner-action@v2-action? IIRC, that build step was very slow compared to the others in libsndfile-binaries.