Add pedalboard.io.AudioStream support for Linux

[sarmentow]

Implements #366

[psobot]

@sarmentow thanks for this! My one concern (except for the compilation errors at the moment) is that this code as written will require users to have libasound installed in order to load Pedalboard. As it's often used in server environments where libasound may not be installed, it'd be great to make libasound an optional dependency instead of one that prevents the library from loading. Would you be comfortable trying to make that change?

[sarmentow]

Sure. I'll add the JUCE_ALSA flag and set it to 0 in the compiler flags for MacOS and Windows to avoid including anything ALSA-related (should've done that before, sorry!), but I'm curious as to what should be done in the Linux build.

I'm thinking of adding back the ifdefs you had for preventing compilation of the AudioStream header (and the juce_audio_devices header) and only include these files and link ALSA if some environment variable like LINUX_ENABLE_ALSA is set, similar to how debug builds work (i.e. by setting the DEBUG environment variable).

Tell me if that sounds good and I'll start work on it right away!

[psobot]

Thanks!

I'm thinking of adding back the ifdefs you had for preventing compilation of the AudioStream header (and the juce_audio_devices header) and only include these files and link ALSA if some environment variable like LINUX_ENABLE_ALSA is set, similar to how debug builds work (i.e. by setting the DEBUG environment variable).

This would work if compiling from scratch (which I suppose is better than nothing) but won't enable the feature for pip-installed builds.

If we did want to allow using libasound if it's present but avoid crashing if it's not, the "proper" way would be to dynamically load libasound (i.e.: using dlopen), but then we'd have to rewrite pretty much all 1,300 lines of juce_ALSA_linux.cpp to support shared objects. 😬

[psobot]

D'oh, scratch that - it looks like it is possible to statically link libasound:

Compilation of static library
-----------------------------

If you would like to use the static ALSA library, you need to use these
options for the configure script:

	./configure --enable-shared=no --enable-static=yes

This should work, as long as it doesn't inflate the binary size too much. If we statically link libasound and there are no transient load-time dependencies, then this will "just work" on Linux regardless of if the host system has libasound installed or not. But to do so, we'll have to find a way to include libasound in the build, which might be just as hard as my suggestion above (to use dlopen).

[sarmentow]

Okay, I'll investigate the possibility of statically linking libasound and come back with the results.

[sarmentow]

@psobot I managed to statically link libasound in linux builds by adding the alsa-lib-dev package in the before-all install command in the pyproject.toml.

Locally building cp310-manylinux_x86_64, the size of the wheel is 4.5mb for the 0.9.12 version, versus 4.1mb for the same version listed in PyPI (so ~400kb increase in size).

Do you think that's reasonable?

[psobot]

Awesome, nice work @sarmentow! 400kB sounds totally reasonable to me.

I don't have a Linux box with an audio output handy, but I assume that local build does play audio as expected for you? I think the CI runners don't have audio interfaces at all, so as long as the tests still pass there, I think we can be reasonably certain that this change won't break anything for headless/server-based use cases.

[sarmentow]

Yes, the example using AudioStream works as expected after the changes when installing Pedalboard from the new wheel. I'll commit these changes and wait for CI results.

[sarmentow]

@psobot Pre-build on linux is still failing with the error: 'alsa/asoundlib.h' file not found
😕. I didn't realize that I also need to add the alsa-lib package (in Ubuntu it's under the name libasound2-dev) to the Install Linux Dependencies step in the workflow file. Will commit this change as well.

Additionally, I've encountered an error in the Delete Existing Cache for macOS Objects step on macOS. The error message indicates that a token needs to be configured for the command using the Github CLI. Could you please advise on how to proceed with this? Should I add my own GitHub token as a secret in my fork to resolve this issue, or is there another recommended approach?

[psobot]

@sarmentow Whoops, sorry about that Delete Existing Cache step - it seems that's a conflict between how we set up the build process and some of GitHub's security defaults. I'll remove that step from your PR for now.

[sarmentow]

It appears that now that AudioStream actually loads on Linux, the tests in test_audio_stream.py must be reconsidered. We can simply delete the test_create_stream_fails_on_linux and remove all @pytest.mark.skipif(platform.system() == "Linux", ...) and it should work.

I'll run the tests locally... Could've catched this earlier.

[sarmentow]

I've removed the Linux-specific skips from the AudioStream tests, but encountered errors during local test runs when attempting to create AudioStream objects with certain output devices 🤔. To rule out any potential issues with my local environment, I've verified that the same errors occur when running the tests locally on the master branch. Given this, I'm inclined to believe that the issue is specific to my machine, and I'd like to re-run the tests in CI to confirm.

[psobot]

This is looking almost ready to merge - I'm just going to add one CI step to try installing the resulting Linux wheels on a machine that's missing libasound before running tests, to test that the library is indeed statically linking as we expect.

[sarmentow]

While reading the logs of failed checks and reading on the workflows themselves, we seem to be exceeding the 6 hour time limit, which is causing tests to fail on Linux. I'll look into what may be causing tests taking this long.

Also, the step in which libasound is removed is getting a weird error regarding unmet dependencies of the Java Runtime? Will try looking into that as well.

[sarmentow]

@psobot Great news! I was able to reproduce the bug that's been causing our tests to hang 😊.

The issue arises when there are no audio devices available in Linux environments. In these cases, AudioStream.default_output_device_name and AudioStream.default_input_device_name return empty strings instead of None. Creating an AudioStream with both devices set to None leads to a runtime error - which avoids hanging - however, the code doesn't currently handle empty strings, resulting in an assertion failure in JUCE (juce_AudioDeviceManager.cpp:804) and subsequent hanging.

I'll add this consideration to validate constructor arguments in AudioStream.

There's just one problem... This won't solve CI failures since throwing a RuntimeError on tests isn't a successful outcome. Simply skipping tests by adding RuntimeError into the ACCEPTABLE_ERRORS_IN_CI list in test_audio_stream.py if on Linux doesn't seem adequate in the long term. Maybe create some sort of virtual I/O device before testing?

It'd be great to have your inputs on what should be done in terms of CI tests to ensure AudioStream works on Linux 👍 . I'll gladly work on this.

[sarmentow]

After some reading, I found the snd-dummy driver (see: https://www.alsa-project.org/wiki/Matrix:Module-dummy) which provides a virtual sound card for testing AudioStream on Linux. Now a couple of test devices should show up when testing in CI.

[sarmentow]

@psobot pinging for CI approval 🫶 :

[psobot]

Sorry for the delay @sarmentow! I've removed the failing uninstall libasound2 step that I added, but still want to download one of the .whl files that gets built for Linux (automatically by CI) and want to confirm that it does load as expected on a system without libasound2 installed. I'll make the appropriate changes to get CI passing here.

[psobot]

Thanks for the contribution (and your patience) @sarmentow - I'm cutting a new release for Pedalboard v0.9.14 now, which will include this change.

[psobot]

This is now on PyPI as Pedalboard v0.9.14!

[sarmentow]

@psobot Thank you for all the help and support! This was my first PR in a bigger project, definitely learned a lot. 👍👍