Replaces CFFI bindings by pybind11 bindings

[fakufaku]

This PR replaces the CFFI bindings that require a local copy of the dynamic libsamplerate library by pybind11 bindings that are compiled statically with the libsamplerate library.

Together with github actions this allows to easily create binary wheels for a range of python versions on macOS and windows. For Linux and other architectures, all that is required is a C++14 compiler to be available and pip can then compile the wheel locally.

• original discussion

• proof of concept can be installed from pypi

  pip install samplerate2
  

Pros

• No need to distribute libsamplerate dynamic library together with the package.
• No need to install libsamplerate separately
• Smaller code footprint
• Automatic compilation of libsamplerate together with the python bindings using github actions

Cons

• Binary wheels need to be compiled for every version of python separately (this may change after python 3.12 if we switch from pybind11 to nanobind that will start supporting Py_LIMITED_API)
• Sometimes need to fiddle with github actions to get compilation working on all platforms/versions

Changes compared to current version

• Adds attributes channels and converter_type to the CallbackResampler class so that it mirrors Resampler
• For single channel signals arrays of shape (n_samples,) and (n_samples, 1) can be used interchangeably. This is very useful to have a uniform interface for any number of channels.
• Adds extra tests

TODO

• Review
• Update README (part about implementation, CFFI, etc)
• Check the doc builds and is consistent
• (won't fix) For some reason, the build is failing on win/py3.7 (it seems pip can't delete the temporary folder created by cmake). Python 3.7 is end-of-life so does it make sense to spend energy to fix it? I removed it from the github action.
• Edit the github action with the correct username/password for automatic deployment to pypi, or add corresponding secrets

[tuxu]

Thanks a lot, this looks like a great step forward. I left a couple of first comments here and there. Some are nitpicks – not blocking this PR, but should be sorted out eventually.

I will be off for vacation and might not be able to respond within the next week.

[fakufaku]

@tuxu Thanks for the feedback, that's great! I was also on holiday and I will now get back onto this.

[fakufaku]

@tuxu I have updated the doc, docstrings, and the README. Could you please give it a look when you have time?

[tuxu]

@fakufaku This looks great.

I added a commit which fixes the build of the documentation and another one which passes the package version into the CMake build. I don't see any bigger blockers right now.

[tuxu]

I was unsure about the lifetimes of the data at outbuf.ptr here and elsewhere in the code. However, it turned out this creates not a view of the data but a copy instead. This is safe, but it would be a good future optimization to avoid that copy.

NB: Running the tests with address sanitizers (-fsanitize-address) passed without issues 🎉

[fakufaku]

I think that we can replace this by

output.resize(out_shape);

As I understand it, it should avoid the copy.
Also, it is still the same object, so there should not be any data ownership problem.
Here is the definition of the resize function (source).

void resize(ShapeContainer new_shape, bool refcheck = true) {
        detail::npy_api::PyArray_Dims d
            = {// Use reinterpret_cast for PyPy on Windows (remove if fixed, checked on 7.3.1)
               reinterpret_cast<Py_intptr_t *>(new_shape->data()),
               int(new_shape->size())};
        // try to resize, set ordering param to -1 cause it's not used anyway
        auto new_array = reinterpret_steal<object>(
            detail::npy_api::get().PyArray_Resize_(m_ptr, &d, int(refcheck), -1));
        if (!new_array) {
            throw error_already_set();
        }
        if (isinstance<array>(new_array)) {
            *this = std::move(new_array);
        }
    }

I have also created a test to trigger this case (only in resample) and it looks like it works.
I have checked also that the copy does not happen by comparing the value of the pointer to the buffer before and after resize (looks like they are the same).
Is there a more principled way of doing that ?

[fakufaku]

@tuxu Did you have time to take a look at this? Is anything holding us back from merging and making a release?

[ideoforms]

