cpal icon indicating copy to clipboard operation
cpal copied to clipboard

PulseAudio support

Open colinmarc opened this issue 10 months ago • 17 comments

Fixes #259.

I finally got around to this after promising to PR it in #259 a year ago. :sweat_smile:

This is based on https://github.com/colinmarc/pulseaudio-rs/pull/2, which obviously needs to land before this. However, I developed both PRs in parallel. If you're feeling generous and would like to review that as well, I would welcome any feedback.

The first commit is unrelated and a bit opinionated, but it seemed nicer. Let me know if I should move that to a separate PR or just drop it.

colinmarc avatar Feb 27 '25 15:02 colinmarc

@colinmarc ~How does this PR differ from #938? Or were both created independent of each other?~ Disregard this, I just misread the titles :)

jacksongoode avatar Jun 05 '25 10:06 jacksongoode

@colinmarc How does this PR differ from #938? Or were both created independent of each other?

Pipewire and Pulseaudio are completely different protocols. Pulseaudio is the established linux audio server, and Pipewire is the new hotness. This PR implements the Pulseaudio protocol, while the other PR implements the Pipewire protocol.

The other useful thing to know is that Pipewire (the server) supports the Pulse protocol as a first-class thing, and that this library has been tested with both audio servers. That means merging this would be enough to handle both cases. The PA server does not support the Pipewire protocol.

Finally, this is just my opinion, but I think the Pipewire protocol is also significantly more complicated.

colinmarc avatar Jun 05 '25 10:06 colinmarc

I actually misread the issue and would have removed my comment if you weren't so quick to respond! 🤣

The other useful thing to know is that Pipewire (the server) supports the Pulse protocol as a first-class thing, and that this library has been tested with both audio servers. That means merging this would be enough to handle both cases. The PA server does not support the Pipewire protocol.

Right, since Pipewire could just interpret cpal through its PulseAudio interface. Thank you for the explanation :)

jacksongoode avatar Jun 05 '25 10:06 jacksongoode

@colinmarc new maintainer here and doing backlog grooming. So sorry this did not get picked up before, because it seems very worthwhile! Would you be so kind to resolve the conflicts so we can pick it up again?

roderickvd avatar Jul 29 '25 20:07 roderickvd

Great :) Just rebased and tests look good. Let me know if you want the first change as a separate PR (or feel free to just drop it).

colinmarc avatar Aug 02 '25 10:08 colinmarc

Wow, amazing turnaround time! 👍

Coming weeks I don't have access to a machine to test it myself, which, as much I believe you 😉 I would like to do. So for now I'm going to trigger an AI review - hope it's going to bring more value than hallucinations - and take some time for a code review over some days. This is a big contribution.

Let me know if you want the first change as a separate PR (or feel free to just drop it).

Yes, that'd be good if you could extract it.

roderickvd avatar Aug 03 '25 21:08 roderickvd

Amazing pull request, but just one comment:

in src/host/pulseaudio/mod.rs it looks like the app name is hardcoded to "cpal-pulseaudio":

        let client =
            pulseaudio::Client::from_env(c"cpal-pulseaudio").map_err(|_| HostUnavailable)?;

This means apps in the volume mixer will all show up as cpal-pulseaudio, when actually you'd want them to have their own name. It would be cool if I'm able to set this as well as other meta-data like the stream description.

narodnik avatar Aug 22 '25 11:08 narodnik

Amazing pull request, but just one comment:

in src/host/pulseaudio/mod.rs it looks like the app name is hardcoded to "cpal-pulseaudio":

        let client =
            pulseaudio::Client::from_env(c"cpal-pulseaudio").map_err(|_| HostUnavailable)?;

This means apps in the volume mixer will all show up as cpal-pulseaudio, when actually you'd want them to have their own name. It would be cool if I'm able to set this as well as other meta-data like the stream description.

Thanks - where should I pull that from? I don't see a way to parameterize that on the generic host API.

colinmarc avatar Aug 22 '25 12:08 colinmarc

Hey @colinmarc, I didn't go through the code in any detail, but I gave your branch a quick test and it does work in my application. Pretty cool!

jwagner avatar Aug 22 '25 20:08 jwagner

Cool, I rebased and added some fixes.

Yes, that'd be good if you could extract it.

:point_right: #1004 :point_left:

colinmarc avatar Aug 24 '25 11:08 colinmarc

My application now sometimes hangs when running it with the pulseaudio host.

#1  0x0000555555ce1717 in std::thread::park ()
#2  0x00005555558779dd in std::thread::local::LocalKey<T>::with ()
#3  0x000055555586e62d in futures_executor::local_pool::block_on ()
#4  0x00005555558720e1 in <cpal::host::pulseaudio::stream::Stream as cpal::traits::StreamTrait>::play ()
#5  0x0000555555868fd0 in <cpal::platform::platform_impl::Stream as cpal::traits::StreamTrait>::play ()

