Implement `Send` for `Stream`

[timstr]

Currently, Stream does not implement Send, and consequently, I am not allowed to move it from one thread to another in safe Rust. When I want to associate a stream with another struct that is moved between threads, I find myself coming up with the akward workaround of creating an additional thread just to create and store the Stream local to the thread:

// NOTE: Stream is not Send, using a dedicated thread as a workaround
std::thread::spawn(move || {
    let stream = device
        .build_input_stream(&config, data_callback, |err| {
            panic!("CPAL Input stream encountered an error: {}", err);
        })
        .unwrap();
    stream.play().unwrap();
    // synchronization...
    // --- snip ---
    stream.pause().unwrap();
});

It's my understanding that CPAL and its audio backends will create dedicated threads already for audio processing, so this overhead seems wasteful and unecessary. It would be much simpler and more efficient if Stream was Send as well, and the following could simply work:

let stream = device
    .build_input_stream(&config, data_callback, |err| {
        panic!("CPAL Input stream encountered an error: {}", err);
    })
    .unwrap();
stream.play().unwrap();

std::thread::spawn(move || {
    // synchronization...
    // --- snip ---
    stream.pause().unwrap();
});

To be clear, I am not asking that Stream be Sync, since it seems clear that a Stream shouldn't be mutated from multiple threads at the same time without additional synchronization.

Browsing the CPAL source, I find this comment at src/platform/mod.rs

/// The `Stream` implementation associated with the platform's dynamically dispatched
/// [`Host`] type.
// Streams cannot be `Send` or `Sync` if we plan to support Android's AAudio API. This is
// because the stream API is not thread-safe, and the API prohibits calling certain
// functions within the callback.
//
// TODO: Confirm this and add more specific detail and references.
#[must_use = "If the stream is not stored it will not play."]
pub struct Stream(StreamInner, crate::platform::NotSendSyncAcrossAllPlatforms);

I'm not personally familair with Android development, nor do I have an Android device to test on, but the Android AAudio docs on thread safety say (emphasis mine):

The AAudio API is not completely thread safe. You cannot call some of the AAudio functions concurrently from more than one thread at a time. This is because AAudio avoids using mutexes, which can cause thread preemption and glitches.

To be safe, don't call AAudioStream_waitForStateChange() or read or write to the same stream from two different threads. Similarly, don't close a stream in one thread while reading or writing to it in another thread.

To me, this explanation clearly rules out an AAudio backend being Sync, but I don't see anything here preventing the use of AAudio from separate threads at separate times. So at least superficially, it seems like an AAudio backend could indeed be Send. It would be superb if somebody with access to an Android device would be willing to look into this and determine whether an AAudio backend for CPAL can be made Send and still work correctly. If I understand correctly, it would enable all Stream types to be Send and thus make CPAL much more ergonomic to use across threads on all platforms, not just Android.

Thanks for considering.

[marcomaida94]

Same problem here, having Send would be great

[bushrat011899]

To me, this explanation clearly rules out an AAudio backend being Sync, but I don't see anything here preventing the use of AAudio from separate threads at separate times. So at least superficially, it seems like an AAudio backend could indeed be Send.

As an alternative, could features be used to conditionally implement Send for platforms where it is known to be safe to do so? Then this feature will be missing from Android until someone tries it (and presumably) has the prerequisite knowledge to be able to contribute.

That seems to me the easiest path forward, and there is precedent in other Rust crates to do something similar. For example, the latest Bevy release conditionally implements Send for some of its futures when not on WASM.

[0xpr03]

So if all you would need is a Mutex, if you want to support Android. And the stream exists only as a guard to drop the callback (you don't invoke anything after creating the stream - all in the same future => no concurrency).. Then that means you can just

struct SendStream(cpal::Stream)
// they see me Send-ing, they hatin'
unsafe impl Send for SendStream {}

And suddenly this can all be driven from a tokio spawned future (futures::stream..).

[the-drunk-coder]

Not sure whether this belongs here, but it seems like StreamInner is Send on some platforms (Linux-Jack) but not others (MacOs-CoreAudio) ...

[cdellacqua]

I came across this issue while using CPAL on one of my projects. Creating a dedicated thread is what I did too, in combination with a Condvar to make it lightweight and to avoid the need to panic!.

Since the solution felt quite general, I ended up extracting it from my original project. Feel free to copy the code if it fits your needs:

resource_daemon.rs

Example usage with a CPAL stream

[brian-armstrong]

I am also happy to see Stream implement Send. It looks like the change is coming -- is there an upcoming version bump that we can target to refer to cleanly? Thanks for the great work on cpal!

[DAIMENGEN]

I didn't see the latest version of cpal on crates.io. When will the new version be released?

[edwloef]

Not sure whether this belongs here, but it seems like StreamInner is Send on some platforms (Linux-Jack) but not others (MacOs-CoreAudio) ...

can confirm

[roderickvd]

PR to fix that on CoreAudio: #1021