Joren/qbz
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(player): one Direct engine variant for ALSA and WASAPI

Browse Source 
PlaybackEngine::AlsaDirect and StreamType::AlsaDirect become Direct, carrying
Arc<dyn DirectSink> instead of Arc<AlsaDirectStream>. Eleven match arms and
seven sites keep working untouched because they only ever called trait
methods; duplicating them per platform would have been the alternative.

CORRECTION to e61cd7bd8's message: it claimed the ALSA path still
monomorphises to static dispatch. That was true when written and is NOT true
now - the payload is a trait object, so those calls are virtual. Measured
where it lands: write_f32 runs once per 8192-frame chunk, i.e. 5.4 indirect
calls/second at 44.1 kHz and 23.4 at 192 kHz, each immediately entering
blocking device I/O. channels() once at construction, sample_rate/stop/drain
at boundaries. Nothing in the sample path.

Two things Windows CANNOT compile-check, found by reading rather than by the
compiler, and both would have broken the Linux build:

  - Four cfg(linux) arms passed a matched Arc<dyn DirectSink> to
    new_alsa_direct, which takes the concrete type. Arc unsizes one way only.
  - set_hardware_volume is called inside a cfg(linux) block and was not on the
    trait. It is now, with a default that REFUSES rather than silently
    succeeding, overridden by ALSA on Linux only - a sink with no mixer must
    not pretend it moved the volume.

StreamType::Direct is also no longer cfg(linux). Gating it was what stopped
this compiler from checking any arm that handles it, which is the exact blind
spot this port has to work around; the variant now compiles everywhere and
Windows will refuse anything I get wrong in it.

The log prefix moved onto the sink (DirectSink::log_label) rather than being
passed in: four call sites hard-coding "ALSA Direct Engine" is four chances to
mislabel a WASAPI stream. play/pause/stop/set_volume use it too - previously
only the writer thread did, so a WASAPI stream would have logged ALSA
lifecycle lines.

Sol reviewed as the Linux compiler before this commit and found no Linux type
error; the stale references and hard-coded labels above are its findings.