Is as much useful information as I can share right now, I wasn't able to reproduce it outside of my application yet. I also don't know what causes it.

jwagner avatar Aug 26 '25 10:08 jwagner

My application now sometimes hangs when running it with the pulseaudio host.

#1  0x0000555555ce1717 in std::thread::park ()
#2  0x00005555558779dd in std::thread::local::LocalKey<T>::with ()
#3  0x000055555586e62d in futures_executor::local_pool::block_on ()
#4  0x00005555558720e1 in <cpal::host::pulseaudio::stream::Stream as cpal::traits::StreamTrait>::play ()
#5  0x0000555555868fd0 in <cpal::platform::platform_impl::Stream as cpal::traits::StreamTrait>::play ()

Is as much useful information as I can share right now, I wasn't able to reproduce it outside of my application yet. I also don't know what causes it.

Please run in debug and share the source, if possible. As it stands there's no way for me to know whether it's a bug in this PR, pulsaudio-rs, or your app.

colinmarc avatar Aug 26 '25 11:08 colinmarc

Thanks for the quick reply. I managed to get it to happen with a debug binary, I can't share the code of the application in which it happens but I'll try to reproduce it with standalone code or one of the examples but I can't get to it right now. Just wanted to let you know that there might be something.

#0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x00005555573982a7 in std::sys::pal::unix::futex::futex_wait () at library/std/src/sys/pal/unix/futex.rs:72
#2  std::sys::sync::thread_parking::futex::Parker::park () at library/std/src/sys/sync/thread_parking/futex.rs:55
#3  std::thread::Thread::park () at library/std/src/thread/mod.rs:1446
#4  std::thread::park () at library/std/src/thread/mod.rs:1083
#5  0x0000555556699507 in futures_executor::local_pool::run_executor::{closure#0}<core::result::Result<(), pulseaudio::client::ClientError>, futures_executor::local_pool::block_on::{closure_env#0}<pulseaudio::client::record_stream::{impl#1}::started::{async_fn_env#0}>> (thread_notify=0x7ffff7d3ef98)
    at /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/futures-executor-0.3.31/src/local_pool.rs:99
#6  0x00005555566a2be4 in std::thread::local::LocalKey<alloc::sync::Arc<futures_executor::local_pool::ThreadNotify, alloc::alloc::Global>>::try_with<alloc::sync::Arc<futures_executor::local_pool::ThreadNotify, alloc::alloc::Global>, futures_executor::local_pool::run_executor::{closure_env#0}<core::result::Result<(), pulseaudio::client::ClientError>, futures_executor::local_pool::block_on::{closure_env#0}<pulseaudio::client::record_stream::{impl#1}::started::{async_fn_env#0}>>, core::result::Result<(), pulseaudio::client::ClientError>> (self=0x555557a330c0, f=...)
    at /usr/local/rustup/toolchains/1.89.0-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/local.rs:315
#7  0x00005555566a2333 in std::thread::local::LocalKey<alloc::sync::Arc<futures_executor::local_pool::ThreadNotify, alloc::alloc::Global>>::with<alloc::sync::Arc<futures_executor::local_pool::ThreadNotify, alloc::alloc::Global>, futures_executor::local_pool::run_executor::{closure_env#0}<core::result::Result<(), pulseaudio::client::ClientError>, futures_executor::local_pool::block_on::{closure_env#0}<pulseaudio::client::record_stream::{impl#1}::started::{async_fn_env#0}>>, core::result::Result<(), pulseaudio::client::ClientError>> (self=0x555557a330c0, f=...)
    at /usr/local/rustup/toolchains/1.89.0-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/local.rs:279
#8  0x000055555669907c in futures_executor::local_pool::run_executor<core::result::Result<(), pulseaudio::client::ClientError>, futures_executor::local_pool::block_on::{closure_env#0}<pulseaudio::client::record_stream::{impl#1}::started::{async_fn_env#0}>> (f=...)
    at /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/futures-executor-0.3.31/src/local_pool.rs:86
#9  0x0000555556699b0a in futures_executor::local_pool::block_on<pulseaudio::client::record_stream::{impl#1}::started::{async_fn_env#0}> (f=...)
    at /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/futures-executor-0.3.31/src/local_pool.rs:316
#10 0x000055555668b4a4 in cpal::host::pulseaudio::stream::{impl#0}::play (self=0x7ffffffee2f8) at src/host/pulseaudio/stream.rs:31
#11 0x00005555566a42f2 in cpal::platform::platform_impl::{impl#10}::play (self=0x7ffffffee2f0) at src/platform/mod.rs:490

rust log output

