Add pipewire audio backend #1610
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds a PipeWire audio backend to provide native support for the modern PipeWire audio server on Linux systems, offering low-latency audio playback as an alternative to existing backends.
- Implements a complete PipeWire audio sink with proper stream management and error handling
- Adds feature flags and dependency configuration for the new backend
- Integrates the backend into the existing audio backend selection system
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| playback/src/audio_backend/pipewire.rs | Complete PipeWire sink implementation with audio format conversion and stream handling |
| playback/src/audio_backend/mod.rs | Registers the PipeWire backend in the available backends list |
| playback/Cargo.toml | Adds pipewire and libspa dependencies for the playback crate |
| Cargo.toml | Defines the pipewire-backend feature and workspace-level dependencies |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| for &byte in data { | ||
| sender.send(byte).map_err(|_| PipeWireError::NotConnected)?; | ||
| } |
There was a problem hiding this comment.
Sending audio data byte-by-byte through a channel is highly inefficient and will likely cause audio dropouts. Consider sending data in chunks or using a different approach like a ring buffer for better performance.
Copilot uses AI. Check for mistakes.
| F32 => spa::param::audio::AudioFormat::F32LE, | ||
| S32 => spa::param::audio::AudioFormat::S32LE, | ||
| S24 => spa::param::audio::AudioFormat::S24LE, | ||
| S24_3 => spa::param::audio::AudioFormat::S24_32LE, |
There was a problem hiding this comment.
The audio format mapping for S24_3 appears incorrect. S24_3 typically represents 24-bit samples packed in 3 bytes, but S24_32LE represents 24-bit samples in 32-bit containers. This should likely be S24_3LE if available.
| S24_3 => spa::param::audio::AudioFormat::S24_32LE, | |
| S24_3 => spa::param::audio::AudioFormat::S24_3LE, |
Copilot uses AI. Check for mistakes.
| for i in 0..total_bytes { | ||
| match receiver.try_recv() { | ||
| Ok(byte) => { | ||
| slice[i] = byte; | ||
| bytes_read += 1; | ||
| } | ||
| Err(_) => break, | ||
| } | ||
| } |
There was a problem hiding this comment.
Reading audio data byte-by-byte in the audio callback will cause significant performance issues and audio dropouts. The audio callback should be lock-free and operate on larger chunks of data.
Copilot uses AI. Check for mistakes.
| self.initialized = true; | ||
|
|
||
| // Give the thread a moment to initialize | ||
| thread::sleep(std::time::Duration::from_millis(100)); |
There was a problem hiding this comment.
Using a fixed sleep duration for initialization is unreliable. Consider using proper synchronization mechanisms like condition variables or status callbacks to ensure the PipeWire stream is actually ready before returning.
Copilot uses AI. Check for mistakes.
| // Wait for the thread to finish with a timeout | ||
| if let Some(handle) = self._main_loop_handle.take() { | ||
| // Give it a moment to exit gracefully | ||
| thread::sleep(std::time::Duration::from_millis(100)); |
There was a problem hiding this comment.
Using a fixed sleep duration before joining the thread is unreliable. The thread should signal when it's ready to exit, or use a timeout with the join operation.
Copilot uses AI. Check for mistakes.
Cargo.toml
Outdated
| ] } | ||
| url = "2.2" | ||
|
|
||
| # Dependencies for pipewire |
There was a problem hiding this comment.
These should go to playback/Cargo.toml.
|
Hi Roderick, |
|
First a bit of a low-effort but hopefully high-impact reply: does looking at RustAudio/cpal#938 help? That implementation is not perfect, but the use of In your fork I see that you're pushing a Hope this helps. |
For your consideration : My implementation of a native pipewire audio-backend.