I verified that this refactor works on my M1 MacBook Pro, both x86_64 and arm64 versions of Python 3.12. @tuxu, it would be awesome if this fix could be merged and published when you have time! Thanks for all your work on this.

[tuxu]

@fakufaku @ideoforms Really sorry for the delay. Nothing holding us back from merging. I'll try to get this done today.

[tuxu]

Thanks, @fakufaku for the great work! The new release is live: https://pypi.org/project/samplerate/0.2.0/#files

[fakufaku]

Woot 🎉 Thank you for the help and feedback on the code! Happy I could help!

[ideoforms]

Nice one @tuxu @fakufaku!

I've just tested out the pypi-published version 0.2.0 on a couple of laptops here.

Python 3.9 on Intel macOS Sonoma
pip couldn't locate a compatible wheel (as older versions of pip on macOS don't recognise binary versions beyond cpXX-cpXX-macosx_10.x - I think that's this issue), but succeeded in automatically building and installing, and test ran fine ✅

Python 3.11 on Intel macOS Sonoma
pip successfully located and installed a wheel (samplerate-0.2.0-cp311-cp311-macosx_12_0_universal2.whl), and test ran fine ✅

Python 3.9 on Apple M1 macOS Ventura
pip couldn't locate a compatible wheel (as no wheels appear to exist for Python 3.9 / macOS arm64), but succeeded in automatically building and installing, and test ran fine ✅

Python 3.11 on Apple M1 macOS Ventura
pip located and installed a wheel (samplerate-0.2.0-cp311-cp311-macosx_12_0_universal2.whl). However, the .so contained in the wheel did not run. 🚫

ImportError: dlopen(/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/samplerate.cpython-311-darwin.so, 0x0002): tried: '/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/samplerate.cpython-311-darwin.so' (mach-o file, but is an incompatible architecture (have 'x86_64', need 'arm64')), '/System/Volumes/Preboot/Cryptexes/OS/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/samplerate.cpython-311-darwin.so' (no such file), '/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/samplerate.cpython-311-darwin.so' (mach-o file, but is an incompatible architecture (have 'x86_64', need 'arm64'))

I downloaded the wheel manually, and confirmed that, indeed, the .so in the "universal" wheel isn't really universal, but only contains an x86_64 slice.

(1641)(mbp2022:Downloads)$ unzip samplerate-0.2.0-cp311-cp311-macosx_12_0_universal2.whl
Archive:  samplerate-0.2.0-cp311-cp311-macosx_12_0_universal2.whl
  inflating: samplerate.cpython-311-darwin.so
  inflating: samplerate-0.2.0.dist-info/LICENSE.rst
  inflating: samplerate-0.2.0.dist-info/METADATA
  inflating: samplerate-0.2.0.dist-info/WHEEL
  inflating: samplerate-0.2.0.dist-info/top_level.txt
  inflating: samplerate-0.2.0.dist-info/RECORD
(1641)(mbp2022:Downloads)$ file samplerate.cpython-311-darwin.so
samplerate.cpython-311-darwin.so: Mach-O 64-bit bundle x86_64

@tuxu Maybe this is an issue with GitHub Actions' build for Mac arm64?

[tuxu]

@ideoforms Thanks for the feedback, and you're correct, the universal wheels are sadly broken for arm64. I guess this happens when CMake is called from Python. Will have a look. The GitHub runners are x86_64, so arm64 needs cross compilation.

[shauneccles]

If you need an M1 runner I can recommend flyCI, they provide a pretty good amount of CI time for OSS projects for free.

https://www.flyci.net/

(Not affiliated, I just use them with LedFx to test on apple silicon, we use sample rate for audio processing)

[tuxu]

@shauneccles Thanks for the tip! I'll check that out. Running tests on arm64 would be nice to have.

@ideoforms Meanwhile I found the problem with the universal2 wheels (see #12). I removed the broken wheels from PyPI. Better no wheels than broken ones.