[2025-08-26T11:17:16Z DEBUG my code] building input stream F32, StreamConfig { channels: 2, sample_rate: SampleRate(48000), buffer_size: Fixed(512) } 11
// call to build_input_stream
[2025-08-26T11:17:16Z DEBUG pulseaudio::client::reactor] CLIENT [1026]: CreateRecordStream(RecordStreamParams { sample_spec: SampleSpec { format: Float32Le, channels: 2, sample_rate: 48000 }, channel_map: [FrontLeft, FrontRight], source_index: Some(71), source_name: None, buffer_attr: BufferAttr { max_length: 4096, target_length: 4096, pre_buffering: 4294967295, minimum_request_length: 4294967295, fragment_size: 4294967295 }, flags: StreamFlags { start_corked: true, no_remap_channels: false, no_remix_channels: false, fix_format: false, fix_rate: false, fix_channels: false, no_move: false, variable_rate: false, peak_detect: false, start_muted: None, adjust_latency: false, early_requests: false, no_inhibit_auto_suspend: false, fail_on_suspend: false, relative_volume: false, passthrough: false }, direct_on_input_index: None, cvolume: None, props: {}, formats: [] })
[2025-08-26T11:17:16Z DEBUG pulseaudio::client::reactor] SERVER [1026]: Reply
[New Thread 0x7fffdd7fa6c0 (LWP 75906)]
[2025-08-26T11:17:16Z DEBUG my code] initalizing stream
// call to play() which never returns
[2025-08-26T11:17:16Z DEBUG pulseaudio::client::reactor] CLIENT [1027]: GetRecordLatency(LatencyParams { channel: 0, now: SystemTime { tv_sec: 1756207036, tv_nsec: 697746000 } })
[2025-08-26T11:17:16Z DEBUG pulseaudio::client::reactor] CLIENT [1028]: CorkRecordStream(CorkStreamParams { channel: 0, cork: false })
[2025-08-26T11:17:16Z DEBUG pulseaudio::client::reactor] SERVER [1027]: Reply
[2025-08-26T11:17:16Z DEBUG pulseaudio::client::reactor] SERVER [1028]: Reply
[2025-08-26T11:17:16Z DEBUG pulseaudio::client::reactor] CLIENT [1029]: GetRecordLatency(LatencyParams { channel: 0, now: SystemTime { tv_sec: 1756207036, tv_nsec: 806186135 } })
[2025-08-26T11:17:16Z DEBUG pulseaudio::client::reactor] SERVER [1029]: Reply
[2025-08-26T11:17:16Z DEBUG pulseaudio::client::reactor] CLIENT [1030]: GetRecordLatency(LatencyParams { channel: 0, now: SystemTime { tv_sec: 1756207036, tv_nsec: 915999910 } })
[2025-08-26T11:17:16Z DEBUG pulseaudio::client::reactor] SERVER [1030]: Reply
[2025-08-26T11:17:17Z DEBUG pulseaudio::client::reactor] CLIENT [1031]: GetRecordLatency(LatencyParams { channel: 0, now: SystemTime { tv_sec: 1756207037, tv_nsec: 25555174 } })
[2025-08-26T11:17:17Z DEBUG pulseaudio::client::reactor] SERVER [1031]: Reply
[2025-08-26T11:17:17Z DEBUG pulseaudio::client::reactor] CLIENT [1032]: GetRecordLatency(LatencyParams { channel: 0, now: SystemTime { tv_sec: 1756207037, tv_nsec: 135355354 } })
...keeps on polling GetRecordLatency

I can more or less reliably reproduce the issue in my application now by switching between devices. I tried adapting the record_wav sample to get the same result but failed so far.

jwagner avatar Aug 26 '25 11:08 jwagner

That was maddening to isolate but I can finally reproduce it outside of my application. The issue seems to happen when alsa and pulse audio devices exist at the same time, even if only one device has an active stream.

Here is a crudely modified version of record_wav to reproduce the issue: https://github.com/jwagner/cpal/blob/reproduce-pulseaudio-freeze/examples/record_wav.rs

I think arguably the main issue here is that the alsa device keeps an open handle around when it is not needed and not in the pulse integration. Even though it would be really nice if that failed with an error or timeout when the device is locked.

Not keeping the devices around has it's issues too. As far as I know there is no reliable way to refer to a device in cpal other than keeping a reference to it. The only alternative seems to be to find the device by name, but I don't think there is any guarantee that the names are unique.

jwagner avatar Aug 26 '25 22:08 jwagner

Pushed fixes for the documentation issues you brought up in review.

For the hang @jwagner diagnosed (thank you for chasing that down!), we would need to add a timeout to cpal for waiting for the stream to start. I'm not sure what's an appropriate value for that timeout, since devices can be all sorts of things, even over a network. From pulseaudio-rs's perspective, we sent the uncork command, and the started() future resolves when we get the first bytes from the recording device. That never happens, and there's no further information at all from the pulse daemon - no error or anything to relay back. The daemon probably stuck on a lock itself.

On the other hand, the issue is a bit niche, and, as @jwagner said, probably a bug on the ALSA side. Most people will use either pulse or ALSA, not both. So we could just open an issue for someone else to take a look at in the future, and merge this as-is.

colinmarc avatar Oct 04 '25 13:10 colinmarc