cc
This commit is contained in:
vicrodh committed 2026-08-27 11:11:49 -06:00
1 parent e61cd7bd8a
commit 2aa8d2d5f1
5 files changed
+101 -49
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/qbz-audio/src/alsa_direct.rs
+10
View File
@@ -1059,4 +1059,14 @@ impl crate::backend::DirectSink for AlsaDirectStream {
fn channels(&self) -> u16 {
AlsaDirectStream::channels(self)
}
fn log_label(&self) -> &'static str {
"ALSA Direct Engine"
}
/// Linux only, because the inherent method is: the non-Linux stub of this
/// type has no mixer at all, and there the trait default refusal is the
/// honest answer.
#[cfg(target_os = "linux")]
fn set_hardware_volume(&self, volume: f32) -> Result<(), String> {
AlsaDirectStream::set_hardware_volume(self, volume)
}
}
crates/qbz-audio/src/backend.rs
+16
View File
@@ -1003,4 +1003,20 @@ pub trait DirectSink: Send + Sync + 'static {
fn stop(&self) -> Result<(), String>;
fn sample_rate(&self) -> u32;
fn channels(&self) -> u16;
/// Log prefix for this sink. On the trait rather than a parameter so a
/// call site cannot label an ALSA stream "WASAPI" or the reverse - which
/// is exactly what a hard-coded label at four call sites invites.
fn log_label(&self) -> &'static str;
/// Device-side volume, where the sink has one.
///
/// The default REFUSES rather than silently succeeding: a sink without a
/// hardware mixer must not pretend it moved the volume. Every call site is
/// already gated on the engine's `hardware_volume` flag, which is false for
/// any such sink - permanently so for WASAPI exclusive, where the phase B
/// contract rules out ISimpleAudioVolume.
fn set_hardware_volume(&self, _volume: f32) -> Result<(), String> {
Err("this sink has no hardware volume".to_string())
}
}
crates/qbz-audio/src/wasapi_direct.rs
+8 -4
View File
@@ -759,11 +759,12 @@ mod imp {
// Partial period. Zero-padding keeps the samples we DO have
// instead of dropping them, which a silent buffer would.
buf.resize(need, 0);
// Same reasoning: the last period of a drain is partial by
// definition.
if draining.is_none() && !starved {
// Counted EVERY time, unlike the fully-empty case: a partial
// period is a discrete glitch that just happened, whereas an
// empty queue is one starvation that lasts. Same reasoning as
// above for why a drain's last partial period is not a fault.
if draining.is_none() {
underruns.fetch_add(1, Ordering::Relaxed);
starved = true;
}
}
if render.write_to_device(n, &buf, None).is_err() {
@@ -1066,4 +1067,7 @@ impl crate::backend::DirectSink for WasapiDirectStream {
fn channels(&self) -> u16 {
WasapiDirectStream::channels(self)
}
fn log_label(&self) -> &'static str {
"WASAPI Direct Engine"
}
}
crates/qbz-player/src/player/mod.rs
+20 -18
View File
@@ -544,8 +544,14 @@ enum StreamType {
#[cfg(target_os = "macos")]
exclusive_guard: Option<qbz_audio::CoreAudioExclusiveGuard>,
},
#[cfg(target_os = "linux")]
AlsaDirect(Arc<qbz_audio::AlsaDirectStream>),
/// Any bit-perfect DIRECT sink: ALSA hw: on Linux, WASAPI exclusive on
/// Windows. One variant rather than one per platform, because everything
/// downstream treats them identically - see `qbz_audio::backend::DirectSink`.
///
/// NOT cfg-gated: gating it to Linux is what made the Windows compiler
/// unable to check any of the arms that handle it, which is the whole
/// blind spot this port has to work around.
Direct(Arc<dyn qbz_audio::backend::DirectSink>),
/// Native JACK output (#263 Tier 3). QBZ as a JACK client with stable ports;
/// NOT bit-perfect (resampled to the graph rate).
#[cfg(target_os = "linux")]
@@ -898,7 +904,7 @@ fn try_init_stream_with_backend(
return Some(result.map(|(stream, mode)| {
log::info!("ALSA Direct stream created with mode: {:?}", mode);
state.set_bit_perfect_mode(Some(mode));
StreamType::AlsaDirect(Arc::new(stream))
StreamType::Direct(Arc::new(stream))
}));
}
}
@@ -2220,8 +2226,7 @@ impl Player {
}
}
}
#[cfg(target_os = "linux")]
StreamType::AlsaDirect(alsa_stream) => {
StreamType::Direct(alsa_stream) => {
*consecutive_sink_failures = 0;
thread_state.set_stream_error(false);
let hardware_volume = thread_settings
@@ -2229,7 +2234,7 @@ impl Player {
.ok()
.map(|s| s.alsa_hardware_volume)
.unwrap_or(false);
PlaybackEngine::new_alsa_direct(
PlaybackEngine::new_direct(
alsa_stream.clone(),
hardware_volume,
)
@@ -2666,14 +2671,13 @@ impl Player {
}
}
}
#[cfg(target_os = "linux")]
StreamType::AlsaDirect(alsa_stream) => {
StreamType::Direct(alsa_stream) => {
let hardware_volume = thread_settings
.lock()
.ok()
.map(|s| s.alsa_hardware_volume)
.unwrap_or(false);
PlaybackEngine::new_alsa_direct(
PlaybackEngine::new_direct(
alsa_stream.clone(),
hardware_volume,
)
@@ -3012,7 +3016,7 @@ impl Player {
carrier,
device
);
*stream_opt = Some(StreamType::AlsaDirect(stream.clone()));
*stream_opt = Some(StreamType::Direct(stream.clone()));
thread_state.set_current_device(Some(device));
*current_track_sample_rate = Some(carrier);
*current_track_channels = Some(2);
@@ -3134,7 +3138,7 @@ impl Player {
if little_endian { "LE" } else { "BE" },
device
);
*stream_opt = Some(StreamType::AlsaDirect(stream.clone()));
*stream_opt = Some(StreamType::Direct(stream.clone()));
thread_state.set_current_device(Some(device));
*current_track_sample_rate = Some(rate);
*current_track_channels = Some(2);
@@ -3364,17 +3368,16 @@ impl Player {
}
}
}
#[cfg(target_os = "linux")]
StreamType::AlsaDirect(alsa_stream) => {
StreamType::Direct(alsa_stream) => {
let hardware_volume = thread_settings
.lock()
.ok()
.map(|s| s.alsa_hardware_volume)
.unwrap_or(false);
PlaybackEngine::new_alsa_direct(
PlaybackEngine::new_direct(
alsa_stream.clone(),
hardware_volume,
)
)
}
#[cfg(target_os = "linux")]
StreamType::Jack(jack_stream) => {
@@ -3614,14 +3617,13 @@ impl Player {
}
}
}
#[cfg(target_os = "linux")]
StreamType::AlsaDirect(alsa_stream) => {
StreamType::Direct(alsa_stream) => {
let hardware_volume = thread_settings
.lock()
.ok()
.map(|s| s.alsa_hardware_volume)
.unwrap_or(false);
PlaybackEngine::new_alsa_direct(
PlaybackEngine::new_direct(
alsa_stream.clone(),
hardware_volume,
)
crates/qbz-player/src/player/playback_engine.rs
+47 -27
View File
@@ -8,6 +8,7 @@
//! to enable gapless playback. When one source ends, the next is picked up
//! seamlessly without interrupting the PCM stream.
#[cfg(target_os = "linux")]
use qbz_audio::AlsaDirectStream;
#[cfg(target_os = "linux")]
use qbz_audio::JackStream;
@@ -74,9 +75,14 @@ impl<S> SourceQueue<S> {
pub enum PlaybackEngine {
/// Rodio-based (PipeWire, Pulse, ALSA via CPAL)
Rodio { sink: RodioPlayer },
/// Direct ALSA (hw: devices, bit-perfect) with gapless source queue
AlsaDirect {
stream: Arc<AlsaDirectStream>,
/// A bit-perfect DIRECT sink with a gapless source queue: ALSA hw: on
/// Linux, WASAPI exclusive on Windows. The two are interchangeable here -
/// the writer thread only ever calls `DirectSink`.
Direct {
stream: Arc<dyn qbz_audio::backend::DirectSink>,
/// Log prefix for THIS sink. Without it `play`/`pause`/`stop`/
/// `set_volume` printed "ALSA Direct Engine" for a WASAPI stream.
label: &'static str,
is_playing: Arc<AtomicBool>,
should_stop: Arc<AtomicBool>,
position_frames: Arc<AtomicU64>,
@@ -129,7 +135,13 @@ impl PlaybackEngine {
/// Create ALSA Direct engine with gapless source queue.
/// Spawns a single writer thread that lives for the engine's lifetime.
pub fn new_alsa_direct(stream: Arc<AlsaDirectStream>, hardware_volume: bool) -> Self {
/// Takes ANY direct sink. The log prefix comes from the SINK, so no call
/// site can mislabel it.
pub fn new_direct(
stream: Arc<dyn qbz_audio::backend::DirectSink>,
hardware_volume: bool,
) -> Self {
let label = stream.log_label();
let is_playing = Arc::new(AtomicBool::new(false));
let should_stop = Arc::new(AtomicBool::new(false));
let position_frames = Arc::new(AtomicU64::new(0));
@@ -158,13 +170,14 @@ impl PlaybackEngine {
queue_c,
transition_c,
channels,
"ALSA Direct Engine",
label,
);
})
};
Self::AlsaDirect {
Self::Direct {
stream,
label,
is_playing,
should_stop,
position_frames,
@@ -303,7 +316,8 @@ impl PlaybackEngine {
sink.append(source);
Ok(())
}
Self::AlsaDirect {
Self::Direct {
label,
is_playing,
should_stop,
position_frames,
@@ -323,9 +337,9 @@ impl PlaybackEngine {
should_stop.store(false, Ordering::SeqCst);
source_transition.store(false, Ordering::SeqCst);
is_playing.store(true, Ordering::SeqCst);
log::info!("[ALSA Direct Engine] First source queued, playback starting");
log::info!("[{label}] First source queued, playback starting");
} else {
log::info!("[ALSA Direct Engine] Source queued for gapless transition");
log::info!("[{label}] Source queued for gapless transition");
}
Ok(())
@@ -372,8 +386,10 @@ impl PlaybackEngine {
pub fn play(&self) {
match self {
Self::Rodio { sink } => sink.play(),
Self::AlsaDirect { is_playing, .. } => {
log::info!("[ALSA Direct Engine] Resume requested");
Self::Direct {
label, is_playing, ..
} => {
log::info!("[{label}] Resume requested");
is_playing.store(true, Ordering::SeqCst);
}
#[cfg(target_os = "linux")]
@@ -393,8 +409,10 @@ impl PlaybackEngine {
pub fn pause(&self) {
match self {
Self::Rodio { sink } => sink.pause(),
Self::AlsaDirect { is_playing, .. } => {
log::info!("[ALSA Direct Engine] Pause requested");
Self::Direct {
label, is_playing, ..
} => {
log::info!("[{label}] Pause requested");
is_playing.store(false, Ordering::SeqCst);
}
#[cfg(target_os = "linux")]
@@ -425,8 +443,9 @@ impl PlaybackEngine {
Self::Rodio { sink } => {
sink.stop();
}
Self::AlsaDirect {
Self::Direct {
stream,
label,
is_playing,
should_stop,
playback_thread,
@@ -435,7 +454,7 @@ impl PlaybackEngine {
if should_stop.load(Ordering::SeqCst) {
return; // Already stopped
}
log::info!("[ALSA Direct Engine] Stop requested");
log::info!("[{label}] Stop requested");
should_stop.store(true, Ordering::SeqCst);
is_playing.store(false, Ordering::SeqCst);
@@ -444,7 +463,7 @@ impl PlaybackEngine {
}
if let Err(e) = stream.stop() {
log::warn!("[ALSA Direct Engine] Stop failed: {}", e);
log::warn!("[{label}] Stop failed: {}", e);
}
}
#[cfg(target_os = "linux")]
@@ -493,8 +512,9 @@ impl PlaybackEngine {
pub fn set_volume(&self, volume: f32) {
match self {
Self::Rodio { sink } => sink.set_volume(volume),
Self::AlsaDirect {
Self::Direct {
stream,
label,
hardware_volume,
..
} => {
@@ -502,12 +522,12 @@ impl PlaybackEngine {
#[cfg(target_os = "linux")]
{
if let Err(e) = stream.set_hardware_volume(volume) {
log::warn!("[ALSA Direct Engine] Hardware volume failed: {}", e);
log::warn!("[{label}] Hardware volume failed: {}", e);
}
}
} else {
log::debug!(
"[ALSA Direct Engine] Hardware volume control disabled (use DAC/amplifier)"
"[{label}] Hardware volume control disabled (use DAC/amplifier)"
);
}
}
@@ -530,7 +550,7 @@ impl PlaybackEngine {
pub fn empty(&self) -> bool {
match self {
Self::Rodio { sink } => sink.empty(),
Self::AlsaDirect {
Self::Direct {
is_playing,
source_queue,
..
@@ -555,7 +575,7 @@ impl PlaybackEngine {
pub fn take_source_transition(&self) -> bool {
match self {
Self::Rodio { .. } => false,
Self::AlsaDirect {
Self::Direct {
source_transition, ..
} => source_transition
.compare_exchange(true, false, Ordering::SeqCst, Ordering::SeqCst)
@@ -580,7 +600,7 @@ impl PlaybackEngine {
pub fn position_secs(&self) -> Option<u64> {
match self {
Self::Rodio { .. } => None,
Self::AlsaDirect {
Self::Direct {
position_frames,
stream,
..
@@ -615,7 +635,7 @@ impl PlaybackEngine {
pub fn duration_secs(&self) -> Option<u64> {
match self {
Self::Rodio { .. } => None,
Self::AlsaDirect {
Self::Direct {
duration_frames,
stream,
..
@@ -640,14 +660,14 @@ impl PlaybackEngine {
/// Check if using ALSA Direct engine
#[allow(dead_code)]
pub fn is_alsa_direct(&self) -> bool {
matches!(self, Self::AlsaDirect { .. })
pub fn is_direct(&self) -> bool {
matches!(self, Self::Direct { .. })
}
}
/// Single long-lived writer thread for ALSA Direct.
/// Single long-lived writer thread for any bit-perfect direct sink.
///
/// Continuously reads samples from the current source and writes to ALSA.
/// Continuously reads samples from the current source and writes to the sink.
/// When a source ends, seamlessly picks up the next one from the queue
/// (gapless transition). If no next source is available, drains the ALSA
/// buffer and waits for the next source or a stop signal.