From 79e4baedd01e60651087a7033984fe367430fb1b Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 10:29:38 +1000 Subject: [PATCH 01/21] 4g: bus meters publishing + housekeeping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the last gap before Phase 5's monitor TUI: per-app meter events already publish on the meters topic via the registry watcher; bus-level DSP meters now also publish. 4g — Bus meters headroom_core::meters::BusMetrics is an Arc> snapshot owned by the playback callback (try_lock; skip on contention) and read by the AGC controller on each 50 ms tick. Carries: compressor GR, limiter total/soft/hard GR, true peak. The AGC controller combines these with its ebur128 readings (momentary, short-term, integrated) and the current smoothed AGC target, then publishes a headroom_ipc::MeterTick on Topic::Meters. Publish cadence honours profile.meters.publish_hz, capped at the AGC tick rate (20 Hz). Lower publish_hz throttles to every Nth tick. Mode::I added to the AGC's EbuR128 so loudness_global() is available without a second ebur128 instance. Bounded cost — a histogram walk per call, <=20 Hz. LUFS values are sanitised to a -200.0 dB floor via finite_or_floor() — ebur128 returns -inf (not Err) for "no usable measurement yet," and non-finite f32 can't survive JSON serialisation (serde_json renders as null). Housekeeping shipped alongside headroom-client moved from [dependencies] to [dev-dependencies] in headroom-core — it's only used inside ipc::server's tests. Verified by full clippy + test run; production builds no longer pull it in. Pre-existing clippy nits cleared (limiter.rs x5, app_level.rs, ipc/ops.rs, pw/filter.rs). All field_reassign_with_default or assign_op_pattern in test code; stage-6 commit ran clippy without --all-targets so these slipped through. Verified 178 tests passing (28 dsp + 48 dsp + 20 ipc + 106 core including +2 new meters tests + 4 client). Clippy clean at default level with -D warnings --all-targets. Smoke test: monitor meters subscription receives 20 Hz MeterTick events with the expected JSON shape (all fields finite). --- crates/headroom-core/Cargo.toml | 4 +- crates/headroom-core/src/agc.rs | 206 +++++++++++++++++++------- crates/headroom-core/src/app_level.rs | 2 +- crates/headroom-core/src/ipc/ops.rs | 2 +- crates/headroom-core/src/lib.rs | 1 + crates/headroom-core/src/meters.rs | 88 +++++++++++ crates/headroom-core/src/pw/filter.rs | 35 ++++- crates/headroom-core/src/runtime.rs | 6 +- crates/headroom-dsp/src/limiter.rs | 35 +++-- 9 files changed, 309 insertions(+), 70 deletions(-) create mode 100644 crates/headroom-core/src/meters.rs diff --git a/crates/headroom-core/Cargo.toml b/crates/headroom-core/Cargo.toml index 00a3dc0..f22230c 100644 --- a/crates/headroom-core/Cargo.toml +++ b/crates/headroom-core/Cargo.toml @@ -12,7 +12,6 @@ authors.workspace = true [dependencies] headroom-dsp = { workspace = true } headroom-ipc = { workspace = true } -headroom-client = { workspace = true } # test-only: integration tests serde = { workspace = true } serde_json = { workspace = true } @@ -49,6 +48,9 @@ ebur128 = { workspace = true } [dev-dependencies] criterion = { workspace = true } +# Only used in `ipc::server::tests` to round-trip a real client +# against the spawned IPC server. +headroom-client = { workspace = true } [features] default = [] diff --git a/crates/headroom-core/src/agc.rs b/crates/headroom-core/src/agc.rs index 8fb8f19..354eca5 100644 --- a/crates/headroom-core/src/agc.rs +++ b/crates/headroom-core/src/agc.rs @@ -16,7 +16,9 @@ use std::time::Duration; use ebur128::{EbuR128, Mode}; +use headroom_ipc::{Event, MeterTick, Topic}; +use crate::meters::SharedBusMetrics; use crate::pw::filter::FilterControl; use crate::state::SharedState; @@ -50,8 +52,14 @@ pub struct AgcController { /// enable flag exactly when it changes. last_enabled: bool, /// Last short-term loudness observed; surfaced for status / - /// meters in a future sub-stage. + /// `meters` topic. last_short_term_lufs: f32, + /// Bus-level DSP snapshot written by the filter's playback + /// callback. Used to fill the `MeterTick` payload published on + /// `Topic::Meters`. + bus_metrics: SharedBusMetrics, + /// Tick counter for `publish_hz` throttling. Wraps freely. + meter_tick_counter: u32, } impl AgcController { @@ -66,9 +74,18 @@ impl AgcController { measurement_consumer: rtrb::Consumer, filter_control: FilterControl, daemon: SharedState, + bus_metrics: SharedBusMetrics, ) -> Result { - let ebu = EbuR128::new(channels, sample_rate, Mode::S | Mode::M | Mode::TRUE_PEAK) - .map_err(AgcInitError::from)?; + // `Mode::I` (integrated, gated) costs a histogram walk per + // `loudness_global()` call — bounded, fine at 20 Hz meter + // cadence. Added so the `meters` topic can surface integrated + // LUFS without a second ebur128 instance. + let ebu = EbuR128::new( + channels, + sample_rate, + Mode::S | Mode::M | Mode::I | Mode::TRUE_PEAK, + ) + .map_err(AgcInitError::from)?; Ok(Self { sample_rate, channels, @@ -79,6 +96,8 @@ impl AgcController { smoothed_target_db: 0.0, last_enabled: true, last_short_term_lufs: LOUDNESS_FLOOR_LUFS, + bus_metrics, + meter_tick_counter: 0, }) } @@ -99,26 +118,68 @@ impl AgcController { /// One control-loop iteration. Should be invoked at [`AGC_TICK`] /// cadence by a main-loop timer source. + /// + /// Three things happen here: + /// + /// 1. AGC enable/disable transition is observed and pushed to + /// the audio thread. + /// 2. The measurement ring is drained into `ebur128` and the + /// short-term loudness is cached. This runs **regardless of + /// AGC enabled** so the `meters` topic can keep surfacing LUFS + /// when the user has only enabled the compressor / limiter. + /// 3. If AGC is enabled, a smoothed target gain is computed and + /// pushed to the audio thread. + /// 4. Bus-level meters are published on `Topic::Meters` honouring + /// `profile.meters.publish_hz`. pub fn tick(&mut self) { - // Snapshot the AGC section out from under the daemon lock. - // Hold the lock only long enough to clone the small config. - let cfg = { + // Snapshot what we need out from under the daemon lock. Hold + // the lock only long enough to clone the small config. + let (cfg, publish_hz) = { let s = self.daemon.lock(); - s.profiles.effective().agc.clone() + let p = s.profiles.effective(); + (p.agc.clone(), p.meters.publish_hz) }; - // React to enable/disable transitions before doing measurement - // work — flipping off should stop pushing target updates and - // tell the audio thread to unwind back to 0 dB. if cfg.enabled != self.last_enabled { self.filter_control.set_agc_enabled(cfg.enabled); self.last_enabled = cfg.enabled; } - if !cfg.enabled { - return; + + // Drain the measurement ring + feed ebur128 unconditionally. + self.consume_measurements(); + let short_term = finite_or_floor( + self.ebu.loudness_shortterm().map(|v| v as f32).ok(), + ); + self.last_short_term_lufs = short_term; + + if cfg.enabled + && short_term > cfg.silence_threshold_lufs + && short_term.is_finite() + { + let raw_target = cfg.target_lufs - short_term; + let clamped = raw_target.clamp(-cfg.max_cut_db, cfg.max_boost_db); + + // Slow leaky-integrator smoother on the tick cadence. + // attack when target is dropping (gain reduction toward + // the signal), release when target is rising back toward + // unity / boost. + let dt_ms = AGC_TICK.as_secs_f32() * 1000.0; + let alpha = if clamped < self.smoothed_target_db { + alpha_for_dt(cfg.attack_ms, dt_ms) + } else { + alpha_for_dt(cfg.release_ms, dt_ms) + }; + self.smoothed_target_db += alpha * (clamped - self.smoothed_target_db); + self.filter_control + .set_agc_target_db(self.smoothed_target_db); } - // Drain up to TICK_BUF_SAMPLES from the measurement ring. + self.publish_meters(publish_hz); + } + + /// Drain up to [`TICK_BUF_SAMPLES`] from the measurement ring and + /// feed them through `ebur128`. + fn consume_measurements(&mut self) { let mut buf = [0.0_f32; TICK_BUF_SAMPLES]; let mut n = 0; while n < buf.len() { @@ -131,7 +192,7 @@ impl AgcController { } } if n == 0 { - return; // No samples yet (early boot or silence); leave target alone. + return; } // ebur128 wants whole frames; drop any odd trailing sample. let usable = (n / self.channels as usize) * self.channels as usize; @@ -140,39 +201,61 @@ impl AgcController { } if let Err(e) = self.ebu.add_frames_f32(&buf[..usable]) { tracing::warn!(error = %e, "ebur128 add_frames_f32 failed"); + } + } + + /// Publish a `MeterTick` event on `Topic::Meters` if this tick + /// falls on the `publish_hz` cadence. + fn publish_meters(&mut self, publish_hz: f32) { + if !self.should_publish(publish_hz) { return; } + let bus = *self.bus_metrics.lock(); + // `ebur128` returns `-inf` (not `Err`) for "no useful + // measurement yet" — typically early-boot or while the input + // is pure silence. `-inf` can't survive JSON serialisation + // (serde_json renders non-finite f32 as null), so floor here. + let momentary = finite_or_floor( + self.ebu.loudness_momentary().map(|v| v as f32).ok(), + ); + let integrated = finite_or_floor( + self.ebu.loudness_global().map(|v| v as f32).ok(), + ); - let short_term = self - .ebu - .loudness_shortterm() - .map(|v| v as f32) - .unwrap_or(LOUDNESS_FLOOR_LUFS); - self.last_short_term_lufs = short_term; - - // Silence gate: if the program is below the threshold, hold - // the current target. This avoids ramping gain up during - // legitimate quiet passages. - if short_term <= cfg.silence_threshold_lufs || !short_term.is_finite() { - return; - } - - let raw_target = cfg.target_lufs - short_term; - let clamped = raw_target.clamp(-cfg.max_cut_db, cfg.max_boost_db); - - // Slow leaky-integrator smoother on the tick cadence. attack - // when target is dropping (gain reduction toward the signal), - // release when target is rising back toward unity / boost. - let dt_ms = AGC_TICK.as_secs_f32() * 1000.0; - let alpha = if clamped < self.smoothed_target_db { - alpha_for_dt(cfg.attack_ms, dt_ms) - } else { - alpha_for_dt(cfg.release_ms, dt_ms) + let tick = MeterTick { + momentary_lufs: momentary, + shortterm_lufs: self.last_short_term_lufs, + integrated_lufs: integrated, + true_peak_dbtp: bus.true_peak_dbtp, + // Total path GR is additive in log domain. Both values + // are ≤ 0 dB when reducing. + gain_reduction_db: bus.compressor_gr_db + bus.limiter_total_gr_db, + compressor_gr_db: bus.compressor_gr_db, + limiter_gr_db: bus.limiter_total_gr_db, + agc_gain_db: self.smoothed_target_db, }; - self.smoothed_target_db += alpha * (clamped - self.smoothed_target_db); - self.filter_control - .set_agc_target_db(self.smoothed_target_db); + if let Ok(event) = Event::new(Topic::Meters, "tick", &tick) { + self.daemon.lock().broadcaster.publish(Topic::Meters, event); + } + } + + /// Tick-rate gate for the `meters` publish loop. Caps at + /// [`AGC_TICK`]'s native rate (20 Hz) — `publish_hz` above that is + /// silently clamped. + fn should_publish(&mut self, publish_hz: f32) -> bool { + if publish_hz <= 0.0 { + return false; + } + let agc_hz = 1000.0 / AGC_TICK.as_millis() as f32; + if publish_hz >= agc_hz { + self.meter_tick_counter = self.meter_tick_counter.wrapping_add(1); + return true; + } + let skip = (agc_hz / publish_hz).round().max(1.0) as u32; + let now = self.meter_tick_counter; + self.meter_tick_counter = self.meter_tick_counter.wrapping_add(1); + now % skip == 0 } /// Reset the smoothed target and the underlying `ebur128` state. @@ -181,16 +264,31 @@ impl AgcController { pub fn reset(&mut self) { self.smoothed_target_db = 0.0; self.last_short_term_lufs = LOUDNESS_FLOOR_LUFS; - // ebur128 doesn't expose a public reset, so rebuild it. - if let Ok(fresh) = - EbuR128::new(self.channels, self.sample_rate, Mode::S | Mode::M | Mode::TRUE_PEAK) - { + // ebur128 doesn't expose a public reset, so rebuild it. Keep + // the same mode set used in `new()` so meter publishing stays + // consistent. + if let Ok(fresh) = EbuR128::new( + self.channels, + self.sample_rate, + Mode::S | Mode::M | Mode::I | Mode::TRUE_PEAK, + ) { self.ebu = fresh; } self.filter_control.set_agc_target_db(0.0); } } +/// Coerce a possibly-non-finite LUFS measurement into a finite value +/// suitable for serialisation. `-inf` (the `ebur128` "no usable +/// reading" sentinel) and `NaN` both collapse to +/// [`LOUDNESS_FLOOR_LUFS`]. +fn finite_or_floor(v: Option) -> f32 { + match v { + Some(x) if x.is_finite() => x, + _ => LOUDNESS_FLOOR_LUFS, + } +} + /// `tau_ms`-time-constant leaky-integrator alpha for a tick of /// duration `dt_ms`. `1 - exp(-dt / tau)`; clamps to `[0, 1]`. fn alpha_for_dt(tau_ms: f32, dt_ms: f32) -> f32 { @@ -219,6 +317,7 @@ impl From for crate::error::DaemonError { #[cfg(test)] mod tests { use super::*; + use crate::meters; use crate::profile_store::ProfileStore; use crate::pw::filter::{AudioCmd, FilterControl}; use crate::state::{self, DaemonState}; @@ -232,12 +331,15 @@ mod tests { rtrb::Producer, rtrb::Consumer, SharedState, + SharedBusMetrics, ) { let (m_prod, m_cons) = RingBuffer::::new(8192); let (control, cmd_cons) = FilterControl::for_testing(32); let state = state::shared(DaemonState::new(ProfileStore::builtin())); - let agc = AgcController::new(SR, CH, m_cons, control, state.clone()).unwrap(); - (agc, m_prod, cmd_cons, state) + let bus = meters::shared(); + let agc = + AgcController::new(SR, CH, m_cons, control, state.clone(), bus.clone()).unwrap(); + (agc, m_prod, cmd_cons, state, bus) } fn push_silence(prod: &mut rtrb::Producer, frames: usize) { @@ -258,7 +360,7 @@ mod tests { #[test] fn tick_with_no_samples_does_nothing() { - let (mut agc, _prod, mut cmd_cons, _state) = fixture(); + let (mut agc, _prod, mut cmd_cons, _state, _bus) = fixture(); agc.tick(); assert!(cmd_cons.pop().is_err(), "no samples → no target push"); assert_eq!(agc.current_target_db(), 0.0); @@ -266,7 +368,7 @@ mod tests { #[test] fn tick_under_silence_threshold_holds_target() { - let (mut agc, mut prod, mut cmd_cons, _state) = fixture(); + let (mut agc, mut prod, mut cmd_cons, _state, _bus) = fixture(); push_silence(&mut prod, 4800); // 100ms of silence agc.tick(); // ebur128 may report -inf or values below the silence @@ -279,7 +381,7 @@ mod tests { #[test] fn tick_with_audible_signal_pushes_target() { - let (mut agc, mut prod, mut cmd_cons, _state) = fixture(); + let (mut agc, mut prod, mut cmd_cons, _state, _bus) = fixture(); // Pump multiple ticks worth so ebur128's short-term window // (~3 s) starts producing values. for _ in 0..40 { @@ -299,7 +401,7 @@ mod tests { #[test] fn agc_disable_in_profile_flips_audio_thread() { - let (mut agc, _prod, mut cmd_cons, state) = fixture(); + let (mut agc, _prod, mut cmd_cons, state, _bus) = fixture(); // First tick with the default-enabled profile. agc.tick(); // Drain any commands. diff --git a/crates/headroom-core/src/app_level.rs b/crates/headroom-core/src/app_level.rs index a1e2917..57f9e9a 100644 --- a/crates/headroom-core/src/app_level.rs +++ b/crates/headroom-core/src/app_level.rs @@ -369,7 +369,7 @@ mod tests { // Immediately after the write, force a different reduction — // the rate limit must suppress any further write within 100 ms. let t1 = c.last_write_at.unwrap() + Duration::from_millis(10); - c.smoothed_reduction_db = c.smoothed_reduction_db + 6.0; // synthetic kick + c.smoothed_reduction_db += 6.0; // synthetic kick let v = c.process_block(db_to_lin(0.0), db_to_lin(-3.0).powi(2), t1); assert!(v.is_none(), "rate limit should have blocked the follow-up write"); } diff --git a/crates/headroom-core/src/ipc/ops.rs b/crates/headroom-core/src/ipc/ops.rs index 4e1974a..ec1c222 100644 --- a/crates/headroom-core/src/ipc/ops.rs +++ b/crates/headroom-core/src/ipc/ops.rs @@ -505,7 +505,7 @@ mod tests { assert!( body.get("warnings") .and_then(|w| w.as_array()) - .map_or(true, |a| a.is_empty()), + .is_none_or(|a| a.is_empty()), "expected empty/absent warnings on healthy startup" ); } diff --git a/crates/headroom-core/src/lib.rs b/crates/headroom-core/src/lib.rs index 112854e..443018d 100644 --- a/crates/headroom-core/src/lib.rs +++ b/crates/headroom-core/src/lib.rs @@ -17,6 +17,7 @@ pub mod agc; pub mod app_level; pub mod error; pub mod ipc; +pub mod meters; pub mod profile; pub mod profile_store; pub mod profile_watcher; diff --git a/crates/headroom-core/src/meters.rs b/crates/headroom-core/src/meters.rs new file mode 100644 index 0000000..0bbd3bf --- /dev/null +++ b/crates/headroom-core/src/meters.rs @@ -0,0 +1,88 @@ +//! Bus-level meter snapshot shared between the audio thread and the +//! AGC controller. +//! +//! Phase 4g. +//! +//! The audio thread writes [`BusMetrics`] after each +//! `playback_process` call using `try_lock` — it must never block on +//! the lock. The AGC controller reads on its 50 ms tick, combines +//! with `ebur128` readings (momentary / short-term / integrated +//! LUFS) and the current AGC gain target, and publishes a +//! [`headroom_ipc::MeterTick`] on `Topic::Meters` for any IPC client +//! that's subscribed. +//! +//! Per-app meter events (Phase 6e) are a separate stream emitted +//! directly from the registry watcher. The two coexist on the same +//! topic; clients see both kinds and key off the event payload shape +//! to tell them apart. +//! +//! Wait-free on the audio side: a missed write (lock contended for +//! the few nanoseconds the reader holds it) is harmless — the next +//! quantum overwrites the slot. Dropped meter samples don't degrade +//! the AGC; the controller reads the freshest available snapshot. + +use std::sync::Arc; + +use parking_lot::Mutex; + +/// Snapshot of bus-level DSP metrics, written by the audio thread +/// after the AGC → Compressor → Limiter chain runs. +#[derive(Debug, Clone, Copy, Default, PartialEq)] +pub struct BusMetrics { + /// Compressor gain reduction in dB (negative when reducing). + pub compressor_gr_db: f32, + /// Limiter total gain reduction in dB (the min of soft and hard + /// gain, in dB). + pub limiter_total_gr_db: f32, + /// Limiter soft-tier gain reduction in dB. + pub limiter_soft_gr_db: f32, + /// Limiter hard-tier gain reduction in dB. Non-zero only when + /// the soft tier wasn't enough — that's the alarm condition. + pub limiter_hard_gr_db: f32, + /// True peak in dBTP observed by the limiter's per-quantum peak + /// detector. Bounded above by the hard ceiling on the *output*; + /// this field is the peak the limiter *saw on its input*, which + /// is informative for tuning soft-tier headroom. + pub true_peak_dbtp: f32, +} + +/// Cheap-to-clone shared handle. Audio thread + AGC controller each +/// hold a clone. +pub type SharedBusMetrics = Arc>; + +/// Construct an empty shared metrics handle. +#[must_use] +pub fn shared() -> SharedBusMetrics { + Arc::new(Mutex::new(BusMetrics::default())) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_is_all_zero() { + let m = BusMetrics::default(); + assert_eq!(m.compressor_gr_db, 0.0); + assert_eq!(m.limiter_total_gr_db, 0.0); + assert_eq!(m.limiter_soft_gr_db, 0.0); + assert_eq!(m.limiter_hard_gr_db, 0.0); + assert_eq!(m.true_peak_dbtp, 0.0); + } + + #[test] + fn shared_is_cheap_to_clone() { + let a = shared(); + let b = a.clone(); + *a.lock() = BusMetrics { + compressor_gr_db: -3.0, + limiter_total_gr_db: -1.0, + limiter_soft_gr_db: -1.0, + limiter_hard_gr_db: 0.0, + true_peak_dbtp: -0.5, + }; + let snap = *b.lock(); + assert!((snap.compressor_gr_db - -3.0).abs() < 1e-6); + assert!((snap.true_peak_dbtp - -0.5).abs() < 1e-6); + } +} diff --git a/crates/headroom-core/src/pw/filter.rs b/crates/headroom-core/src/pw/filter.rs index 2b3053e..1d654dd 100644 --- a/crates/headroom-core/src/pw/filter.rs +++ b/crates/headroom-core/src/pw/filter.rs @@ -53,6 +53,7 @@ use headroom_dsp::{ }; use crate::error::DaemonError; +use crate::meters::{BusMetrics, SharedBusMetrics}; use crate::pw::sink::NODE_NAME as PROCESSED_SINK_NAME; /// Sample rate the filter operates at. The DSP kernels are @@ -215,6 +216,11 @@ struct PlaybackState { samples_starved: u64, /// Counter of measurement samples dropped (best-effort push). measurement_dropped: u64, + /// Bus-level meter snapshot shared with the AGC controller for + /// meter publication. Audio thread does `try_lock` and skips on + /// contention (which is vanishingly rare — the reader holds the + /// lock for nanoseconds). + bus_metrics: SharedBusMetrics, } /// The filter pipeline. @@ -252,6 +258,10 @@ pub struct FilterBundle { /// Consumer end of the AGC measurement ring. Hand to the /// `headroom-core::agc` controller. pub measurement_consumer: Consumer, + /// Bus-level meter snapshot. The audio thread keeps it fresh on + /// every `playback_process` call; the AGC controller reads it on + /// each tick and publishes a `MeterTick` event. + pub bus_metrics: SharedBusMetrics, } impl Filter { @@ -275,6 +285,7 @@ impl Filter { let control = FilterControl { cmd_producer: Arc::new(Mutex::new(cmd_producer)), }; + let bus_metrics = crate::meters::shared(); let compressor = Compressor::new(init.compressor, FILTER_SAMPLE_RATE as f32); let limiter = Limiter::new(init.limiter, FILTER_SAMPLE_RATE as f32); @@ -302,6 +313,7 @@ impl Filter { limiter, samples_starved: 0, measurement_dropped: 0, + bus_metrics: bus_metrics.clone(), }) .process(playback_process) .register() @@ -350,6 +362,7 @@ impl Filter { }, control, measurement_consumer, + bus_metrics, }) } } @@ -577,6 +590,21 @@ fn playback_process(stream: &pipewire::stream::StreamRef, state: &mut PlaybackSt .saturating_add((starved_frames * CHANNELS as usize) as u64); } + // Snapshot bus-level meter state for the AGC controller. `try_lock` + // so we never block on a daemon-thread reader; a contended quantum + // simply drops this update — the next one along will land. + if produced_frames > 0 { + if let Some(mut metrics) = state.bus_metrics.try_lock() { + *metrics = BusMetrics { + compressor_gr_db: state.compressor.gain_reduction_db(), + limiter_total_gr_db: state.limiter.gain_reduction_db(), + limiter_soft_gr_db: state.limiter.soft_gain_reduction_db(), + limiter_hard_gr_db: state.limiter.hard_gain_reduction_db(), + true_peak_dbtp: state.limiter.true_peak_dbtp(), + }; + } + } + // Tell PipeWire how much we wrote. let chunk = data.chunk_mut(); *chunk.size_mut() = (max_frames * stride_bytes) as u32; @@ -650,8 +678,11 @@ mod tests { let mut compressor = Compressor::new(CompressorConfig::default(), SR); let mut limiter = Limiter::new(LimiterConfig::default(), SR); let mut agc = AgcGain::new(AgcGainConfig::default(), SR); - let mut bad = LimiterConfig::default(); - bad.oversample = 8; // structural; can't apply in place + let bad = LimiterConfig { + // structural; can't apply in place + oversample: 8, + ..LimiterConfig::default() + }; // Should not panic, should not change the limiter. apply_audio_cmd( AudioCmd::SetLimiter(bad), diff --git a/crates/headroom-core/src/runtime.rs b/crates/headroom-core/src/runtime.rs index 6c7c2a5..d88f3b5 100644 --- a/crates/headroom-core/src/runtime.rs +++ b/crates/headroom-core/src/runtime.rs @@ -107,6 +107,7 @@ pub fn run(profiles: ProfileStore) -> Result<(), DaemonError> { filter: _filter, control: filter_control, measurement_consumer, + bus_metrics, } = Filter::create(pw.core(), filter_init)?; daemon_state.lock().filter_control = Some(filter_control.clone()); @@ -114,13 +115,16 @@ pub fn run(profiles: ProfileStore) -> Result<(), DaemonError> { // loop via a timer source; reads the active profile's [agc] // config at each tick (so profile.use takes effect on the next // tick) and pushes a smoothed target_db to the audio thread via - // FilterControl. + // FilterControl. Also publishes `meters` topic ticks at + // `profile.meters.publish_hz` (capped at 20 Hz, the AGC tick + // rate) — 4g. let agc_controller = AgcController::new( crate::pw::filter::FILTER_SAMPLE_RATE, crate::pw::filter::CHANNELS, measurement_consumer, filter_control, daemon_state.clone(), + bus_metrics, ) .map_err(DaemonError::from)?; let agc_controller = Rc::new(RefCell::new(agc_controller)); diff --git a/crates/headroom-dsp/src/limiter.rs b/crates/headroom-dsp/src/limiter.rs index 4fc6939..617f7d2 100644 --- a/crates/headroom-dsp/src/limiter.rs +++ b/crates/headroom-dsp/src/limiter.rs @@ -624,10 +624,12 @@ mod tests { fn try_set_config_applies_scalar_changes() { let sr = 48_000.0; let mut l = Limiter::new(LimiterConfig::default(), sr); - let mut cfg = LimiterConfig::default(); - cfg.ceiling_dbtp = -3.0; - cfg.release_ms = 200.0; - cfg.hold_ms = 10.0; + let cfg = LimiterConfig { + ceiling_dbtp: -3.0, + release_ms: 200.0, + hold_ms: 10.0, + ..LimiterConfig::default() + }; assert_eq!(l.try_set_config(cfg), SetConfigOutcome::Applied); assert!((l.ceiling_dbtp() - -3.0).abs() < 1e-6); let active = l.config(); @@ -640,8 +642,10 @@ mod tests { let sr = 48_000.0; let mut l = Limiter::new(LimiterConfig::default(), sr); // Start with soft on. Disable it. - let mut cfg = LimiterConfig::default(); - cfg.soft = None; + let mut cfg = LimiterConfig { + soft: None, + ..LimiterConfig::default() + }; assert_eq!(l.try_set_config(cfg), SetConfigOutcome::Applied); assert!(l.config().soft.is_none()); assert!(l.effective_soft_ceiling_dbtp().is_none()); @@ -664,8 +668,10 @@ mod tests { fn try_set_config_rejects_oversample_change() { let sr = 48_000.0; let mut l = Limiter::new(LimiterConfig::default(), sr); - let mut cfg = LimiterConfig::default(); - cfg.oversample = 8; + let cfg = LimiterConfig { + oversample: 8, + ..LimiterConfig::default() + }; assert_eq!(l.try_set_config(cfg), SetConfigOutcome::StructuralChange); // Limiter unchanged. assert_eq!(l.config().oversample, LimiterConfig::default().oversample); @@ -675,8 +681,11 @@ mod tests { fn try_set_config_rejects_lookahead_change() { let sr = 48_000.0; let mut l = Limiter::new(LimiterConfig::default(), sr); - let mut cfg = LimiterConfig::default(); - cfg.lookahead_ms = 5.0; // resizes delay + peak buffer + let cfg = LimiterConfig { + // resizes delay + peak buffer + lookahead_ms: 5.0, + ..LimiterConfig::default() + }; assert_eq!(l.try_set_config(cfg), SetConfigOutcome::StructuralChange); } @@ -684,8 +693,10 @@ mod tests { fn try_set_config_rejects_fir_taps_change() { let sr = 48_000.0; let mut l = Limiter::new(LimiterConfig::default(), sr); - let mut cfg = LimiterConfig::default(); - cfg.fir_taps = 63; + let cfg = LimiterConfig { + fir_taps: 63, + ..LimiterConfig::default() + }; assert_eq!(l.try_set_config(cfg), SetConfigOutcome::StructuralChange); } From e528a9841793a73ef606b73b43e29d6225d111de Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 13:35:27 +1000 Subject: [PATCH 02/21] 5: monitor TUI + wire fill-ins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `headroom monitor` becomes a full-screen ratatui TUI by default; the previous behaviour (line-delimited JSON, useful for scripts and tests) is preserved behind --json. 5 — Monitor TUI New `crates/headroom-cli/src/tui.rs` (~700 lines incl. tests). Main thread does subscribe + initial status() + route_list() before entering raw mode, so connect errors surface as clean stderr messages instead of corrupting the terminal. A reader thread owns the headroom_client::Client and forwards each subscription event through a crossbeam channel; an input thread blocks on event::read() and forwards keys (q / Esc / Ctrl-C) through a second channel; the main thread `select!`s both plus a 10 Hz ticker (so uptime + staleness display advance even when no events are flowing). On quit the OS reaps the reader; a CLI tool doesn't need a graceful UnixStream shutdown. Layout: outer block carries the profile / version / uptime in the top-right title and a footer with subscribed topics + an overflow / error / disconnected banner when relevant. Inside: bus DSP gauges (AGC target, compressor GR, limiter GR, true peak), a loudness panel (momentary / short-term / integrated, greyed when stale), and a streams table with route + Layer A reduction column. Wire types caught up to the daemon `headroom-ipc::RoutingEvent` gained `StreamRemoved`, `LayerAAttached`, `LayerADetached` variants — these are events the daemon already publishes (registry.rs §pw) but that weren't typed in the proto. Without `StreamRemoved` the TUI would accumulate departed streams forever; without the Layer A pair the per-stream column couldn't track tap state. New `LayerALevel` struct types the `meters/layer_a_level` payload (node_id, app, volume_lin, reduction_db). `headroom_core::agc::LOUDNESS_FLOOR_LUFS` is now `pub` — it's published as-is in MeterTick.*_lufs fields when ebur128 has no useful measurement yet, so clients need it to render "no measurement" without hard-coding `-200.0`. Toolchain notes ratatui and crossterm pinned to =0.28.1. Newer ratatui pulls in `instability` 0.3.12 + `darling` 0.23 which need rustc 1.88+; the project pins 1.86 via rust-toolchain.toml. Lockfile also pins `instability` to 0.3.7 and `darling` to 0.20.10 (older patches that still build on 1.86). Verified 185 tests passing (was 178: +5 for TUI event mapping + fmt_uptime, +2 for stream_removed / layer_a_level handling). Clippy clean at -D warnings --all-targets. Live smoke: daemon emits routing/{stream_routed, stream_removed, layer_a_attached, layer_a_detached} and meters/{tick, layer_a_level} in shapes that round-trip cleanly through the new typed enums. TUI binary survives raw-mode init + initial RPCs + subscription against a live daemon. Known unrelated daemon gap (to be fixed next): pre-existing streams aren't actually re-linked when the daemon writes target.object — WirePlumber updates metadata but doesn't tear the old link down or create a new one into the processed sink. Bus DSP path therefore sees silence even when status reports route=processed. Not Phase 5; addressed separately. --- Cargo.lock | 380 ++++++++++++++- Cargo.toml | 6 + crates/headroom-cli/Cargo.toml | 3 + crates/headroom-cli/src/main.rs | 64 ++- crates/headroom-cli/src/tui.rs | 810 +++++++++++++++++++++++++++++++ crates/headroom-core/src/agc.rs | 6 +- crates/headroom-ipc/src/lib.rs | 6 +- crates/headroom-ipc/src/proto.rs | 39 ++ 8 files changed, 1283 insertions(+), 31 deletions(-) create mode 100644 crates/headroom-cli/src/tui.rs diff --git a/Cargo.lock b/Cargo.lock index 5d6e4ab..a3283c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,6 +11,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "allocator-api2" +version = "0.2.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" + [[package]] name = "anes" version = "0.1.6" @@ -128,12 +134,27 @@ version = "1.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8efb64bd706a16a1bdde310ae86b351e4d21550d98d056f22f8a7f7a2183fec" +[[package]] +name = "cassowary" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df8670b8c7b9dae1793364eafadf7239c40d669904660c5960d74cfd80b46a53" + [[package]] name = "cast" version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" +[[package]] +name = "castaway" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dec551ab6e7578819132c713a93c022a05d60159dc86e7a7050223577484c55a" +dependencies = [ + "rustversion", +] + [[package]] name = "cc" version = "1.2.62" @@ -253,6 +274,20 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" +[[package]] +name = "compact_str" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b79c4069c6cad78e2e0cdfcbd26275770669fb39fd308a752dc110e83b9af32" +dependencies = [ + "castaway", + "cfg-if", + "itoa", + "rustversion", + "ryu", + "static_assertions", +] + [[package]] name = "convert_case" version = "0.6.0" @@ -320,12 +355,72 @@ version = "0.8.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" +[[package]] +name = "crossterm" +version = "0.28.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "829d955a0bb380ef178a640b91779e3987da38c9aea133b20614cfed8cdea9c6" +dependencies = [ + "bitflags 2.11.1", + "crossterm_winapi", + "mio 1.2.0", + "parking_lot", + "rustix", + "signal-hook", + "signal-hook-mio", + "winapi", +] + +[[package]] +name = "crossterm_winapi" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "acdd7c62a3665c7f6830a51635d9ac9b23ed385797f70a83bb8bafe9c572ab2b" +dependencies = [ + "winapi", +] + [[package]] name = "crunchy" version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "460fbee9c2c2f33933d720630a6a0bac33ba7053db5344fac858d4b8952d77d5" +[[package]] +name = "darling" +version = "0.20.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f63b86c8a8826a49b8c21f08a2d07338eec8d900540f8630dc76284be802989" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.20.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95133861a8032aaea082871032f5815eb9e98cef03fa916ab4500513994df9e5" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "strsim", + "syn", +] + +[[package]] +name = "darling_macro" +version = "0.20.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d336a2a514f6ccccaa3e09b02d41d35330c07ddf03a62165fcec10bb561c7806" +dependencies = [ + "darling_core", + "quote", + "syn", +] + [[package]] name = "dasp_frame" version = "0.11.0" @@ -391,6 +486,18 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + +[[package]] +name = "foldhash" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" + [[package]] name = "fsevent-sys" version = "4.1.0" @@ -505,6 +612,17 @@ dependencies = [ "zerocopy", ] +[[package]] +name = "hashbrown" +version = "0.15.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" +dependencies = [ + "allocator-api2", + "equivalent", + "foldhash", +] + [[package]] name = "hashbrown" version = "0.17.1" @@ -516,9 +634,12 @@ name = "headroom-cli" version = "0.1.0" dependencies = [ "clap", + "crossbeam-channel", + "crossterm", "headroom-client", "headroom-core", "headroom-ipc", + "ratatui", "serde_json", "thiserror 2.0.18", "tracing", @@ -591,6 +712,12 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" +[[package]] +name = "ident_case" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" + [[package]] name = "indexmap" version = "2.14.0" @@ -598,7 +725,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d466e9454f08e4a911e14806c24e16fba1b4c121d1ea474396f396069cf949d9" dependencies = [ "equivalent", - "hashbrown", + "hashbrown 0.17.1", +] + +[[package]] +name = "indoc" +version = "2.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "79cf5c93f93228cf8efb3ba362535fb11199ac548a09ce117c9b1adc3030d706" +dependencies = [ + "rustversion", ] [[package]] @@ -621,6 +757,19 @@ dependencies = [ "libc", ] +[[package]] +name = "instability" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0bf9fed6d91cfb734e7476a06bde8300a1b94e217e1b523b6f0cd1a01998c71d" +dependencies = [ + "darling", + "indoc", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "is-terminal" version = "0.4.17" @@ -656,6 +805,15 @@ dependencies = [ "either", ] +[[package]] +name = "itertools" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.18" @@ -738,6 +896,12 @@ dependencies = [ "system-deps", ] +[[package]] +name = "linux-raw-sys" +version = "0.4.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d26c52dbd32dccf2d10cac7725f8eae5296885fb5703b261f7d0a0739ec807ab" + [[package]] name = "lock_api" version = "0.4.14" @@ -753,6 +917,15 @@ version = "0.4.29" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5e5032e24019045c762d3c0f28f5b6b8bbf38563a65908389bf7978758920897" +[[package]] +name = "lru" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "234cf4f4a04dc1f57e24b96cc0cd600cf2af460d4161ac5ecdd0af8e1f3b2a38" +dependencies = [ + "hashbrown 0.15.5", +] + [[package]] name = "matchers" version = "0.2.0" @@ -786,6 +959,18 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "mio" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "50b7e5b27aa02a74bac8c3f23f448f8d87ff11f92d3aac1a6ed369ee08cc56c1" +dependencies = [ + "libc", + "log", + "wasi", + "windows-sys 0.61.2", +] + [[package]] name = "nix" version = "0.27.1" @@ -821,7 +1006,7 @@ dependencies = [ "kqueue", "libc", "log", - "mio", + "mio 0.8.11", "walkdir", "windows-sys 0.48.0", ] @@ -896,6 +1081,12 @@ dependencies = [ "windows-link", ] +[[package]] +name = "paste" +version = "1.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" + [[package]] name = "pin-project-lite" version = "0.2.17" @@ -954,6 +1145,27 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "ratatui" +version = "0.28.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fdef7f9be5c0122f890d58bdf4d964349ba6a6161f705907526d891efabba57d" +dependencies = [ + "bitflags 2.11.1", + "cassowary", + "compact_str", + "crossterm", + "instability", + "itertools 0.13.0", + "lru", + "paste", + "strum", + "strum_macros", + "unicode-segmentation", + "unicode-truncate", + "unicode-width", +] + [[package]] name = "redox_syscall" version = "0.5.18" @@ -1004,6 +1216,31 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" +[[package]] +name = "rustix" +version = "0.38.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fdb5bc1ae2baa591800df16c9ca78619bf65c0488b41b96ccec5d11220d8c154" +dependencies = [ + "bitflags 2.11.1", + "errno", + "libc", + "linux-raw-sys", + "windows-sys 0.59.0", +] + +[[package]] +name = "rustversion" +version = "1.0.22" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" + +[[package]] +name = "ryu" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" + [[package]] name = "same-file" version = "1.0.6" @@ -1096,6 +1333,17 @@ dependencies = [ "signal-hook-registry", ] +[[package]] +name = "signal-hook-mio" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b75a19a7a740b25bc7944bdee6172368f988763b744e3d4dfe753f6b4ece40cc" +dependencies = [ + "libc", + "mio 1.2.0", + "signal-hook", +] + [[package]] name = "signal-hook-registry" version = "1.4.8" @@ -1118,12 +1366,40 @@ version = "1.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" +[[package]] +name = "static_assertions" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" + [[package]] name = "strsim" version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" +[[package]] +name = "strum" +version = "0.26.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fec0f0aef304996cf250b31b5a10dee7980c85da9d759361292b8bca5a18f06" +dependencies = [ + "strum_macros", +] + +[[package]] +name = "strum_macros" +version = "0.26.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c6bee85a5a24955dc440386795aa378cd9cf82acd5f764469152d2270e581be" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "rustversion", + "syn", +] + [[package]] name = "syn" version = "2.0.117" @@ -1327,6 +1603,17 @@ version = "1.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9629274872b2bfaf8d66f5f15725007f635594914870f65218920345aa11aa8c" +[[package]] +name = "unicode-truncate" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b3644627a5af5fa321c95b9b235a72fd24cd29c648c2c379431e6628655627bf" +dependencies = [ + "itertools 0.13.0", + "unicode-segmentation", + "unicode-width", +] + [[package]] name = "unicode-width" version = "0.1.14" @@ -1410,7 +1697,16 @@ version = "0.48.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" dependencies = [ - "windows-targets", + "windows-targets 0.48.5", +] + +[[package]] +name = "windows-sys" +version = "0.59.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b" +dependencies = [ + "windows-targets 0.52.6", ] [[package]] @@ -1428,13 +1724,29 @@ version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9a2fa6e2155d7247be68c096456083145c183cbbbc2764150dda45a87197940c" dependencies = [ - "windows_aarch64_gnullvm", - "windows_aarch64_msvc", - "windows_i686_gnu", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_gnullvm", - "windows_x86_64_msvc", + "windows_aarch64_gnullvm 0.48.5", + "windows_aarch64_msvc 0.48.5", + "windows_i686_gnu 0.48.5", + "windows_i686_msvc 0.48.5", + "windows_x86_64_gnu 0.48.5", + "windows_x86_64_gnullvm 0.48.5", + "windows_x86_64_msvc 0.48.5", +] + +[[package]] +name = "windows-targets" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" +dependencies = [ + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", + "windows_i686_gnullvm", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", ] [[package]] @@ -1443,42 +1755,90 @@ version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" + [[package]] name = "windows_aarch64_msvc" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" +[[package]] +name = "windows_aarch64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" + [[package]] name = "windows_i686_gnu" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" +[[package]] +name = "windows_i686_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" + +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" + [[package]] name = "windows_i686_msvc" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" +[[package]] +name = "windows_i686_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" + [[package]] name = "windows_x86_64_gnu" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" +[[package]] +name = "windows_x86_64_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" + [[package]] name = "windows_x86_64_gnullvm" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" + [[package]] name = "windows_x86_64_msvc" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" +[[package]] +name = "windows_x86_64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" + [[package]] name = "winnow" version = "0.7.15" diff --git a/Cargo.toml b/Cargo.toml index 61cbdd9..3e86881 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,12 @@ tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt"] } # CLI clap = { version = "4.5", features = ["derive"] } +# TUI (monitor). Pinned to versions whose transitive deps still build +# on the project's pinned rustc 1.86 (newer ratatui pulls +# `instability` 0.3.12 + `darling` 0.23 which need 1.88+). +ratatui = "=0.28.1" +crossterm = "=0.28.1" + # Concurrency / control plane crossbeam-channel = "0.5" parking_lot = "0.12" diff --git a/crates/headroom-cli/Cargo.toml b/crates/headroom-cli/Cargo.toml index d310f42..ee5e7c9 100644 --- a/crates/headroom-cli/Cargo.toml +++ b/crates/headroom-cli/Cargo.toml @@ -19,6 +19,9 @@ headroom-core = { workspace = true } headroom-ipc = { workspace = true } clap = { workspace = true } +crossbeam-channel = { workspace = true } +crossterm = { workspace = true } +ratatui = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } diff --git a/crates/headroom-cli/src/main.rs b/crates/headroom-cli/src/main.rs index 3977755..ae79f21 100644 --- a/crates/headroom-cli/src/main.rs +++ b/crates/headroom-cli/src/main.rs @@ -6,6 +6,8 @@ #![forbid(unsafe_code)] +mod tui; + use std::path::PathBuf; use std::process::ExitCode; @@ -66,12 +68,19 @@ enum Cmd { /// Reload profile files from disk. Reload, - /// Subscribe to event topics and print as line-delimited JSON. + /// Live monitor. Defaults to a full-screen TUI; `--json` falls back + /// to the line-delimited JSON stream that previous versions + /// produced (useful for scripting and tests). Monitor { - /// Topics to subscribe to (comma-separated). - /// Defaults to `meters` if none given. + /// Topics to subscribe to (comma-separated). Only honoured with + /// `--json`; the TUI always subscribes to all four event topics. #[arg(value_delimiter = ',', default_value = "meters")] topics: Vec, + + /// Emit one JSON event per line on stdout instead of drawing + /// the TUI. + #[arg(long)] + json: bool, }, } @@ -170,13 +179,28 @@ fn init_tracing() { fn run() -> Result<(), CliError> { let cli = Cli::parse(); - init_tracing(); + + // TUI takes over the terminal; don't let `tracing` scribble on top + // of it. The JSON-mode monitor also benefits from a quieter stderr. + let tui_mode = matches!(&cli.cmd, Cmd::Monitor { json: false, .. }); + if !tui_mode { + init_tracing(); + } match cli.cmd { Cmd::Daemon => { headroom_core::run().map_err(|e| CliError::Daemon(e.to_string()))?; Ok(()) } + Cmd::Monitor { json: false, .. } => { + // Connect on the main thread so the initial `status` / + // `route.list` round-trips happen before we enter raw mode. + let client = match cli.socket.as_deref() { + Some(p) => Client::connect_at(p)?, + None => Client::connect()?, + }; + tui::run(client).map_err(CliError::Tui) + } cmd => with_client(cli.socket.as_deref(), |c| dispatch(c, cmd)), } } @@ -247,18 +271,23 @@ fn dispatch(client: &mut Client, cmd: Cmd) -> Result<(), CliError> { let reloaded = client.profile_reload()?; println!("reloaded: {reloaded:?}"); } - Cmd::Monitor { topics } => { - let pw_topics: Vec = topics.iter().copied().map(Topic::from).collect(); - client.subscribe(&pw_topics)?; - loop { - let ev = client.next_event()?; - println!( - "{} {}/{} {}", - chrono_like_now(), - ev.topic, - ev.event, - serde_json::to_string(&ev.data)?, - ); + Cmd::Monitor { topics, json } => { + if json { + let pw_topics: Vec = + topics.iter().copied().map(Topic::from).collect(); + client.subscribe(&pw_topics)?; + loop { + let ev = client.next_event()?; + println!( + "{} {}/{} {}", + chrono_like_now(), + ev.topic, + ev.event, + serde_json::to_string(&ev.data)?, + ); + } + } else { + unreachable!("TUI monitor is dispatched before `with_client`") } } } @@ -276,6 +305,9 @@ enum CliError { #[error("json: {0}")] Json(#[from] serde_json::Error), + #[error("tui: {0}")] + Tui(tui::TuiError), + #[error("{0}")] Other(String), } diff --git a/crates/headroom-cli/src/tui.rs b/crates/headroom-cli/src/tui.rs new file mode 100644 index 0000000..600b8a9 --- /dev/null +++ b/crates/headroom-cli/src/tui.rs @@ -0,0 +1,810 @@ +//! `headroom monitor` TUI. Subscribes to `meters`, `routing`, +//! `profile`, and `daemon`, renders bus DSP gauges + loudness + +//! per-stream routing + status header. +//! +//! Architecture: the main thread owns the terminal and the draw loop. +//! A reader thread owns the `Client` and forwards each subscription +//! event over a crossbeam channel. On quit the main thread restores +//! the terminal and exits; the reader thread is reaped by the OS. +//! (A CLI binary doesn't need a graceful reader shutdown — the kernel +//! tears the UnixStream down on process exit.) + +use std::collections::BTreeMap; +use std::io; +use std::thread; +use std::time::{Duration, Instant}; + +use crossbeam_channel::{select, tick, unbounded, Receiver}; +use crossterm::event::{self, Event as CtEvent, KeyCode, KeyEvent, KeyModifiers}; +use headroom_client::{Client, ClientError}; +use headroom_ipc::{ + DaemonEvent, Event, LayerALevel, MeterTick, ProfileEvent, Route, RoutingEvent, Status, + StreamRoute, Topic, +}; +use ratatui::{ + layout::{Alignment, Constraint, Direction, Layout, Rect}, + style::{Color, Modifier, Style, Stylize}, + text::{Line, Span}, + widgets::{Block, Borders, Cell, Gauge, Paragraph, Row, Table}, + Frame, Terminal, +}; + +/// Errors specific to the TUI subcommand. +#[derive(Debug, thiserror::Error)] +pub enum TuiError { + #[error("client: {0}")] + Client(#[from] ClientError), + + #[error("terminal: {0}")] + Io(#[from] io::Error), +} + +/// Entry point — owns the connected client through initial RPCs, then +/// hands it off to the reader thread and enters the draw loop. +pub fn run(mut client: Client) -> Result<(), TuiError> { + // Subscribe + initial state, all on the main thread before the + // terminal goes into raw mode. Any error here bubbles cleanly. + let topics = [Topic::Meters, Topic::Routing, Topic::Profile, Topic::Daemon]; + client.subscribe(&topics)?; + let status = client.status()?; + let route_list = client.route_list()?; + + // Spawn reader. + let (tx, rx) = unbounded::(); + let reader_handle = thread::Builder::new() + .name("headroom-monitor-rx".into()) + .spawn(move || reader_loop(client, tx)) + .map_err(TuiError::Io)?; + + // Terminal up. + let mut terminal = ratatui::init(); + let outcome = draw_loop(&mut terminal, status, route_list, rx); + ratatui::restore(); + + // Detach the reader: process exit (or the dropped channel) will + // tear the connection down. We don't need its result. + drop(reader_handle); + + outcome +} + +// --------------------------------------------------------------------------- +// Reader thread +// --------------------------------------------------------------------------- + +enum Msg { + Event(Event), + Disconnected(String), +} + +fn reader_loop(mut client: Client, tx: crossbeam_channel::Sender) { + loop { + match client.next_event() { + Ok(ev) => { + if tx.send(Msg::Event(ev)).is_err() { + return; + } + } + Err(e) => { + let _ = tx.send(Msg::Disconnected(e.to_string())); + return; + } + } + } +} + +// --------------------------------------------------------------------------- +// State +// --------------------------------------------------------------------------- + +struct UiState { + daemon_version: String, + profile: String, + bypass: bool, + /// Daemon uptime as of connect, plus our local elapsed. + base_uptime_s: u64, + connected_at: Instant, + default_route: Route, + streams: BTreeMap, + /// Per-stream Layer A state. Presence = tap attached; the inner + /// `Option` is the latest smoothed reduction in dB (None + /// until the first `meters/layer_a_level` event arrives). + layer_a: BTreeMap>, + meters: Option, + /// Wall-clock instant the last meter tick arrived. Used to show + /// staleness if the audio thread stops feeding the AGC. + last_meter_at: Option, + overflow_total: u64, + last_error: Option, + disconnected: Option, +} + +impl UiState { + fn new(status: Status, route_list: headroom_ipc::RouteList) -> Self { + let mut streams = BTreeMap::new(); + for s in route_list.current { + streams.insert(s.node_id, s); + } + // Streams reported on `status` superset; merge. + for s in status.streams.iter() { + streams.entry(s.node_id).or_insert_with(|| s.clone()); + } + Self { + daemon_version: status.version, + profile: status.profile, + bypass: status.bypass, + base_uptime_s: status.uptime_s, + connected_at: Instant::now(), + default_route: route_list.default_route, + streams, + layer_a: BTreeMap::new(), + meters: None, + last_meter_at: None, + overflow_total: 0, + last_error: None, + disconnected: None, + } + } + + fn uptime_s(&self) -> u64 { + self.base_uptime_s + .saturating_add(self.connected_at.elapsed().as_secs()) + } + + fn apply_event(&mut self, ev: Event) { + match ev.topic { + Topic::Meters if ev.event == "tick" => { + if let Ok(m) = serde_json::from_value::(ev.data) { + self.meters = Some(m); + self.last_meter_at = Some(Instant::now()); + } + } + Topic::Meters if ev.event == "layer_a_level" => { + if let Ok(l) = serde_json::from_value::(ev.data) { + self.layer_a.insert(l.node_id, Some(l.reduction_db)); + } + } + Topic::Routing => { + if let Ok(re) = serde_json::from_value::(routing_payload(&ev)) { + match re { + RoutingEvent::StreamRouted { node_id, app, to } => { + self.streams.insert( + node_id, + StreamRoute { + node_id, + app, + route: to, + }, + ); + } + RoutingEvent::StreamRemoved { node_id } => { + self.streams.remove(&node_id); + self.layer_a.remove(&node_id); + } + RoutingEvent::LayerAAttached { node_id, .. } => { + // Mark managed; reduction unknown until the + // first `layer_a_level` event lands. + self.layer_a.entry(node_id).or_insert(None); + } + RoutingEvent::LayerADetached { node_id } => { + self.layer_a.remove(&node_id); + } + RoutingEvent::RuleChanged => { /* TUI doesn't display rules */ } + _ => {} + } + } + } + Topic::Profile => { + if let Ok(ProfileEvent::Changed { name, .. }) = + serde_json::from_value::(profile_payload(&ev)) + { + self.profile = name; + } + } + Topic::Daemon => { + if let Ok(de) = serde_json::from_value::(daemon_payload(&ev)) { + match de { + DaemonEvent::Overflow { + lost, total_lost, .. + } => { + self.overflow_total = total_lost.max(self.overflow_total + lost as u64); + } + DaemonEvent::Error { code, message } => { + self.last_error = Some(format!("{code}: {message}")); + } + DaemonEvent::Shutdown => { + self.disconnected = Some("daemon shutdown".into()); + } + DaemonEvent::Started { version } => { + self.daemon_version = version; + } + _ => {} + } + } + } + _ => {} + } + } +} + +/// The wire frame carries `{event, topic, data}` — the typed enum lives +/// inside `data` but is `#[serde(tag = "event")]`, so we re-inject the +/// event name to make serde happy. Same dance for the other topics. +fn routing_payload(ev: &Event) -> serde_json::Value { + inject_event(&ev.event, &ev.data) +} +fn profile_payload(ev: &Event) -> serde_json::Value { + inject_event(&ev.event, &ev.data) +} +fn daemon_payload(ev: &Event) -> serde_json::Value { + inject_event(&ev.event, &ev.data) +} + +fn inject_event(event: &str, data: &serde_json::Value) -> serde_json::Value { + let mut obj = match data { + serde_json::Value::Object(m) => m.clone(), + _ => serde_json::Map::new(), + }; + obj.insert("event".into(), serde_json::Value::String(event.to_string())); + serde_json::Value::Object(obj) +} + +// --------------------------------------------------------------------------- +// Draw loop +// --------------------------------------------------------------------------- + +fn draw_loop( + terminal: &mut Terminal, + status: Status, + route_list: headroom_ipc::RouteList, + rx: Receiver, +) -> Result<(), TuiError> { + let mut state = UiState::new(status, route_list); + // 10 Hz redraw floor so uptime + staleness counters tick even when + // there are no events flowing. + let ticker = tick(Duration::from_millis(100)); + let input_rx = spawn_input_thread(); + + loop { + terminal.draw(|f| draw(f, &state))?; + + select! { + recv(rx) -> msg => match msg { + Ok(Msg::Event(ev)) => state.apply_event(ev), + Ok(Msg::Disconnected(reason)) => { + state.disconnected = Some(reason); + // Final paint, then linger briefly so the user sees + // the disconnected banner. + terminal.draw(|f| draw(f, &state))?; + thread::sleep(Duration::from_millis(800)); + return Ok(()); + } + Err(_) => return Ok(()), + }, + recv(input_rx) -> msg => match msg { + Ok(InputMsg::Quit) => return Ok(()), + Ok(InputMsg::Other) => {} + Err(_) => return Ok(()), + }, + recv(ticker) -> _ => {} + } + } +} + +// --------------------------------------------------------------------------- +// Input thread +// --------------------------------------------------------------------------- + +enum InputMsg { + Quit, + Other, +} + +fn spawn_input_thread() -> Receiver { + let (tx, rx) = unbounded::(); + thread::Builder::new() + .name("headroom-monitor-input".into()) + .spawn(move || loop { + // Block on the next terminal event; crossterm's read() is + // a blocking syscall against stdin. + let Ok(ev) = event::read() else { return }; + let msg = match ev { + CtEvent::Key(k) if is_quit(&k) => InputMsg::Quit, + CtEvent::Key(_) | CtEvent::Resize(_, _) => InputMsg::Other, + _ => continue, + }; + if tx.send(msg).is_err() { + return; + } + }) + .expect("spawn input thread"); + rx +} + +fn is_quit(k: &KeyEvent) -> bool { + matches!(k.code, KeyCode::Char('q') | KeyCode::Esc) + || (k.modifiers.contains(KeyModifiers::CONTROL) + && matches!(k.code, KeyCode::Char('c') | KeyCode::Char('C'))) +} + +// --------------------------------------------------------------------------- +// Drawing +// --------------------------------------------------------------------------- + +fn draw(f: &mut Frame, state: &UiState) { + let area = f.area(); + let outer = Block::default() + .borders(Borders::ALL) + .title(Span::styled( + " headroom monitor ", + Style::default().add_modifier(Modifier::BOLD), + )) + .title_top(Line::from(header_status(state)).right_aligned()) + .title_bottom(Line::from(footer_text(state)).right_aligned()); + let inner = outer.inner(area); + f.render_widget(outer, area); + + let chunks = Layout::default() + .direction(Direction::Vertical) + .constraints([ + Constraint::Length(6), // bus gauges + Constraint::Length(5), // loudness + Constraint::Min(4), // streams table + ]) + .split(inner); + + draw_bus(f, chunks[0], state); + draw_loudness(f, chunks[1], state); + draw_streams(f, chunks[2], state); +} + +fn header_status(state: &UiState) -> Vec> { + let bypass_span = if state.bypass { + Span::styled( + " BYPASS ", + Style::default().fg(Color::Black).bg(Color::Yellow), + ) + } else { + Span::styled(" processed ", Style::default().fg(Color::Green)) + }; + vec![ + Span::raw(" profile: "), + Span::styled(state.profile.clone(), Style::default().bold()), + Span::raw(" "), + bypass_span, + Span::raw(format!( + " v{} uptime {} ", + state.daemon_version, + fmt_uptime(state.uptime_s()) + )), + ] +} + +fn footer_text(state: &UiState) -> Vec> { + let mut parts: Vec = vec![ + Span::raw(" q/Esc/Ctrl-C quit "), + Span::styled("·", Style::default().fg(Color::DarkGray)), + Span::raw(" subscribed: meters routing profile daemon "), + ]; + if state.overflow_total > 0 { + parts.push(Span::styled("·", Style::default().fg(Color::DarkGray))); + parts.push(Span::styled( + format!(" dropped: {} ", state.overflow_total), + Style::default().fg(Color::Yellow), + )); + } + if let Some(err) = &state.last_error { + parts.push(Span::styled("·", Style::default().fg(Color::DarkGray))); + parts.push(Span::styled( + format!(" daemon error: {err} "), + Style::default().fg(Color::Red), + )); + } + if let Some(reason) = &state.disconnected { + parts.push(Span::styled("·", Style::default().fg(Color::DarkGray))); + parts.push(Span::styled( + format!(" disconnected: {reason} "), + Style::default().fg(Color::Red).add_modifier(Modifier::BOLD), + )); + } + parts +} + +fn draw_bus(f: &mut Frame, area: Rect, state: &UiState) { + let block = Block::default() + .borders(Borders::ALL) + .title(" bus dsp "); + let inner = block.inner(area); + f.render_widget(block, area); + + let rows = Layout::default() + .direction(Direction::Vertical) + .constraints([ + Constraint::Length(1), + Constraint::Length(1), + Constraint::Length(1), + Constraint::Length(1), + ]) + .split(inner); + + let m = state.meters; + draw_gauge_row( + f, + rows[0], + GaugeRow { + label: "AGC target", + value: m.map(|t| t.agc_gain_db), + min: -12.0, + max: 12.0, + unit: "dB", + color: Color::Cyan, + }, + ); + draw_gauge_row( + f, + rows[1], + GaugeRow { + label: "Compressor GR", + value: m.map(|t| t.compressor_gr_db), + min: -24.0, + max: 0.0, + unit: "dB", + color: Color::Magenta, + }, + ); + draw_gauge_row( + f, + rows[2], + GaugeRow { + label: "Limiter GR", + value: m.map(|t| t.limiter_gr_db), + min: -24.0, + max: 0.0, + unit: "dB", + color: Color::Red, + }, + ); + draw_gauge_row( + f, + rows[3], + GaugeRow { + label: "True peak", + value: m.map(|t| t.true_peak_dbtp), + min: -60.0, + max: 3.0, + unit: "dBTP", + color: Color::Green, + }, + ); +} + +struct GaugeRow<'a> { + label: &'a str, + value: Option, + min: f32, + max: f32, + unit: &'a str, + color: Color, +} + +/// One labeled gauge row: `LABEL VALUE [████░░░░] min..max`. +fn draw_gauge_row(f: &mut Frame, area: Rect, row: GaugeRow<'_>) { + let GaugeRow { + label, + value, + min, + max, + unit, + color, + } = row; + let cols = Layout::default() + .direction(Direction::Horizontal) + .constraints([ + Constraint::Length(16), + Constraint::Length(14), + Constraint::Min(8), + Constraint::Length(14), + ]) + .split(area); + + f.render_widget(Paragraph::new(format!(" {label}")), cols[0]); + + let value_str = value + .map(|v| format!("{v:+7.2} {unit}")) + .unwrap_or_else(|| " -- ".to_string()); + f.render_widget( + Paragraph::new(value_str).alignment(Alignment::Right), + cols[1], + ); + + let pct = match value { + Some(v) => { + let clamped = v.clamp(min, max); + ((clamped - min) / (max - min)).clamp(0.0, 1.0) as f64 + } + None => 0.0, + }; + let gauge = Gauge::default() + .gauge_style(Style::default().fg(color)) + .ratio(pct) + .label(""); + f.render_widget(gauge, cols[2]); + + f.render_widget( + Paragraph::new(format!("{min:.0}..{max:.0} ")).alignment(Alignment::Right), + cols[3], + ); +} + +fn draw_loudness(f: &mut Frame, area: Rect, state: &UiState) { + let block = Block::default() + .borders(Borders::ALL) + .title(" loudness (BS.1770) "); + let inner = block.inner(area); + f.render_widget(block, area); + + let staleness = state + .last_meter_at + .map(|t| t.elapsed()) + .unwrap_or(Duration::ZERO); + let stale = staleness > Duration::from_millis(500); + + let (mom, st, intg) = match state.meters { + Some(m) => (Some(m.momentary_lufs), Some(m.shortterm_lufs), Some(m.integrated_lufs)), + None => (None, None, None), + }; + + let lines = vec![ + lufs_line("Momentary (400 ms)", mom, stale), + lufs_line("Short-term (3 s)", st, stale), + lufs_line("Integrated (gated)", intg, stale), + ]; + f.render_widget(Paragraph::new(lines), inner); +} + +fn lufs_line(label: &str, v: Option, stale: bool) -> Line<'static> { + let val = match v { + Some(x) if x > headroom_core::agc::LOUDNESS_FLOOR_LUFS + 0.5 => { + format!("{x:+7.2} LUFS") + } + Some(_) => " -- LUFS".to_string(), + None => " -- LUFS".to_string(), + }; + let style = if stale { + Style::default().fg(Color::DarkGray) + } else { + Style::default() + }; + Line::from(vec![ + Span::raw(format!(" {label:<24}")), + Span::styled(val, style), + ]) +} + +fn draw_streams(f: &mut Frame, area: Rect, state: &UiState) { + let title = format!( + " streams ({}) — default: {} ", + state.streams.len(), + state.default_route + ); + let block = Block::default().borders(Borders::ALL).title(title); + + let header = Row::new(vec!["node", "app", "route", "layer A"]) + .style(Style::default().add_modifier(Modifier::BOLD)); + + let rows: Vec = state + .streams + .values() + .map(|s| { + let route_cell = match s.route { + Route::Processed => Cell::from("processed").style(Style::default().fg(Color::Green)), + Route::Bypass => Cell::from("bypass").style(Style::default().fg(Color::Yellow)), + }; + let la_cell = match state.layer_a.get(&s.node_id) { + Some(Some(db)) => Cell::from(format!("{db:+5.1} dB")) + .style(Style::default().fg(Color::Magenta)), + Some(None) => Cell::from("attached") + .style(Style::default().fg(Color::DarkGray)), + None => Cell::from("—").style(Style::default().fg(Color::DarkGray)), + }; + Row::new(vec![ + Cell::from(s.node_id.to_string()), + Cell::from(s.app.clone()), + route_cell, + la_cell, + ]) + }) + .collect(); + + let widths = [ + Constraint::Length(8), + Constraint::Min(20), + Constraint::Length(12), + Constraint::Length(10), + ]; + let table = Table::new(rows, widths).header(header).block(block); + f.render_widget(table, area); +} + +fn fmt_uptime(s: u64) -> String { + let h = s / 3600; + let m = (s % 3600) / 60; + let sec = s % 60; + if h > 0 { + format!("{h}h{m:02}m{sec:02}s") + } else if m > 0 { + format!("{m}m{sec:02}s") + } else { + format!("{sec}s") + } +} + +#[cfg(test)] +mod tests { + use super::*; + use headroom_ipc::{Sinks, Status}; + + fn empty_state() -> UiState { + let status = Status { + version: "test".into(), + protocol: 1, + uptime_s: 0, + profile: "default".into(), + bypass: false, + sinks: Sinks::default(), + streams: vec![], + warnings: vec![], + }; + let route_list = headroom_ipc::RouteList { + rules: vec![], + current: vec![], + default_route: Route::Processed, + }; + UiState::new(status, route_list) + } + + #[test] + fn meter_tick_event_updates_state() { + let mut state = empty_state(); + let tick = MeterTick { + momentary_lufs: -19.3, + shortterm_lufs: -20.1, + integrated_lufs: -19.8, + true_peak_dbtp: -1.4, + gain_reduction_db: -2.1, + compressor_gr_db: -0.8, + limiter_gr_db: -1.3, + agc_gain_db: 0.5, + }; + let ev = Event::new(Topic::Meters, "tick", &tick).unwrap(); + state.apply_event(ev); + let got = state.meters.expect("meters set"); + assert!((got.momentary_lufs - tick.momentary_lufs).abs() < f32::EPSILON); + assert!((got.true_peak_dbtp - tick.true_peak_dbtp).abs() < f32::EPSILON); + assert!(state.last_meter_at.is_some()); + } + + #[test] + fn stream_removed_prunes_state() { + let mut state = empty_state(); + // Insert via stream_routed first. + state.apply_event( + Event::new( + Topic::Routing, + "stream_routed", + &serde_json::json!({ "node_id": 7, "app": "x", "to": "processed" }), + ) + .unwrap(), + ); + state.apply_event( + Event::new( + Topic::Routing, + "layer_a_attached", + &serde_json::json!({ "node_id": 7, "app": "x" }), + ) + .unwrap(), + ); + assert!(state.streams.contains_key(&7)); + assert!(state.layer_a.contains_key(&7)); + + state.apply_event( + Event::new( + Topic::Routing, + "stream_removed", + &serde_json::json!({ "node_id": 7 }), + ) + .unwrap(), + ); + assert!(!state.streams.contains_key(&7)); + assert!(!state.layer_a.contains_key(&7)); + } + + #[test] + fn layer_a_level_updates_reduction() { + let mut state = empty_state(); + state.apply_event( + Event::new( + Topic::Routing, + "layer_a_attached", + &serde_json::json!({ "node_id": 11, "app": "loud-app" }), + ) + .unwrap(), + ); + assert_eq!(state.layer_a.get(&11), Some(&None)); + + state.apply_event( + Event::new( + Topic::Meters, + "layer_a_level", + &serde_json::json!({ + "node_id": 11, + "app": "loud-app", + "volume_lin": 0.256_f32, + "reduction_db": -11.8_f32, + }), + ) + .unwrap(), + ); + let r = state.layer_a.get(&11).copied().flatten().unwrap(); + assert!((r - -11.8).abs() < 1e-4); + } + + #[test] + fn routing_event_inserts_stream() { + let mut state = empty_state(); + let ev = Event::new( + Topic::Routing, + "stream_routed", + &serde_json::json!({ + "node_id": 42, + "app": "firefox", + "to": "bypass", + }), + ) + .unwrap(); + state.apply_event(ev); + let s = state.streams.get(&42).expect("stream tracked"); + assert_eq!(s.app, "firefox"); + assert_eq!(s.route, Route::Bypass); + } + + #[test] + fn profile_changed_updates_active() { + let mut state = empty_state(); + let ev = Event::new( + Topic::Profile, + "changed", + &serde_json::json!({ + "name": "night", + "previous": "default", + }), + ) + .unwrap(); + state.apply_event(ev); + assert_eq!(state.profile, "night"); + } + + #[test] + fn daemon_overflow_accumulates() { + let mut state = empty_state(); + let ev = Event::new( + Topic::Daemon, + "overflow", + &serde_json::json!({ + "lost_topic": "meters", + "lost": 3u32, + "total_lost": 5u64, + }), + ) + .unwrap(); + state.apply_event(ev); + assert_eq!(state.overflow_total, 5); + } + + #[test] + fn fmt_uptime_buckets() { + assert_eq!(fmt_uptime(5), "5s"); + assert_eq!(fmt_uptime(75), "1m15s"); + assert_eq!(fmt_uptime(3725), "1h02m05s"); + } +} diff --git a/crates/headroom-core/src/agc.rs b/crates/headroom-core/src/agc.rs index 354eca5..b4b54ce 100644 --- a/crates/headroom-core/src/agc.rs +++ b/crates/headroom-core/src/agc.rs @@ -32,8 +32,10 @@ const TICK_BUF_SAMPLES: usize = 8192; /// Loudness floor we treat as "no usable measurement yet" — returned /// by `ebur128` before its short-term window has filled, or during -/// digital silence. -const LOUDNESS_FLOOR_LUFS: f32 = -200.0; +/// digital silence. Published as-is in `MeterTick.*_lufs` fields, so +/// clients can use this constant to recognise "no measurement" without +/// hard-coding the number. +pub const LOUDNESS_FLOOR_LUFS: f32 = -200.0; /// Slow AGC controller. pub struct AgcController { diff --git a/crates/headroom-ipc/src/lib.rs b/crates/headroom-ipc/src/lib.rs index 1bb9490..c9d45ff 100644 --- a/crates/headroom-ipc/src/lib.rs +++ b/crates/headroom-ipc/src/lib.rs @@ -13,9 +13,9 @@ mod proto; pub use codec::{Codec, DEFAULT_MAX_FRAME_BYTES, MIN_MAX_FRAME_BYTES}; pub use error::{Error, ErrorCode, ProtoError}; pub use proto::{ - DaemonEvent, Event, HelloData, MeterTick, Op, ProfileEvent, ProfileInfo, Request, Response, - ResponsePayload, Route, RouteList, RouteRule, RouteRuleMatch, RoutingEvent, ServerFrame, - SinkInfo, Sinks, Status, StreamRoute, Topic, + DaemonEvent, Event, HelloData, LayerALevel, MeterTick, Op, ProfileEvent, ProfileInfo, Request, + Response, ResponsePayload, Route, RouteList, RouteRule, RouteRuleMatch, RoutingEvent, + ServerFrame, SinkInfo, Sinks, Status, StreamRoute, Topic, }; /// Wire-protocol version. Bumped only on incompatible changes. diff --git a/crates/headroom-ipc/src/proto.rs b/crates/headroom-ipc/src/proto.rs index 4b5aabf..cbe761c 100644 --- a/crates/headroom-ipc/src/proto.rs +++ b/crates/headroom-ipc/src/proto.rs @@ -525,10 +525,49 @@ pub enum RoutingEvent { /// Route assigned. to: Route, }, + /// A stream tracked by the routing engine went away (its + /// PipeWire node disappeared). Clients should drop any state + /// indexed by `node_id`. + StreamRemoved { + /// Node id of the departed stream. + node_id: u32, + }, + /// A Layer A (per-app level control) tap was attached to a + /// stream — the daemon will start managing its + /// `Props.channelVolumes` and publishing `meters/layer_a_level` + /// events for it. + LayerAAttached { + /// Node id of the managed stream. + node_id: u32, + /// Application identifier. + app: String, + }, + /// A Layer A tap was torn down (typically because the stream + /// went away). Clients should drop Layer A state for `node_id`. + LayerADetached { + /// Node id whose tap was torn down. + node_id: u32, + }, /// A persistent rule was added, replaced, or removed. RuleChanged, } +/// `meters/layer_a_level` payload — published when the per-app +/// (Layer A) level controller writes a new `channelVolumes` value to +/// a managed stream. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct LayerALevel { + /// Source PipeWire node id. + pub node_id: u32, + /// Application identifier. + pub app: String, + /// Linear volume that was written (1.0 = unity). + pub volume_lin: f32, + /// Smoothed gain reduction the controller currently asserts, in + /// dB. ≤ 0 dB when reducing. + pub reduction_db: f32, +} + /// `daemon` topic events. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(tag = "event", rename_all = "snake_case")] From df8af6c4d2481d84a43da93172a0e63e09e1c035 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 15:36:15 +1000 Subject: [PATCH 03/21] 4k: routing establishes explicit links, not just target.object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5 smoke-tested the monitor TUI and surfaced that the bus DSP never sees signal: bus meters stay at the LUFS floor / -200 dBTP even when `headroom status` reports a stream as route=processed. The root cause is in routing, not the TUI. Why writing target.object alone wasn't enough The daemon's routing engine wrote `target.object` on the stream node and relied on WirePlumber to (re-)link the stream to the declared sink. That works for streams the daemon creates itself (`headroom-filter.playback`): the `pw_stream` carries target.object at connect time, before WP sees the node global, so WP's first linking decision honours it. For external clients (pw-cat, Strawberry) the order is reversed: WP links the stream the instant the node global appears, *before* the daemon's registry callback fires `try_route_stream`. The metadata write that follows is a no-op for routing — WP doesn't re-link in response to a target.object change on an already-linked node. Verified manually: writing target.object on a live stream + severing its bad link did NOT cause WP to relink to the declared target. WP just left the stream unrouted. What this commit changes RoutingState now tracks `Link` registry globals (`links_by_id` + `outbound_links_by_node` reverse index) and Audio/Sink globals by name (`sinks_by_name` now also carries `headroom-processed`, not just the real-hardware sinks). On every routing decision — `try_route_stream`, `apply_pw_command(RouteStream)`, and the bypass-retarget pass inside `adopt_new_real_sink` — the daemon also enqueues a `PendingRoute` for the source node. Two enforcement paths: - **Fast vigilance** in `try_capture_link`: when WP creates a new link out of a managed stream that lands on a different Audio/Sink, the daemon calls `registry.destroy_global(link_id)` immediately. Links to non-sinks (Layer A taps, other downstream consumers) are left alone — Layer A owns those. - **50 ms drain loop** in `apply_pending_routes`: for each pending route, once the source's output ports and the target sink's input ports are visible on the registry, the daemon destroys any remaining outbound link landing on the wrong sink and creates the desired link via `link-factory` (new `create_routing_link` helper — non-passive variant of the existing `create_explicit_link` Layer A uses). The owned `Link` proxies live in `managed_route_links` keyed by source node id; dropping them tears the links down via `object.linger = "false"`. `target.object` writes are kept (cheap hint that helps fresh pw_streams and documents intent) but are no longer the source of truth. Verified All 185 tests still pass; clippy clean at -D warnings --all-targets. Live smoke (pw-cat /dev/zero of a 1 kHz sine at -20 dBFS into `--target headroom-processed`): - Before: pw-cat:output → Mbox:playback directly; bus meters pinned at floor, integrated_lufs = -200, true_peak = -200. - After: `routed pw-cat → headroom-processed` followed within 50 ms by `explicit routing link established`; pw-link confirms pw-cat:output → headroom-processed:playback (+ the Layer A tap link, preserved). Bus meters show momentary -28 → -16 LUFS, true_peak around -34 to -19 dBTP, compressor GR -2.6 dB, limiter GR -6.7 dB — i.e. the bus DSP chain is processing signal end-to-end for the first time. - Layer A tap creation logs exactly once (vs. the create/destroy fighting loop the first cut had before `enforce_link_for_managed_stream` learned to skip non-sink destinations). Known limits not addressed here - `default.audio.sink` reassertion by WP. The daemon still writes `default.audio.sink = headroom-processed` but WP's session policy may rewrite it back. With explicit links, this is now mostly cosmetic — new streams whose target.object matches headroom-processed will be routed correctly via the same enforcement path even if default is something else. The metadata side will be tightened later if it turns out to matter operationally. - A spurious filter.playback → processed:playback feedback link still appears in the live graph (the bus filter's own output being linked back to its sink). Suspected source: a leftover rule on the filter node. To investigate separately; doesn't currently affect signal flow because filter capture sees signal from the real producer. --- crates/headroom-core/src/pw/registry.rs | 435 +++++++++++++++++++++++- 1 file changed, 431 insertions(+), 4 deletions(-) diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index 6a0c465..6f81acb 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -89,6 +89,44 @@ enum PortDirection { Out, } +/// Lightweight view of a `Link` registry global. Tracked so the +/// routing engine can find any WirePlumber-created link out of a +/// managed stream's output port and destroy it via +/// `registry.destroy_global` when it conflicts with the daemon's +/// declared route. See `4k`: writing `target.object` alone is +/// insufficient — WP only honours it at connect time, not on +/// metadata change, so the daemon owns the link layer for any +/// stream it actively routes. +#[derive(Debug, Clone, Copy)] +struct LinkInfo { + /// The source (writer) port id. + output_port: u32, + /// The sink (reader) port id. + input_port: u32, + /// The source node id (owner of `output_port`). Cached so we + /// don't have to walk `ports_by_node` to answer "what node is + /// at the head of this link?". + output_node: u32, + /// The destination node id (owner of `input_port`). + input_node: u32, +} + +/// Per-stream routing intent recorded by `try_route_stream` and +/// resolved by `apply_pending_routes` once the source's output +/// ports + target sink's input ports are both visible on the +/// registry. The daemon retries on every drain tick. +#[derive(Debug, Clone)] +struct PendingRoute { + /// Target sink's `node.name` (`headroom-processed` or the + /// current real-sink name). + target_sink_name: String, + /// Cached app label for telemetry on completion. + app_label: String, + /// Logical decision; mirrors what we publish in + /// `routing/stream_routed`. + route: Route, +} + /// Subject id passed to `set_property` for keys that aren't bound to /// a specific node (system-wide settings like `default.audio.sink`). const METADATA_SUBJECT_GLOBAL: u32 = 0; @@ -140,6 +178,22 @@ pub struct RoutingState { /// `link.input.port`). Maintained additively in `on_global`; /// entries removed in `on_global_remove`. ports_by_node: HashMap>, + /// All known `Link` registry globals, keyed by the link's own + /// global id. See [`LinkInfo`] for the rationale. + links_by_id: HashMap, + /// Set of outbound link ids per source node. Lets us answer + /// "what links currently exit this node?" without scanning + /// every link. + outbound_links_by_node: HashMap>, + /// Streams whose route has been declared but whose explicit + /// links haven't been built yet (typically because ports are + /// still arriving on the registry). Drained by + /// [`Self::apply_pending_routes`]. + pending_routes: HashMap, + /// Explicit `link-factory` `Link` proxies owned by the daemon, + /// keyed by source stream node id. Kept alive so the links + /// persist; dropped on stream removal or route change. + managed_route_links: HashMap>, } /// Per-stream Layer A bundle: the tap (audio path), the controller @@ -201,16 +255,27 @@ impl RoutingState { pw_command_rx, managed_streams: HashMap::new(), ports_by_node: HashMap::new(), + links_by_id: HashMap::new(), + outbound_links_by_node: HashMap::new(), + pending_routes: HashMap::new(), + managed_route_links: HashMap::new(), } } /// Drain any [`PwCommand`]s the IPC threads posted while we - /// weren't looking. Called by the polling timer source on every - /// tick. + /// weren't looking, then run a pass of routing-link enforcement. + /// Called by the 50 ms timer source installed in + /// [`crate::pw::PwContext::run_until_signal`]. + /// + /// Routing-link enforcement is intentionally tied to the same + /// (slow, operator-grade) tick rate as IPC command processing — + /// routing is a control-plane concern and a 50 ms ceiling on + /// "stream appeared but isn't linked yet" latency is fine. pub fn drain_pw_commands(&mut self) { while let Ok(cmd) = self.pw_command_rx.try_recv() { self.apply_pw_command(cmd); } + self.apply_pending_routes(); } fn apply_pw_command(&mut self, cmd: PwCommand) { @@ -234,6 +299,7 @@ impl RoutingState { } }; self.write_stream_target(node_id, &target_name, &app_label); + self.enqueue_route(node_id, target_name, app_label, to); } } } @@ -254,10 +320,137 @@ impl RoutingState { self.try_route_stream(global, back); } ObjectType::Port => self.try_capture_port(global), + ObjectType::Link => self.try_capture_link(global), _ => {} } } + /// Track a Link global so routing can find/destroy conflicting + /// links. Also runs the vigilance check: if the new link + /// originates from a stream we route and lands somewhere other + /// than its declared target, destroy it immediately. WP often + /// links streams the instant they appear — faster than our + /// node-global callback fires — so we depend on this fast-path + /// teardown plus the slower `apply_pending_routes` retry. + fn try_capture_link(&mut self, global: &GlobalObject<&DictRef>) { + let Some(props) = &global.props else { + tracing::debug!(link_id = global.id, "link global without props"); + return; + }; + let dict: &DictRef = props; + let parse = |k: &str| dict.get(k).and_then(|s| s.parse::().ok()); + let (Some(output_port), Some(input_port), Some(output_node), Some(input_node)) = ( + parse("link.output.port"), + parse("link.input.port"), + parse("link.output.node"), + parse("link.input.node"), + ) else { + tracing::debug!( + link_id = global.id, + out_port = ?parse("link.output.port"), + in_port = ?parse("link.input.port"), + out_node = ?parse("link.output.node"), + in_node = ?parse("link.input.node"), + "link global with incomplete props" + ); + return; + }; + + let info = LinkInfo { + output_port, + input_port, + output_node, + input_node, + }; + tracing::debug!( + link_id = global.id, + output_port, + input_port, + output_node, + input_node, + "captured link global" + ); + self.links_by_id.insert(global.id, info); + self.outbound_links_by_node + .entry(output_node) + .or_default() + .push(global.id); + + self.enforce_link_for_managed_stream(global.id, &info); + } + + /// If `link` originates from a stream the daemon is routing, and + /// it lands on a *different* Audio/Sink than the declared + /// target, destroy it. Links to non-sinks (Layer A taps, e.g. + /// other streams the source feeds) are left alone — Layer A + /// owns its own passive links and we don't want to fight it. + fn enforce_link_for_managed_stream(&mut self, link_id: u32, info: &LinkInfo) { + let intent = self.intent_for_node(info.output_node); + let Some((target_sink_node_id, target_input_ports)) = intent else { + return; + }; + if info.input_node == target_sink_node_id + && target_input_ports.iter().any(|p| *p == info.input_port) + { + return; // link lands on the intended target — keep + } + // If the destination isn't a known sink, leave it alone. + // It's likely a Layer A tap or some other downstream + // consumer the daemon doesn't own. + let dest_is_sink = self + .sinks_by_name + .values() + .any(|&id| id == info.input_node); + if !dest_is_sink { + return; + } + match self.registry.destroy_global(link_id).into_result() { + Ok(_) => tracing::debug!( + link_id, + output_node = info.output_node, + input_node = info.input_node, + "destroyed conflicting link for managed stream" + ), + Err(e) => tracing::warn!( + link_id, + output_node = info.output_node, + error = ?e, + "failed to destroy conflicting link" + ), + } + } + + /// Resolve a source node to `(target_sink_node_id, + /// target_input_port_ids)` if the daemon currently intends to + /// route it. Used by the link-vigilance fast path. + fn intent_for_node(&self, source_node: u32) -> Option<(u32, Vec)> { + let target_name = if let Some(p) = self.pending_routes.get(&source_node) { + p.target_sink_name.clone() + } else if self.managed_route_links.contains_key(&source_node) { + let s = self.daemon.lock(); + let entry = s.streams.get(&source_node)?; + match entry.route { + Route::Processed => PROCESSED_SINK_NAME.to_owned(), + Route::Bypass => s.real_sink.name.clone()?, + } + } else { + return None; + }; + let target_node = *self.sinks_by_name.get(&target_name)?; + let target_inputs: Vec = self + .ports_by_node + .get(&target_node)? + .iter() + .filter(|p| p.direction == PortDirection::In) + .map(|p| p.port_id) + .collect(); + if target_inputs.is_empty() { + None + } else { + Some((target_node, target_inputs)) + } + } + fn try_capture_port(&mut self, global: &GlobalObject<&DictRef>) { let Some(props) = &global.props else { return }; let dict: &DictRef = props; @@ -376,7 +569,7 @@ impl RoutingState { self.write_default_audio_sink(PROCESSED_SINK_NAME); } - fn try_capture_processed_sink_id(&self, global: &GlobalObject<&DictRef>) { + fn try_capture_processed_sink_id(&mut self, global: &GlobalObject<&DictRef>) { let Some(props) = &global.props else { return }; let dict: &DictRef = props; if dict.get("node.name") != Some(PROCESSED_SINK_NAME) { @@ -387,6 +580,13 @@ impl RoutingState { tracing::info!(node_id = global.id, "captured headroom-processed node id"); s.processed_sink_id = Some(global.id); } + drop(s); + // Also expose the processed sink in `sinks_by_name` so the + // 4k routing engine can resolve `headroom-processed` to its + // node id (and from there, its input ports) when wiring + // explicit links for processed-routed streams. + self.sinks_by_name + .insert(PROCESSED_SINK_NAME.to_owned(), global.id); } fn try_route_stream( @@ -430,11 +630,23 @@ impl RoutingState { match decision { RoutingDecision::Route(Route::Processed) => { self.write_stream_target(info.node_id, PROCESSED_SINK_NAME, &app_label); + self.enqueue_route( + info.node_id, + PROCESSED_SINK_NAME.to_owned(), + app_label.clone(), + Route::Processed, + ); self.record_route(info.node_id, app_label.clone(), Route::Processed); } RoutingDecision::Route(Route::Bypass) => { if let Some(name) = real_sink_name.as_deref() { self.write_stream_target(info.node_id, name, &app_label); + self.enqueue_route( + info.node_id, + name.to_owned(), + app_label.clone(), + Route::Bypass, + ); } else { // We haven't seen `default.audio.sink` resolve yet // (very early boot). Record the route; the stream @@ -668,6 +880,180 @@ impl RoutingState { } } + /// Record routing intent for `node_id`. Subsequent ticks of + /// [`Self::apply_pending_routes`] will tear down any conflicting + /// link and create the explicit link-factory link from the + /// source's output ports to `target_sink_name`'s input ports. + /// + /// If a previous route for this node is still pending it gets + /// replaced — last intent wins. + fn enqueue_route( + &mut self, + node_id: u32, + target_sink_name: String, + app_label: String, + route: Route, + ) { + // Replacing intent: drop any old managed links for this + // stream so apply_pending_routes can rebuild against the new + // target. Dropping the proxies destroys the links via + // `object.linger = "false"`. + self.managed_route_links.remove(&node_id); + self.pending_routes.insert( + node_id, + PendingRoute { + target_sink_name, + app_label, + route, + }, + ); + } + + /// Drain `pending_routes` once per timer tick: for every stream + /// whose source ports + declared target's ports are both visible + /// on the registry, tear down any conflicting outbound links + /// and create the explicit link-factory links the daemon + /// promises in `routing/stream_routed`. Intents that aren't + /// ready yet stay in the queue. + fn apply_pending_routes(&mut self) { + // Take a snapshot of the keys we'll try this tick; we mutate + // `self.managed_route_links` while iterating so we can't hold + // a borrow on `pending_routes`. + let candidates: Vec = self.pending_routes.keys().copied().collect(); + if !candidates.is_empty() { + tracing::debug!( + pending = candidates.len(), + "apply_pending_routes pass" + ); + } + for node_id in candidates { + let Some(intent) = self.pending_routes.get(&node_id).cloned() else { + continue; + }; + + let Some(&target_node) = self.sinks_by_name.get(&intent.target_sink_name) else { + tracing::debug!( + node_id, + target = intent.target_sink_name.as_str(), + "pending route: target sink not yet on registry" + ); + continue; // target sink not yet on registry + }; + let Some(src_outs) = + collect_ports(&self.ports_by_node, node_id, PortDirection::Out) + else { + tracing::debug!(node_id, "pending route: source has no output ports yet"); + continue; + }; + let Some(target_ins) = + collect_ports(&self.ports_by_node, target_node, PortDirection::In) + else { + tracing::debug!(node_id, target_node, "pending route: target has no input ports yet"); + continue; + }; + // Stereo v0 — pair by ordinal. + if src_outs.len() < 2 || target_ins.len() < 2 { + tracing::debug!( + node_id, + src_outs = src_outs.len(), + target_ins = target_ins.len(), + "pending route: not enough ports yet" + ); + continue; + } + let want: Vec<(u32, u32)> = src_outs + .iter() + .take(2) + .zip(target_ins.iter().take(2)) + .map(|(o, i)| (o.port_id, i.port_id)) + .collect(); + let want_set: std::collections::HashSet<(u32, u32)> = want.iter().copied().collect(); + + // 1) Destroy outbound links from this stream that land + // on a *different* sink. Links to non-sinks (Layer A + // taps, etc.) are left alone — they're managed by + // someone else (Layer A's own retry loop) and aren't + // alternatives to the target sink. + let existing: Vec = self + .outbound_links_by_node + .get(&node_id) + .cloned() + .unwrap_or_default(); + for link_id in existing { + let Some(info) = self.links_by_id.get(&link_id).copied() else { + continue; + }; + if want_set.contains(&(info.output_port, info.input_port)) { + continue; // already correct — keep + } + let dest_is_sink = self + .sinks_by_name + .values() + .any(|&id| id == info.input_node); + if !dest_is_sink { + continue; // probably a Layer A tap or similar + } + if let Err(e) = self.registry.destroy_global(link_id).into_result() { + tracing::warn!( + link_id, + node_id, + target = intent.target_sink_name.as_str(), + error = ?e, + "apply_pending_routes: destroy_global failed" + ); + } + } + + // 2) Create any missing wanted links. + let already_wanted: std::collections::HashSet<(u32, u32)> = self + .outbound_links_by_node + .get(&node_id) + .into_iter() + .flatten() + .filter_map(|id| self.links_by_id.get(id)) + .map(|info| (info.output_port, info.input_port)) + .collect(); + let mut created: Vec = self + .managed_route_links + .remove(&node_id) + .unwrap_or_default(); + let mut all_ok = true; + for (out_port, in_port) in &want { + if already_wanted.contains(&(*out_port, *in_port)) { + continue; + } + match create_routing_link(&self.core, *out_port, *in_port) { + Ok(link) => created.push(link), + Err(e) => { + tracing::warn!( + node_id, + out_port, + in_port, + target = intent.target_sink_name.as_str(), + error = %e, + "apply_pending_routes: create_object failed; retry next tick" + ); + all_ok = false; + break; + } + } + } + if !created.is_empty() { + self.managed_route_links.insert(node_id, created); + } + if all_ok { + tracing::info!( + node_id, + app = intent.app_label.as_str(), + target = intent.target_sink_name.as_str(), + route = intent.route.as_str(), + "explicit routing link established" + ); + self.pending_routes.remove(&node_id); + } + } + } + /// Write `target.object = {"name":""}` for `node_id`. fn write_stream_target(&self, node_id: u32, sink_name: &str, app_label: &str) { let Some(md) = &self.default_metadata else { @@ -728,7 +1114,7 @@ impl RoutingState { /// Update `preferred_real_sink` and retarget every bypass-routed /// stream + the filter playback + re-assert headroom-processed as /// default. - fn adopt_new_real_sink(&self, new_sink_name: String) { + fn adopt_new_real_sink(&mut self, new_sink_name: String) { let (bypass_targets, resolved_node_id) = { let mut s = self.daemon.lock(); let Some(targets) = s.apply_real_sink_change(&new_sink_name) else { @@ -751,6 +1137,12 @@ impl RoutingState { for (node_id, app_label) in &bypass_targets { self.write_stream_target(*node_id, &new_sink_name, app_label); + self.enqueue_route( + *node_id, + new_sink_name.clone(), + app_label.clone(), + Route::Bypass, + ); } if !bypass_targets.is_empty() { tracing::info!( @@ -825,6 +1217,26 @@ impl RoutingState { } self.ports_by_node.retain(|_, ports| !ports.is_empty()); + // Drop any link tracking entries: either `node_id` IS a link + // global, or it's a node whose links we should forget. + if let Some(info) = self.links_by_id.remove(&node_id) { + if let Some(v) = self.outbound_links_by_node.get_mut(&info.output_node) { + v.retain(|&id| id != node_id); + if v.is_empty() { + self.outbound_links_by_node.remove(&info.output_node); + } + } + } + // node_id may be a node — drop its outbound list and any link + // entries that referenced it as source. + self.outbound_links_by_node.remove(&node_id); + self.links_by_id + .retain(|_, info| info.output_node != node_id && info.input_node != node_id); + + // Stream gone — drop pending intent + managed Link proxies. + self.pending_routes.remove(&node_id); + self.managed_route_links.remove(&node_id); + if self.filter_playback_id == Some(node_id) { tracing::debug!(node_id, "filter playback removed from registry"); self.filter_playback_id = None; @@ -970,6 +1382,21 @@ fn create_explicit_link(core: &Core, output_port: u32, input_port: u32) -> Resul core.create_object::("link-factory", &props) } +/// `link-factory` invocation for the main routing path: an active +/// (non-passive) link that drives the downstream sink. Used by 4k +/// to forcibly route streams when WirePlumber's target.object +/// respect is unreliable for already-linked streams. +fn create_routing_link(core: &Core, output_port: u32, input_port: u32) -> Result { + let out_str = output_port.to_string(); + let in_str = input_port.to_string(); + let props = properties! { + "link.output.port" => out_str.as_str(), + "link.input.port" => in_str.as_str(), + "object.linger" => "false", + }; + core.create_object::("link-factory", &props) +} + /// Write `Props.channelVolumes = [vol, vol]` (stereo) to the bound /// node. Used by [`RoutingState::drain_layer_a`] for Layer A's /// per-stream attenuation. Allocates a small POD buffer on the heap; From 8af6dff98d042435a59c5d14a10b4e508bd5b298 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 15:58:18 +1000 Subject: [PATCH 04/21] 4l: filter.playback through 4k enforcement + sticky default.audio.sink MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups from 4k's commit body, both surfaced by the same smoke-test setup. filter.playback through 4k `try_capture_filter_playback` and the bypass-retarget pass in `adopt_new_real_sink` now call `enqueue_route` on top of the existing `write_stream_target`. Without that, WirePlumber was fanning the filter's output port to *both* the real sink (the intended target) and `headroom-processed:playback` — a feedback loop where the filter's output flowed back into the processed sink, then through monitor → filter capture → DSP → filter playback again. Plumbing 4k for the filter required two small tweaks elsewhere: - `enforce_link_for_managed_stream` and `apply_pending_routes` were destroying every non-target outbound link from a managed source. That included Layer A passive tap links, which sent Layer A's own retry loop into a create/destroy fight with this code. Both paths now skip links whose destination isn't a known Audio/Sink, so only WP-created sink links get torn down. - The processed sink is now also recorded in `sinks_by_name` (previously skipped because it's "tracked elsewhere" in `processed_sink_id`). `apply_pending_routes` resolves the target by name, so it needed processed visible here to handle Route::Processed. Sticky default.audio.sink `adopt_new_real_sink` previously short-circuited via `apply_real_sink_change` when the real sink name hadn't changed — which meant the *first* time WP rewrote `default.audio.sink` away from `headroom-processed` we'd re-assert, but on every subsequent rewrite to the same Mbox value we'd skip out before reaching the re-assert call at the bottom of the function. WP won 1-0 after the first round. Fixed by hoisting the re-assertion into a dedicated method (`reassert_default_processed`) with a per-second attempt cap (10 per second), called both from the idempotent early-exit path and from the end of the full retarget path. The cap is what keeps a hostile WP policy from pulling us into a hot loop — at 10 Hz we tolerate a brief metadata storm, then back off for the remainder of the window. Verified 185 tests still pass; clippy clean at -D warnings --all-targets. Live smoke against a running PipeWire/WP: - `pw-metadata` confirms `default.audio.sink` settles on `headroom-processed` after daemon startup (daemon wrote 3 times in ~30 ms, WP yielded; metadata then stayed put). - `pw-link` confirms `headroom-filter.playback:output_{FL,FR}` has exactly one outbound link each — to the Mbox playback ports — with no link back to processed:playback. - Sine-into-processed regression still passes: 59/59 meter ticks above the floor, momentary_lufs around -28, true peak around -21 dBTP — bus DSP chain still processing end-to-end after the filter's link surface was tightened. --- crates/headroom-core/src/pw/registry.rs | 84 ++++++++++++++++++++++--- 1 file changed, 75 insertions(+), 9 deletions(-) diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index 6f81acb..13c0790 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -194,6 +194,11 @@ pub struct RoutingState { /// keyed by source stream node id. Kept alive so the links /// persist; dropped on stream removal or route change. managed_route_links: HashMap>, + /// Window-based limit for `default.audio.sink` re-assertions + /// so a hostile WP policy can't pull us into a hot loop. + /// `(window_start, attempts_in_window)`. See + /// [`Self::reassert_default_processed`]. + default_reassertion: Option<(std::time::Instant, u32)>, } /// Per-stream Layer A bundle: the tap (audio path), the controller @@ -259,6 +264,7 @@ impl RoutingState { outbound_links_by_node: HashMap::new(), pending_routes: HashMap::new(), managed_route_links: HashMap::new(), + default_reassertion: None, } } @@ -521,10 +527,22 @@ impl RoutingState { self.filter_playback_id = Some(global.id); // If a real sink is already known, pin the filter to it // immediately. Common at boot when the filter playback global - // arrives after we've adopted the prior default. + // arrives after we've adopted the prior default. Both writing + // target.object (the cheap hint) AND enqueuing through 4k's + // explicit-link path matters here — without the explicit + // enforcement, WirePlumber also fans the filter's output back + // into `headroom-processed:playback`, creating a tight + // feedback loop (filter output → processed sink → filter + // capture → filter output). let target = self.daemon.lock().real_sink.name.clone(); if let Some(name) = target { - self.write_stream_target(global.id, &name, "headroom-filter.playback"); + self.write_stream_target(global.id, &name, FILTER_PLAYBACK_NODE_NAME); + self.enqueue_route( + global.id, + name, + FILTER_PLAYBACK_NODE_NAME.to_owned(), + Route::Bypass, + ); } } @@ -1111,6 +1129,40 @@ impl RoutingState { self.adopt_new_real_sink(name); } + /// Re-assert `default.audio.sink = headroom-processed` with a + /// per-second attempt cap. WirePlumber's session policy will + /// often immediately rewrite our value back to the user's + /// stored preference; this method fights back so apps that + /// resolve to "the default sink" land in the processor. + /// + /// We tolerate up to `MAX_PER_WINDOW` rewrites per + /// `WINDOW` — enough to outlast WP's typical 1-2 follow-up + /// writes and (in adversarial cases) ride out a brief metadata + /// storm — then back off for the rest of the window. Explicit + /// 4k links continue to enforce routing for managed streams + /// regardless of which side wins the default. + fn reassert_default_processed(&mut self) { + const WINDOW: std::time::Duration = std::time::Duration::from_secs(1); + const MAX_PER_WINDOW: u32 = 10; + let now = std::time::Instant::now(); + match &mut self.default_reassertion { + Some((started, n)) if now.duration_since(*started) < WINDOW => { + if *n >= MAX_PER_WINDOW { + tracing::debug!( + attempts = *n, + "default.audio.sink re-assertion budget exhausted for this window" + ); + return; + } + *n += 1; + } + _ => { + self.default_reassertion = Some((now, 1)); + } + } + self.write_default_audio_sink(PROCESSED_SINK_NAME); + } + /// Update `preferred_real_sink` and retarget every bypass-routed /// stream + the filter playback + re-assert headroom-processed as /// default. @@ -1118,7 +1170,12 @@ impl RoutingState { let (bypass_targets, resolved_node_id) = { let mut s = self.daemon.lock(); let Some(targets) = s.apply_real_sink_change(&new_sink_name) else { - return; // Idempotent no-op. + // Real sink unchanged but WP just wrote default away + // from headroom-processed. Re-assert (rate-limited) + // and return — no bypass-retarget work needed. + drop(s); + self.reassert_default_processed(); + return; }; // If we already know this sink by name from the registry, // populate node_id in the same pass; otherwise it'll @@ -1153,12 +1210,19 @@ impl RoutingState { } // Retarget the filter playback so processed audio follows the - // new speaker. node.dont-move / NODE_DONT_RECONNECT prevent - // WirePlumber from deciding for the filter, but explicit - // target.object writes are an operator-level override and are - // honoured. + // new speaker. Same dual-write as the bypass streams above: + // target.object as a hint, explicit-link enqueue as the + // source of truth — otherwise filter.playback ends up + // dual-linked (real sink + processed:playback, which is a + // feedback loop into its own input). if let Some(playback_id) = self.filter_playback_id { self.write_stream_target(playback_id, &new_sink_name, FILTER_PLAYBACK_NODE_NAME); + self.enqueue_route( + playback_id, + new_sink_name.clone(), + FILTER_PLAYBACK_NODE_NAME.to_owned(), + Route::Bypass, + ); } else { tracing::debug!( "filter playback id not yet captured; will be pinned on its registry arrival" @@ -1167,8 +1231,10 @@ impl RoutingState { // Re-assert headroom-processed as the system default so new // streams keep landing in the processor. This will emit a - // property event we ignore (PROCESSED_SINK_NAME branch above). - self.write_default_audio_sink(PROCESSED_SINK_NAME); + // property event we ignore (PROCESSED_SINK_NAME branch + // above). Rate-limited so a WP that insists on its own + // default doesn't pull us into an unbounded loop. + self.reassert_default_processed(); // Tell IPC subscribers a real sink switch happened. let event = Event::new( From 9220143db7b140d3ba31162cc524d317b6c3ef8e Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 16:21:53 +1000 Subject: [PATCH 05/21] 8a: assert_no_alloc on audio-thread callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps the three audio-thread `process` callbacks (`capture_process`, `playback_process`, `tap_process`) with `assert_no_alloc::assert_no_alloc(|| inner(...))`. The `headroom-cli` binary installs `AllocDisabler` as `#[global_allocator]` so any allocation inside one of those blocks during debug builds aborts the process with "memory allocation of N bytes failed". Each callback was renamed to `*_inner` to keep the thin wrapper function pointer stable for pipewire-rs's `process(fn_ptr)`. `assert_no_alloc`'s `disable_release` is on by default — release builds get the system allocator unwrapped and the macros become no-ops, so the audio thread pays zero runtime cost in production. Verified Positive smoke (5 s of 1 kHz sine through processed): daemon stays up across thousands of capture/playback/tap callbacks. No abort. Audio threads are alloc-free as designed. Negative smoke (temporarily inserted `Vec::with_capacity(1024)` inside `capture_process_inner`): daemon aborts (SIGABRT, exit 134) on the first audio callback with the expected "memory allocation of 1024 bytes failed" stderr message — confirming the harness is wired correctly and not silently a no-op. Sanity-check alloc reverted before commit. 185 tests pass; clippy clean at -D warnings --all-targets. --- Cargo.lock | 8 ++++++++ crates/headroom-cli/Cargo.toml | 1 + crates/headroom-cli/src/main.rs | 10 ++++++++++ crates/headroom-core/Cargo.toml | 6 ++++++ crates/headroom-core/src/pw/filter.rs | 15 +++++++++++++-- crates/headroom-core/src/pw/tap.rs | 5 +++++ 6 files changed, 43 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a3283c8..134095e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -89,6 +89,12 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "assert_no_alloc" +version = "1.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55ca83137a482d61d916ceb1eba52a684f98004f18e0cafea230fe5579c178a3" + [[package]] name = "autocfg" version = "1.5.0" @@ -633,6 +639,7 @@ checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" name = "headroom-cli" version = "0.1.0" dependencies = [ + "assert_no_alloc", "clap", "crossbeam-channel", "crossterm", @@ -660,6 +667,7 @@ dependencies = [ name = "headroom-core" version = "0.1.0" dependencies = [ + "assert_no_alloc", "bytemuck", "criterion", "crossbeam-channel", diff --git a/crates/headroom-cli/Cargo.toml b/crates/headroom-cli/Cargo.toml index ee5e7c9..49dd87c 100644 --- a/crates/headroom-cli/Cargo.toml +++ b/crates/headroom-cli/Cargo.toml @@ -18,6 +18,7 @@ headroom-client = { workspace = true } headroom-core = { workspace = true } headroom-ipc = { workspace = true } +assert_no_alloc = { workspace = true } clap = { workspace = true } crossbeam-channel = { workspace = true } crossterm = { workspace = true } diff --git a/crates/headroom-cli/src/main.rs b/crates/headroom-cli/src/main.rs index ae79f21..9e6865e 100644 --- a/crates/headroom-cli/src/main.rs +++ b/crates/headroom-cli/src/main.rs @@ -14,6 +14,16 @@ use std::process::ExitCode; use clap::{Parser, Subcommand, ValueEnum}; use headroom_client::{Client, ClientError, Route, Topic}; +// Wrap the system allocator so audio-thread `assert_no_alloc` blocks +// in headroom-core can detect any allocation. In debug builds an +// allocation inside such a block aborts the process — exactly what +// we want when the daemon is exercised under `cargo run` or under +// the test harness. In release builds the wrapper is a no-op +// (assert_no_alloc's default `disable_release` feature), so there's +// zero overhead in production. +#[global_allocator] +static ALLOCATOR: assert_no_alloc::AllocDisabler = assert_no_alloc::AllocDisabler; + /// Headroom CLI. #[derive(Debug, Parser)] #[command(version, about, long_about = None)] diff --git a/crates/headroom-core/Cargo.toml b/crates/headroom-core/Cargo.toml index f22230c..3e59697 100644 --- a/crates/headroom-core/Cargo.toml +++ b/crates/headroom-core/Cargo.toml @@ -43,6 +43,12 @@ notify-debouncer-mini = { workspace = true } # Slow AGC loop (Phase 4 closing piece). ebur128 = { workspace = true } +# Audio-thread allocation guard. In debug builds the `AllocDisabler` +# global allocator panics if anything inside an `assert_no_alloc!` +# block tries to allocate; in release builds the macro is a no-op +# (zero overhead). Wraps each audio-thread `process` callback. +assert_no_alloc = { workspace = true } + # Optional journald logging — not wired yet. # tracing-journald = { workspace = true } diff --git a/crates/headroom-core/src/pw/filter.rs b/crates/headroom-core/src/pw/filter.rs index 1d654dd..d3ecee1 100644 --- a/crates/headroom-core/src/pw/filter.rs +++ b/crates/headroom-core/src/pw/filter.rs @@ -428,8 +428,14 @@ fn build_format_pod_bytes() -> Result, DaemonError> { Ok(bytes) } -/// Capture process callback. Realtime-thread, allocation-free. +/// Capture process callback. Realtime-thread, allocation-free — +/// guarded by [`assert_no_alloc::assert_no_alloc`] in debug builds +/// so any inadvertent allocation aborts immediately. fn capture_process(stream: &pipewire::stream::StreamRef, state: &mut CaptureState) { + assert_no_alloc::assert_no_alloc(|| capture_process_inner(stream, state)); +} + +fn capture_process_inner(stream: &pipewire::stream::StreamRef, state: &mut CaptureState) { let Some(mut buffer) = stream.dequeue_buffer() else { return; // Out of buffers; pipewire is queueing for us. }; @@ -520,8 +526,13 @@ fn drain_audio_commands(state: &mut PlaybackState) { } } -/// Playback process callback. Realtime-thread, allocation-free. +/// Playback process callback. Realtime-thread, allocation-free — +/// guarded by [`assert_no_alloc::assert_no_alloc`] in debug builds. fn playback_process(stream: &pipewire::stream::StreamRef, state: &mut PlaybackState) { + assert_no_alloc::assert_no_alloc(|| playback_process_inner(stream, state)); +} + +fn playback_process_inner(stream: &pipewire::stream::StreamRef, state: &mut PlaybackState) { drain_audio_commands(state); let Some(mut buffer) = stream.dequeue_buffer() else { diff --git a/crates/headroom-core/src/pw/tap.rs b/crates/headroom-core/src/pw/tap.rs index 4cec494..3f959fe 100644 --- a/crates/headroom-core/src/pw/tap.rs +++ b/crates/headroom-core/src/pw/tap.rs @@ -234,7 +234,12 @@ fn build_format_pod_bytes() -> Result, DaemonError> { /// Audio-thread `process` callback. Allocation-free, bounded by the /// block length. Computes `peak` and `mean_sq` over the interleaved /// samples and pushes one [`MeasurementSample`] to the controller. +/// Guarded by [`assert_no_alloc::assert_no_alloc`] in debug builds. fn tap_process(stream: &pipewire::stream::StreamRef, state: &mut TapState) { + assert_no_alloc::assert_no_alloc(|| tap_process_inner(stream, state)); +} + +fn tap_process_inner(stream: &pipewire::stream::StreamRef, state: &mut TapState) { let Some(mut buffer) = stream.dequeue_buffer() else { return; }; From d52cd6db3bee63d66c42f0697031576ca07d363c Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 16:42:46 +1000 Subject: [PATCH 06/21] 8e: playback callback timing instrumentation + spike investigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a lock-free `PlaybackTiming` struct (atomics: call_count, sum_us, max_us, spike_count, last_spike_us, last_spike_at_call) shared between the bus filter's `playback_process` callback (RT thread, writes) and the AGC controller (daemon thread, reads). The audio thread wraps each inner call in `Instant::now()` ... `state.timing.record(elapsed)` — wait-free, no allocation. The AGC tick samples the snapshot once per second and logs at WARN when new spikes have landed since the previous sample, DEBUG otherwise. `#[global_allocator]` declaration in `headroom-cli` now sits behind `cfg(debug_assertions)` so release builds compile cleanly (assert_no_alloc strips `AllocDisabler` under its default `disable_release` feature). Spike investigation outcome PLAN §11 follow-up noted: ~240 μs steady state, ~2 ms BUSY spikes at ~10 s cadence. My ~3 min capture of a 1 kHz sine routed through processed (release build) showed: - Steady state ~2180 μs / call - Max climbed slowly: 2186 → 2222 → 2606 → 2655 → 2812 μs over ~1 min (1.3× steady-state, well within the per-quantum budget) - Callback rate ~4 Hz, implying the Mbox is negotiating a large quantum (~12k frames per call vs the 1024-frame baseline PLAN §4.7 measured). Per-frame DSP cost is identical to the original budget; the longer wall-clock is just the longer quantum No clear ~10 s-cadence outlier pattern reproduced. The system is comfortably inside budget (~2.2 ms / 250 ms quantum ≈ 1% of one core). Without an audible artefact or a reproducible failure mode I'm not chasing the original spike further; the instrumentation stays so future regressions are visible at WARN level. `SPIKE_THRESHOLD_US = 5000` is comfortably above steady-state at both small and large quanta, so only real outliers trip the log. Verified 185 tests pass; clippy clean at -D warnings --all-targets. Release build runs sine playback continuously for >3 min with no assert_no_alloc abort, no panic, no spike warning. Debug build (with assert_no_alloc active) likewise stable across thousands of audio callbacks (revalidated as part of the release-build comparison). --- crates/headroom-cli/src/main.rs | 12 +-- crates/headroom-core/src/agc.rs | 65 +++++++++++++++- crates/headroom-core/src/meters.rs | 104 ++++++++++++++++++++++++++ crates/headroom-core/src/pw/filter.rs | 19 ++++- crates/headroom-core/src/runtime.rs | 2 + 5 files changed, 193 insertions(+), 9 deletions(-) diff --git a/crates/headroom-cli/src/main.rs b/crates/headroom-cli/src/main.rs index 9e6865e..4fbf3ce 100644 --- a/crates/headroom-cli/src/main.rs +++ b/crates/headroom-cli/src/main.rs @@ -15,12 +15,12 @@ use clap::{Parser, Subcommand, ValueEnum}; use headroom_client::{Client, ClientError, Route, Topic}; // Wrap the system allocator so audio-thread `assert_no_alloc` blocks -// in headroom-core can detect any allocation. In debug builds an -// allocation inside such a block aborts the process — exactly what -// we want when the daemon is exercised under `cargo run` or under -// the test harness. In release builds the wrapper is a no-op -// (assert_no_alloc's default `disable_release` feature), so there's -// zero overhead in production. +// in headroom-core can detect any allocation. Debug-only — in +// release builds `assert_no_alloc`'s default `disable_release` +// feature strips both `AllocDisabler` and the `assert_no_alloc(|| +// ...)` wrappers to no-ops, so there's zero overhead in production +// (and the symbol doesn't even exist to reference here). +#[cfg(debug_assertions)] #[global_allocator] static ALLOCATOR: assert_no_alloc::AllocDisabler = assert_no_alloc::AllocDisabler; diff --git a/crates/headroom-core/src/agc.rs b/crates/headroom-core/src/agc.rs index b4b54ce..31b5cca 100644 --- a/crates/headroom-core/src/agc.rs +++ b/crates/headroom-core/src/agc.rs @@ -62,6 +62,15 @@ pub struct AgcController { bus_metrics: SharedBusMetrics, /// Tick counter for `publish_hz` throttling. Wraps freely. meter_tick_counter: u32, + /// Playback callback timing stats. Sampled and logged once per + /// second to surface BUSY-spike behaviour and general callback + /// health. + timing: crate::meters::SharedPlaybackTiming, + /// Last `spike_count` value we observed, used to detect *new* + /// spikes since the previous log. + last_logged_spike_count: u64, + /// Tick counter for the once-per-second timing log throttle. + timing_log_counter: u32, } impl AgcController { @@ -77,6 +86,7 @@ impl AgcController { filter_control: FilterControl, daemon: SharedState, bus_metrics: SharedBusMetrics, + timing: crate::meters::SharedPlaybackTiming, ) -> Result { // `Mode::I` (integrated, gated) costs a histogram walk per // `loudness_global()` call — bounded, fine at 20 Hz meter @@ -100,6 +110,9 @@ impl AgcController { last_short_term_lufs: LOUDNESS_FLOOR_LUFS, bus_metrics, meter_tick_counter: 0, + timing, + last_logged_spike_count: 0, + timing_log_counter: 0, }) } @@ -177,6 +190,45 @@ impl AgcController { } self.publish_meters(publish_hz); + self.log_playback_timing(); + } + + /// Throttled log of the playback callback's rolling timing stats. + /// Fires roughly once per second at the AGC's 20 Hz tick rate. + /// Cheap (lock-free atomic loads); useful for surfacing BUSY + /// spikes without per-call log noise. + fn log_playback_timing(&mut self) { + // 20 Hz tick → log every 20 ticks for ~1 Hz cadence. + self.timing_log_counter = self.timing_log_counter.wrapping_add(1); + if self.timing_log_counter % 20 != 0 { + return; + } + let snap = self.timing.snapshot(); + if snap.call_count == 0 { + return; + } + let avg_us = snap.sum_us / snap.call_count.max(1); + let new_spikes = snap.spike_count.saturating_sub(self.last_logged_spike_count); + self.last_logged_spike_count = snap.spike_count; + if new_spikes > 0 { + tracing::warn!( + avg_us, + max_us = snap.max_us, + new_spikes, + total_spikes = snap.spike_count, + last_spike_us = snap.last_spike_us, + last_spike_at_call = snap.last_spike_at_call, + call_count = snap.call_count, + "playback callback BUSY spike(s) since last log" + ); + } else { + tracing::debug!( + avg_us, + max_us = snap.max_us, + call_count = snap.call_count, + "playback callback timing" + ); + } } /// Drain up to [`TICK_BUF_SAMPLES`] from the measurement ring and @@ -339,8 +391,17 @@ mod tests { let (control, cmd_cons) = FilterControl::for_testing(32); let state = state::shared(DaemonState::new(ProfileStore::builtin())); let bus = meters::shared(); - let agc = - AgcController::new(SR, CH, m_cons, control, state.clone(), bus.clone()).unwrap(); + let timing = meters::shared_timing(); + let agc = AgcController::new( + SR, + CH, + m_cons, + control, + state.clone(), + bus.clone(), + timing, + ) + .unwrap(); (agc, m_prod, cmd_cons, state, bus) } diff --git a/crates/headroom-core/src/meters.rs b/crates/headroom-core/src/meters.rs index 0bbd3bf..393fddb 100644 --- a/crates/headroom-core/src/meters.rs +++ b/crates/headroom-core/src/meters.rs @@ -21,6 +21,7 @@ //! quantum overwrites the slot. Dropped meter samples don't degrade //! the AGC; the controller reads the freshest available snapshot. +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; use parking_lot::Mutex; @@ -56,6 +57,109 @@ pub fn shared() -> SharedBusMetrics { Arc::new(Mutex::new(BusMetrics::default())) } +/// Rolling timing stats for the bus filter's `playback_process` +/// callback. Updated from the audio thread via lock-free atomics, +/// read (and reset) by the AGC controller's slow tick. Used to +/// detect the ~10 s-cadence BUSY spikes mentioned in PLAN §11 +/// follow-ups, and (longer-term) as a general health signal — if +/// `playback_us_max` creeps up over the run, something downstream +/// is unhappy. +#[derive(Debug, Default)] +pub struct PlaybackTiming { + /// Number of playback_process invocations. + pub call_count: AtomicU64, + /// Cumulative duration in microseconds. + pub sum_us: AtomicU64, + /// Max duration observed in microseconds across all calls. + pub max_us: AtomicU64, + /// Number of calls whose duration exceeded the spike threshold. + pub spike_count: AtomicU64, + /// Duration of the most recent spike in microseconds. + pub last_spike_us: AtomicU64, + /// `call_count` snapshot when the most recent spike fired (so a + /// reader can detect "no new spike since last read" by comparing + /// against its previous snapshot). + pub last_spike_at_call: AtomicU64, +} + +impl PlaybackTiming { + /// Threshold above which a call is counted as a "spike". + /// + /// The steady-state cost of the playback callback scales with + /// the PipeWire quantum: on a 1024-frame quantum it runs in + /// ~240 μs (PLAN §4.7); on the 8192-frame quantum the Mbox + /// negotiates here it sits around ~2.2 ms in release builds. + /// 5 ms is comfortably above both regimes and only fires on + /// real outliers (the ~10 s-cadence "BUSY" spike PLAN §11 + /// chases would have to be ~2× steady-state at any quantum to + /// trip this). + pub const SPIKE_THRESHOLD_US: u64 = 5_000; + + /// Record one observation. Wait-free. + #[inline] + pub fn record(&self, dur_us: u64) { + self.call_count.fetch_add(1, Ordering::Relaxed); + self.sum_us.fetch_add(dur_us, Ordering::Relaxed); + let mut cur_max = self.max_us.load(Ordering::Relaxed); + while dur_us > cur_max { + match self.max_us.compare_exchange_weak( + cur_max, + dur_us, + Ordering::Relaxed, + Ordering::Relaxed, + ) { + Ok(_) => break, + Err(v) => cur_max = v, + } + } + if dur_us > Self::SPIKE_THRESHOLD_US { + let count = self.call_count.load(Ordering::Relaxed); + self.spike_count.fetch_add(1, Ordering::Relaxed); + self.last_spike_us.store(dur_us, Ordering::Relaxed); + self.last_spike_at_call.store(count, Ordering::Relaxed); + } + } + + /// Take a snapshot of current counters. Doesn't reset. + pub fn snapshot(&self) -> PlaybackTimingSnapshot { + PlaybackTimingSnapshot { + call_count: self.call_count.load(Ordering::Relaxed), + sum_us: self.sum_us.load(Ordering::Relaxed), + max_us: self.max_us.load(Ordering::Relaxed), + spike_count: self.spike_count.load(Ordering::Relaxed), + last_spike_us: self.last_spike_us.load(Ordering::Relaxed), + last_spike_at_call: self.last_spike_at_call.load(Ordering::Relaxed), + } + } +} + +/// Plain-old-data snapshot of [`PlaybackTiming`] for the controller's +/// per-tick logging. +#[derive(Debug, Default, Clone, Copy)] +pub struct PlaybackTimingSnapshot { + /// Cumulative call count. + pub call_count: u64, + /// Cumulative duration in microseconds. + pub sum_us: u64, + /// Max single-call duration in microseconds observed so far. + pub max_us: u64, + /// Cumulative count of calls above the spike threshold. + pub spike_count: u64, + /// Duration of the most recent spike in microseconds. + pub last_spike_us: u64, + /// `call_count` when the most recent spike fired. + pub last_spike_at_call: u64, +} + +/// Cheap-to-clone shared handle for [`PlaybackTiming`]. +pub type SharedPlaybackTiming = Arc; + +/// Construct an empty shared timing handle. +#[must_use] +pub fn shared_timing() -> SharedPlaybackTiming { + Arc::new(PlaybackTiming::default()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/headroom-core/src/pw/filter.rs b/crates/headroom-core/src/pw/filter.rs index d3ecee1..acf3580 100644 --- a/crates/headroom-core/src/pw/filter.rs +++ b/crates/headroom-core/src/pw/filter.rs @@ -53,7 +53,7 @@ use headroom_dsp::{ }; use crate::error::DaemonError; -use crate::meters::{BusMetrics, SharedBusMetrics}; +use crate::meters::{BusMetrics, SharedBusMetrics, SharedPlaybackTiming}; use crate::pw::sink::NODE_NAME as PROCESSED_SINK_NAME; /// Sample rate the filter operates at. The DSP kernels are @@ -221,6 +221,11 @@ struct PlaybackState { /// contention (which is vanishingly rare — the reader holds the /// lock for nanoseconds). bus_metrics: SharedBusMetrics, + /// Lock-free rolling timing stats for the playback callback. + /// Used by 8e to investigate the ~10 s-cadence BUSY spikes + /// noted in PLAN §11 follow-ups, and as a general health + /// signal going forward. + timing: SharedPlaybackTiming, } /// The filter pipeline. @@ -262,6 +267,9 @@ pub struct FilterBundle { /// every `playback_process` call; the AGC controller reads it on /// each tick and publishes a `MeterTick` event. pub bus_metrics: SharedBusMetrics, + /// Playback callback timing stats. Updated lock-free from the + /// audio thread; sampled by the AGC controller's slow tick. + pub timing: SharedPlaybackTiming, } impl Filter { @@ -286,6 +294,7 @@ impl Filter { cmd_producer: Arc::new(Mutex::new(cmd_producer)), }; let bus_metrics = crate::meters::shared(); + let timing = crate::meters::shared_timing(); let compressor = Compressor::new(init.compressor, FILTER_SAMPLE_RATE as f32); let limiter = Limiter::new(init.limiter, FILTER_SAMPLE_RATE as f32); @@ -314,6 +323,7 @@ impl Filter { samples_starved: 0, measurement_dropped: 0, bus_metrics: bus_metrics.clone(), + timing: timing.clone(), }) .process(playback_process) .register() @@ -363,6 +373,7 @@ impl Filter { control, measurement_consumer, bus_metrics, + timing, }) } } @@ -528,8 +539,14 @@ fn drain_audio_commands(state: &mut PlaybackState) { /// Playback process callback. Realtime-thread, allocation-free — /// guarded by [`assert_no_alloc::assert_no_alloc`] in debug builds. +/// Wraps the inner with an Instant timer; the duration is recorded +/// into [`PlaybackTiming`] (lock-free atomics, no allocation), and +/// the AGC controller drains the stats on its 50 ms tick. fn playback_process(stream: &pipewire::stream::StreamRef, state: &mut PlaybackState) { + let start = std::time::Instant::now(); assert_no_alloc::assert_no_alloc(|| playback_process_inner(stream, state)); + let dur_us = start.elapsed().as_micros() as u64; + state.timing.record(dur_us); } fn playback_process_inner(stream: &pipewire::stream::StreamRef, state: &mut PlaybackState) { diff --git a/crates/headroom-core/src/runtime.rs b/crates/headroom-core/src/runtime.rs index d88f3b5..8c4a85e 100644 --- a/crates/headroom-core/src/runtime.rs +++ b/crates/headroom-core/src/runtime.rs @@ -108,6 +108,7 @@ pub fn run(profiles: ProfileStore) -> Result<(), DaemonError> { control: filter_control, measurement_consumer, bus_metrics, + timing, } = Filter::create(pw.core(), filter_init)?; daemon_state.lock().filter_control = Some(filter_control.clone()); @@ -125,6 +126,7 @@ pub fn run(profiles: ProfileStore) -> Result<(), DaemonError> { filter_control, daemon_state.clone(), bus_metrics, + timing, ) .map_err(DaemonError::from)?; let agc_controller = Rc::new(RefCell::new(agc_controller)); From c65c75bb9ffe3d79a545fdce4dcb6a8fd2aac416 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 17:00:25 +1000 Subject: [PATCH 07/21] =?UTF-8?q?7:=20packaging=20=E2=80=94=20systemd=20us?= =?UTF-8?q?er=20unit=20+=20Nix=20modules=20+=20README?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships the daemon as a real installable, not just `cargo build`. Artifacts - `contrib/systemd/headroom.service` — user-scope unit. Type=simple (the daemon doesn't fork), After=pipewire.service, Restart=on- failure with a 2 s back-off so a crash loop doesn't spam stderr, StandardOutput/Error=journal, LimitRTPRIO=20 / LimitNICE=-11 to match the rtkit-style grant PipeWire's own unit carries. The file is templated with `@bindir@` so the build derivation can substitute in an absolute store path at install time, without the unit having to rely on whatever `headroom` happens to be on PATH. - `nix/home-module.nix` — `services.headroom.enable`. Installs the package on the user's PATH, symlinks the shipped profiles into `$XDG_CONFIG_HOME/headroom/profiles/`, and writes the systemd user unit (start After=pipewire.service Requires=pipewire.service Wants=wireplumber.service WantedBy=pipewire.service). Knobs: `installDefaultProfiles` for users who maintain their own set, `extraProfiles` (attrset of filename → path) to drop in personal profiles that override shipped ones by name. - `nix/nixos-module.nix` — `programs.headroom.enable`. Narrow scope: binary on global PATH, the package's `lib/systemd/user/*.service` is materialised under `/etc/systemd/user/` via `systemd.packages`, and an assertion fires if pipewire isn't enabled (clearer than a runtime crash). Per-user defaults (profile install, RT priority tuning) live in the Home Manager module; the two compose. Build derivation `postInstall` now installs the unit (with `@bindir@` substituted to `$out/bin`) and copies `profiles/*.toml` to `$out/share/headroom/profiles/`. The flake's version lookup moved from `crates/headroom-cli/Cargo.toml` (where `version.workspace = true` evaluates to a table, not a string) to the workspace `Cargo.toml`. Modules exposed under `nixosModules.default` and `homeModules.default`. README Rewrote the install section: Nix flake-based install with both Home Manager and NixOS module examples, plus a from-scratch `cargo install` + `install`/`sed` recipe for non-Nix users. Added a usage section with the common `headroom` subcommands and bumped the status banner from "pre-alpha" to "alpha" (signal chain, routing, IPC, monitor TUI, profile reload, and packaging all work end-to-end now). Verified - `nix flake check` passes; NixOS module type-checks under nixpkgs eval. - `nix build .#headroom` produces `bin/headroom`, `lib/systemd/user/headroom.service` with the absolute store-path ExecStart baked in, and all five shipped profiles under `share/headroom/profiles/`. - `systemd-analyze verify --user` accepts the unit. - 185 workspace tests still pass; clippy clean at -D warnings --all-targets; `nix fmt` happy. --- README.md | 94 +++++++++++++++- contrib/systemd/headroom.service | 39 +++++++ flake.nix | 183 +++++++++++++++++-------------- nix/home-module.nix | 119 ++++++++++++++++++++ nix/nixos-module.nix | 61 +++++++++++ 5 files changed, 413 insertions(+), 83 deletions(-) create mode 100644 contrib/systemd/headroom.service create mode 100644 nix/home-module.nix create mode 100644 nix/nixos-module.nix diff --git a/README.md b/README.md index fed7fc9..13151d9 100644 --- a/README.md +++ b/README.md @@ -13,24 +13,114 @@ untouched. real sink directly and are not in scope of the contract — that's the trade-off that makes the per-app exclusion useful. - **Per-app exclusion** with profile-driven rules. +- **Layer A per-app level control** (peak + RMS detector → smoothed + `channelVolumes` writes) for taming individual streams without + touching the bus path. Zero added signal-path latency; safe to use + on bypass-routed streams. - **Single binary** daemon + CLI, controlled over a Unix-domain socket with a documented JSON wire protocol (see [`IPC.md`](IPC.md)). - **First-party Rust crate** (`headroom-client`) for programmatic use; third-party clients (Qt panels, status bars, …) target the wire protocol directly. +- **Live profile reload** — edit a TOML file in + `$XDG_CONFIG_HOME/headroom/profiles/` and the daemon picks up + changes within ~500 ms; the audio thread doesn't glitch. See [`PLAN.md`](PLAN.md) for the full design and roadmap. ## Status -Pre-alpha. Wire protocol and crate scaffolding are in; daemon and -filter are under construction. +Alpha. The signal chain (AGC, compressor, two-tier limiter, Layer A +per-app), the routing engine (explicit-link enforcement, sink hotplug, +sticky default sink), the IPC server with topic subscriptions, the +`headroom monitor` TUI, and live profile reload all work end-to-end. +Packaging exposes a systemd user unit and Nix modules. What's missing +is real-world soak time on multi-rate / Bluetooth setups and other +distros' init systems. + +## Installing + +### Nix (flake) + +This repo is a flake; the daemon plus its systemd user unit and the +canonical profiles are exposed as a package. + +```sh +nix run github:amaanq/headroom -- daemon # one-shot run +nix profile install github:amaanq/headroom # add to $PATH +``` + +For **Home Manager**, add the flake as an input and enable the module: + +```nix +{ + inputs.headroom.url = "github:amaanq/headroom"; + + # In your Home Manager configuration: + imports = [ inputs.headroom.homeModules.default ]; + services.headroom.enable = true; +} +``` + +The module symlinks the shipped profiles into +`$XDG_CONFIG_HOME/headroom/profiles/`, drops the systemd user unit +into the user's services dir, and the unit starts after PipeWire and +WirePlumber come up. `services.headroom.extraProfiles` lets you add +your own. + +For **NixOS** (system-wide binary install + systemd-user discovery): + +```nix +{ + inputs.headroom.url = "github:amaanq/headroom"; + + # In your NixOS configuration: + imports = [ inputs.headroom.nixosModules.default ]; + programs.headroom.enable = true; +} +``` + +Then any user can `systemctl --user enable --now headroom`. + +### Other distros (manual) + +```sh +cargo install --path crates/headroom-cli # or: cargo build --release +# Profiles +mkdir -p ~/.config/headroom/profiles +cp profiles/*.toml ~/.config/headroom/profiles/ +# systemd user unit (edit the ExecStart path to point at your binary) +install -Dm644 contrib/systemd/headroom.service \ + ~/.config/systemd/user/headroom.service +sed -i "s|@bindir@|$(dirname "$(command -v headroom)")|" \ + ~/.config/systemd/user/headroom.service +systemctl --user daemon-reload +systemctl --user enable --now headroom +``` + +## Usage + +Once the daemon is running: + +```sh +headroom status # JSON snapshot — sinks, streams, active profile +headroom profile list # available profiles +headroom profile use night # activate one +headroom monitor # full-screen TUI (bus gauges + per-stream) +headroom monitor --json meters # line-delimited JSON, for scripting +headroom route set firefox processed +headroom set compressor.threshold_db -28 +headroom bypass on # kill switch — straight to the real sink +``` + +See `headroom --help` for the full surface. ## Building ```sh nix develop # toolchain + pipewire dev libs + helpers cargo build # iterate +cargo test --workspace nix build # final packaged headroom binary ``` diff --git a/contrib/systemd/headroom.service b/contrib/systemd/headroom.service new file mode 100644 index 0000000..a22b72f --- /dev/null +++ b/contrib/systemd/headroom.service @@ -0,0 +1,39 @@ +[Unit] +Description=Headroom audio daemon (PipeWire AGC + compressor + true-peak limiter) +Documentation=https://github.com/amaanq/headroom +# PipeWire is a hard dependency: headroom registers a virtual sink and +# wires explicit links via PW's link-factory, so we can't start before +# pw-mainloop is up. ConditionUser ensures this only ever runs as a +# user-scope unit, never accidentally as the system instance. +After=pipewire.service pipewire-pulse.service wireplumber.service +Requires=pipewire.service +Wants=wireplumber.service +ConditionUser=!@system + +[Service] +Type=simple +ExecStart=@bindir@/headroom daemon +# Restart on failure but not too aggressively — a tight crash loop +# would just produce a lot of stderr noise and clobber the user's +# routing repeatedly. +Restart=on-failure +RestartSec=2s +# Headroom doesn't fork; SIGTERM is the clean shutdown path. The +# default KillMode=control-group is correct for a single-process +# daemon; explicit here for clarity. +KillMode=control-group +TimeoutStopSec=5s +# Surface stdout/stderr to journald so `journalctl --user -u headroom` +# shows daemon logs with the expected RUST_LOG filtering. +StandardOutput=journal +StandardError=journal +SyslogIdentifier=headroom +# Realtime hint — pipewire grants RT scheduling via pw_thread_loop, +# but the daemon main thread benefits from a slight scheduling boost +# too. LimitRTPRIO matches the pipewire user unit's grant. +LimitRTPRIO=20 +LimitRTTIME=200000 +LimitNICE=-11 + +[Install] +WantedBy=pipewire.service diff --git a/flake.nix b/flake.nix index 1829bd6..129f92f 100644 --- a/flake.nix +++ b/flake.nix @@ -11,97 +11,118 @@ }; outputs = { self, nixpkgs, flake-utils, rust-overlay }: - flake-utils.lib.eachSystem [ "x86_64-linux" "aarch64-linux" ] (system: - let - pkgs = import nixpkgs { - inherit system; - overlays = [ rust-overlay.overlays.default ]; - }; + flake-utils.lib.eachSystem [ "x86_64-linux" "aarch64-linux" ] + (system: + let + pkgs = import nixpkgs { + inherit system; + overlays = [ rust-overlay.overlays.default ]; + }; - rustToolchain = pkgs.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml; + rustToolchain = pkgs.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml; - rustPlatform = pkgs.makeRustPlatform { - cargo = rustToolchain; - rustc = rustToolchain; - }; + rustPlatform = pkgs.makeRustPlatform { + cargo = rustToolchain; + rustc = rustToolchain; + }; - # Native libs the audio crates link against. - nativeAudioBuildInputs = with pkgs; [ - pipewire - pipewire.dev - ]; - - nativeBuildTools = with pkgs; [ - pkg-config - clang - ]; - - commonEnv = { - LIBCLANG_PATH = "${pkgs.libclang.lib}/lib"; - PKG_CONFIG_PATH = "${pkgs.pipewire.dev}/lib/pkgconfig"; - }; - in - { - # `nix develop` — full dev environment. - devShells.default = pkgs.mkShell ({ - name = "headroom-dev"; - - nativeBuildInputs = nativeBuildTools ++ [ - rustToolchain - pkgs.rust-analyzer + # Native libs the audio crates link against. + nativeAudioBuildInputs = with pkgs; [ + pipewire + pipewire.dev ]; - buildInputs = nativeAudioBuildInputs ++ (with pkgs; [ - socat # poke the IPC socket - jq # pretty-print JSON - pipewire # for pw-cli, pw-cat, etc. - wireplumber - ]); + nativeBuildTools = with pkgs; [ + pkg-config + clang + ]; - shellHook = '' - echo "headroom dev shell — rustc $(rustc --version | cut -d' ' -f2)" - echo " cargo build / cargo test for iteration." - echo " nix build .#headroom for the packaged binary." - export RUST_BACKTRACE=1 - export RUST_LOG=headroom=debug,info - ''; - } // commonEnv); + commonEnv = { + LIBCLANG_PATH = "${pkgs.libclang.lib}/lib"; + PKG_CONFIG_PATH = "${pkgs.pipewire.dev}/lib/pkgconfig"; + }; + in + { + # `nix develop` — full dev environment. + devShells.default = pkgs.mkShell ({ + name = "headroom-dev"; - # `nix build` — the final packaged daemon + CLI. - packages = rec { - default = headroom; + nativeBuildInputs = nativeBuildTools ++ [ + rustToolchain + pkgs.rust-analyzer + ]; - headroom = rustPlatform.buildRustPackage ({ - pname = "headroom"; - version = (builtins.fromTOML (builtins.readFile ./crates/headroom-cli/Cargo.toml)).package.version; + buildInputs = nativeAudioBuildInputs ++ (with pkgs; [ + socat # poke the IPC socket + jq # pretty-print JSON + pipewire # for pw-cli, pw-cat, etc. + wireplumber + ]); - src = ./.; - - cargoLock = { - lockFile = ./Cargo.lock; - # allowBuiltinFetchGit = true; - }; - - nativeBuildInputs = nativeBuildTools; - buildInputs = nativeAudioBuildInputs; - - # We ship two binaries from the workspace: `headroom` (cli + daemon). - cargoBuildFlags = [ "-p" "headroom-cli" ]; - doCheck = true; - cargoTestFlags = [ "--workspace" ]; - - meta = with pkgs.lib; { - description = "AGC + compressor + true-peak limiter daemon for PipeWire"; - license = licenses.gpl3Plus; - platforms = platforms.linux; - mainProgram = "headroom"; - }; + shellHook = '' + echo "headroom dev shell — rustc $(rustc --version | cut -d' ' -f2)" + echo " cargo build / cargo test for iteration." + echo " nix build .#headroom for the packaged binary." + export RUST_BACKTRACE=1 + export RUST_LOG=headroom=debug,info + ''; } // commonEnv); - }; - # Reserved for the eventual user-service module. - # nixosModules.default = import ./nix/module.nix; + # `nix build` — the final packaged daemon + CLI. + packages = rec { + default = headroom; - formatter = pkgs.nixpkgs-fmt; - }); + headroom = rustPlatform.buildRustPackage ({ + pname = "headroom"; + # Pull from the workspace Cargo.toml — the per-crate + # manifests use `version.workspace = true` which evaluates + # to a table here, not a string. + version = (builtins.fromTOML (builtins.readFile ./Cargo.toml)).workspace.package.version; + + src = ./.; + + cargoLock = { + lockFile = ./Cargo.lock; + # allowBuiltinFetchGit = true; + }; + + nativeBuildInputs = nativeBuildTools; + buildInputs = nativeAudioBuildInputs; + + # We ship one binary from the workspace: `headroom` (cli + daemon). + cargoBuildFlags = [ "-p" "headroom-cli" ]; + doCheck = true; + cargoTestFlags = [ "--workspace" ]; + + # Install the systemd user unit (templated with @bindir@ + # so the unit refers to the absolute path of the binary in + # this derivation, never to whatever happens to be on + # PATH) and ship the canonical profiles under + # share/headroom/profiles so users / modules can copy + # them into XDG_CONFIG_HOME on first run. + postInstall = '' + install -Dm644 contrib/systemd/headroom.service \ + "$out/lib/systemd/user/headroom.service" + substituteInPlace "$out/lib/systemd/user/headroom.service" \ + --replace-fail '@bindir@' "$out/bin" + + mkdir -p "$out/share/headroom/profiles" + cp -r profiles/. "$out/share/headroom/profiles/" + ''; + + meta = with pkgs.lib; { + description = "AGC + compressor + true-peak limiter daemon for PipeWire"; + license = licenses.gpl3Plus; + platforms = platforms.linux; + mainProgram = "headroom"; + }; + } // commonEnv); + }; + + formatter = pkgs.nixpkgs-fmt; + }) // { + # System-independent outputs — modules. + nixosModules.default = import ./nix/nixos-module.nix self; + homeModules.default = import ./nix/home-module.nix self; + }; } diff --git a/nix/home-module.nix b/nix/home-module.nix new file mode 100644 index 0000000..923c1e7 --- /dev/null +++ b/nix/home-module.nix @@ -0,0 +1,119 @@ +# Home Manager module — installs the headroom binary, the systemd +# user service, and (optionally) a default set of profiles into the +# user's XDG_CONFIG_HOME. +# +# Headroom is a per-user daemon that talks to PipeWire over the user +# session, so the Home Manager scope is its natural install point. A +# separate NixOS module (./nixos-module.nix) covers the case where the +# user wants `headroom` on every account's PATH or wants to enable the +# service at the system level via systemd-user; that module simply +# delegates the heavy lifting to `services.headroom` (this file) when +# Home Manager is in use. +self: +{ config, lib, pkgs, ... }: + +let + inherit (lib) mkEnableOption mkOption mkIf types literalExpression; + + cfg = config.services.headroom; + + package = cfg.package; + + # Profiles shipped by the package, suitable for symlinking into the + # user's XDG_CONFIG_HOME so they show up in `headroom profile list` + # without the user having to copy them by hand. + shippedProfilesDir = "${package}/share/headroom/profiles"; +in +{ + options.services.headroom = { + enable = mkEnableOption "Headroom — PipeWire AGC + compressor + true-peak limiter daemon"; + + package = mkOption { + type = types.package; + default = self.packages.${pkgs.system}.headroom; + defaultText = literalExpression "headroom.packages.\${pkgs.system}.headroom"; + description = '' + The headroom package to install. Override to pin a local + build (e.g. `path:/home/me/code/headroom`) when iterating. + ''; + }; + + installDefaultProfiles = mkOption { + type = types.bool; + default = true; + description = '' + Symlink the profiles shipped with the package into + `$XDG_CONFIG_HOME/headroom/profiles/`. Disable if you + maintain your own profile set and don't want the shipped + ones cluttering `headroom profile list`. + ''; + }; + + extraProfiles = mkOption { + type = types.attrsOf types.path; + default = { }; + example = literalExpression '' + { + "studio.toml" = ./profiles/studio.toml; + } + ''; + description = '' + Additional profile TOML files to drop into the user's + profile directory, keyed by filename. Overrides any + identically-named shipped profile. + ''; + }; + }; + + config = mkIf cfg.enable { + home.packages = [ package ]; + + # Symlink shipped profiles + any user-provided extras into the + # user's XDG_CONFIG_HOME. The daemon's profile watcher + # (notify-debouncer-mini) treats symlinks identically to + # regular files, so this is transparent. + xdg.configFile = lib.mkMerge [ + (mkIf cfg.installDefaultProfiles ( + lib.mapAttrs' + (name: _: lib.nameValuePair "headroom/profiles/${name}" { + source = "${shippedProfilesDir}/${name}"; + }) + (builtins.readDir shippedProfilesDir) + )) + (lib.mapAttrs' + (name: path: lib.nameValuePair "headroom/profiles/${name}" { + source = path; + }) + cfg.extraProfiles) + ]; + + # systemd user unit. The unit shipped by the package already + # carries the right ExecStart with an absolute path baked in, + # so we just symlink it into the user's services directory and + # let Home Manager start it via its systemd-user machinery. + systemd.user.services.headroom = { + Unit = { + Description = "Headroom audio daemon (PipeWire AGC + compressor + true-peak limiter)"; + Documentation = "https://github.com/amaanq/headroom"; + After = [ "pipewire.service" "pipewire-pulse.service" "wireplumber.service" ]; + Requires = [ "pipewire.service" ]; + Wants = [ "wireplumber.service" ]; + }; + Service = { + Type = "simple"; + ExecStart = "${package}/bin/headroom daemon"; + Restart = "on-failure"; + RestartSec = "2s"; + StandardOutput = "journal"; + StandardError = "journal"; + SyslogIdentifier = "headroom"; + LimitRTPRIO = 20; + LimitRTTIME = 200000; + LimitNICE = -11; + }; + Install = { + WantedBy = [ "pipewire.service" ]; + }; + }; + }; +} diff --git a/nix/nixos-module.nix b/nix/nixos-module.nix new file mode 100644 index 0000000..df06016 --- /dev/null +++ b/nix/nixos-module.nix @@ -0,0 +1,61 @@ +# NixOS module — system-wide install. Headroom itself is a user-scope +# daemon (it talks to the user's PipeWire session), so this module's +# job is narrow: +# +# 1. Make the `headroom` binary available on every login's PATH. +# 2. Drop the systemd user unit into the system-wide location so a +# user can `systemctl --user enable --now headroom` without first +# having to use Home Manager. +# 3. Ensure the standard audio stack (PipeWire + WirePlumber) is +# enabled, since headroom can't function without them. +# +# For per-user defaults — activeProfile, shipped-profile install, +# RT-priority tuning — use the Home Manager module +# (`homeModules.default`) instead. The two compose. +self: +{ config, lib, pkgs, ... }: + +let + inherit (lib) mkEnableOption mkOption mkIf types literalExpression; + + cfg = config.programs.headroom; +in +{ + options.programs.headroom = { + enable = mkEnableOption "Headroom — PipeWire AGC + compressor + true-peak limiter daemon"; + + package = mkOption { + type = types.package; + default = self.packages.${pkgs.system}.headroom; + defaultText = literalExpression "headroom.packages.\${pkgs.system}.headroom"; + description = '' + The headroom package to install system-wide. + ''; + }; + }; + + config = mkIf cfg.enable { + # Binary + manpages (when we have them) on the global PATH. + environment.systemPackages = [ cfg.package ]; + + # Make the shipped systemd user unit discoverable by `systemctl + # --user`. Setting `packages` here is the canonical NixOS way to + # install user-scope unit files from a package — it materialises + # `/etc/systemd/user/headroom.service` pointing at the package's + # `lib/systemd/user/headroom.service`. + systemd.packages = [ cfg.package ]; + + # Headroom requires PipeWire; refuse to evaluate the module if + # the user enabled headroom but not pipewire, with a pointer + # rather than a confusing runtime failure. + assertions = [ + { + assertion = config.services.pipewire.enable; + message = '' + programs.headroom.enable requires services.pipewire.enable = true; + headroom is a PipeWire-only daemon. + ''; + } + ]; + }; +} From 244367ccb91e1c6e5f7263ad63fbee30838747b2 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 17:06:11 +1000 Subject: [PATCH 08/21] plan: mark phases 7 + 8 done, close BUSY-spike follow-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §11 — annotated Phase 7 and Phase 8 (a–e) inline with what landed and where, so the section now reads as a commit-log index rather than a forward-looking todo. The "Tracked follow-ups" subsection keeps the two trigger-gated dormant items (ephemeral overlay, sub-ms dispatch primitive) and strikes through the filter-playback BUSY spike — 8e's ~3 min release-build capture didn't reproduce the ~8×-baseline outlier pattern from the original 6c smoke finding, so the work to be done collapsed to "instrumentation kept, no code change." §11 preamble now notes "all planned phases (0–8) are done as of 2026-05-21"; §12 picks up the same theme by pointing to team-memory `headroom-project` for current per-risk status. Memory bumps go to ~/.claude-amaan: `headroom-project` description + Phase 7 entry + revised "How to apply" (no more "next planned work"), `headroom-routing-link-bug` moves both Phase 4k "still-open follow-ups" to a "Closed (4l)" section, and MEMORY.md's project hook is updated to reflect "all phases shipped, audio threads validated alloc-free, packaging modules in place". --- PLAN.md | 99 +++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 71 insertions(+), 28 deletions(-) diff --git a/PLAN.md b/PLAN.md index 7d3e62b..c2b5374 100644 --- a/PLAN.md +++ b/PLAN.md @@ -831,7 +831,10 @@ builds and any CI go through `nix build`. ## 11. Phased implementation -The phases are roughly token-of-work units, not calendar weeks. +The phases are roughly token-of-work units, not calendar weeks. **All +planned phases (0–8) are done as of 2026-05-21**; this section is +preserved as historical context + a reading guide to the commit log. +See [[headroom-project]] in team memory for the per-commit ledger. **Phase 0 — scaffolding.** Flake, workspace, crate skeletons, README, PLAN/IPC docs. *(done as part of this commit)* @@ -891,33 +894,23 @@ Sub-stages used in commits / TODOs: ### Tracked follow-ups (carried past their sub-stage) Items deliberately deferred from earlier sub-stages so they don't get -lost. Pick up by name when the phase that consumes them lands. +lost. Pick up by name when the trigger that gates them fires. - **Ephemeral overlay mutations.** *(4e follow-up.)* All `route.set` / `setting.set` changes are persisted to `overlay.toml`. A `--ephemeral` flag (or `--volatile`) on the CLI for one-shot tweaks that don't outlive the daemon was considered and dropped from v0 for simplicity. Revisit if real users ask for it; the store-level - change is a flag on the setter methods. -- **Filter playback BUSY spikes (periodic, ~10 s cadence).** *(6c - manual smoke finding, 2026-05.)* On a quiet system with AGC and - per-app both off, the filter's `playback_process` BUSY - occasionally spikes from its ~240 μs steady-state to ~2.0 ms, - correlating with output-sink WAIT spikes of similar size. No - audible impact (sub-quantum at 21 ms). The ~10 s cadence rules - out sliding-max worst-case (which would be input-pattern-driven, - not periodic) and Layer A (the spikes persist with `per_app.enabled - = false`). Suspects with 10 s clocks somewhere: WirePlumber session - policy heartbeat, PipeWire internal graph re-eval, or system-level - scheduling (CPU governor, kernel housekeeping). Diagnostic for - Phase 8: timestamp the playback callback, log when its measured - duration crosses ~1 ms; correlate with `journalctl`, - `wireplumber --verbose`, and `pw-dump` snapshots taken around the - spike. If we can't attribute it to PipeWire-side reschedule and - it's something we can fix in our callback, the candidate - workaround is to break the limiter's per-block work into smaller - chunks (cap allocations / pops / branches per call) for more - predictable timing. + change is a flag on the setter methods. **Dormant** — no user has + asked through Phase 8. +- ~~**Filter playback BUSY spikes (periodic, ~10 s cadence).**~~ + **Closed in 8e (`d52cd6d`).** The instrumentation added by 8e + did not reproduce the ~8×-baseline outlier pattern in a ~3 min + release-build capture; steady state was ~2.2 ms / call at this + hardware's quantum with max growing only to 1.3× baseline. + `PlaybackTiming` stays so future regressions surface at WARN. + Original observation may have been a transient WP/PW housekeeping + artefact under a different config; no actionable code change. - **Sub-millisecond dispatch primitive for spike-reactive writes.** *(Phase 6 optimisation, downgraded from prerequisite.)* The 4i `PwCommand` channel uses a 50 ms polling timer, fine for @@ -1042,18 +1035,68 @@ If those three say "fine," the §4.1 promise is upheld in practice and 6c is acceptance-tested. `jack_iodelay` and other true-round-trip tools are overkill. -**Phase 7 — Packaging.** systemd user unit, install paths, default -profile install, basic NixOS module. +**Phase 7 — Packaging.** *Done — `c65c75b`.* `contrib/systemd/headroom.service` +(user-scope, Type=simple, After=pipewire.service, Restart=on-failure, +journald, LimitRTPRIO=20). The package's `postInstall` substitutes +the unit's `@bindir@` placeholder with an absolute store path and +copies `profiles/*.toml` to `share/headroom/profiles/`. Two Nix +modules: `nixosModules.default` (`programs.headroom.enable` — +binary on global PATH + `systemd.packages` for `systemctl --user` +discovery + hard assertion on `services.pipewire.enable`) and +`homeModules.default` (`services.headroom.enable` — symlinks +shipped profiles into `$XDG_CONFIG_HOME/headroom/profiles/`, +`extraProfiles` attrset for per-user overrides, writes the systemd +user unit). README rewritten with install + usage sections. -**Phase 8 — Hardening.** Latency budget verification on real hardware, -Bluetooth-handoff edge case, profile-reload while audio is flowing, -multi-rate hardware, allocation-tracer sweep with -`assert_no_alloc` in debug. +**Phase 8 — Hardening.** *Done — `9220143` + `d52cd6d` + verification.* +- **8a — `assert_no_alloc` on audio-thread callbacks (`9220143`).** + `#[global_allocator] AllocDisabler` in `headroom-cli/src/main.rs` + behind `cfg(debug_assertions)` (release strips it via the crate's + default `disable_release`). The three RT callbacks + (`capture_process`, `playback_process`, `tap_process`) wrap their + body in `assert_no_alloc(|| inner(...))`. Verified by a deliberate + `Vec::with_capacity` injection → SIGABRT on first audio callback; + reverted before commit. Audio thread proven alloc-free under + multi-thousand-callback live load. +- **8b — live profile-reload under signal flow (verification only).** + Edit `$XDG_CONFIG_HOME/headroom/profiles/.toml` while a + sine plays: notify-debouncer-mini fires, `ProfileStore::reload` + runs, `setting.set` propagates via `FilterControl`'s rtrb to the + audio thread. Compressor GR went 0 → −9.3 dB ≈ 1 s after edit + and back to 0 after restore; 180 meter ticks over 9 s with max + inter-tick gap = exact 50.0 ms (the AGC period). No glitches. +- **8c — sink hotplug / default-sink change (verification only).** + `wpctl set-default ` while daemon runs: + `on_metadata_property` fires, `adopt_new_real_sink` runs, + filter.playback re-pinned via 4k explicit-link enforcement, + `routing/real_sink_changed` emitted on the wire. Bounces back + cleanly. +- **8d — multi-rate hardware (partial / deferred).** Filter is + hardcoded F32 stereo @ 48 kHz; PipeWire's link layer inserts a + resampler at the filter.playback → real-sink edge when rates + differ; bus DSP stays at 48 kHz internally. Architecture is + sound; real-hardware validation (USB DAC at 96k etc.) deferred + until available. +- **8e — playback callback timing instrumentation (`d52cd6d`).** + Lock-free `PlaybackTiming` atomics in `meters.rs`; AGC controller + drains once per second and logs at WARN above + `SPIKE_THRESHOLD_US = 5000`. The original ~10 s-cadence ~8× + spike pattern from §11 follow-ups *did not reproduce* in a ~3 min + release-build capture; steady state 2.2 ms / call at ~4 Hz, + max climbed to only 1.3× baseline. Instrumentation kept so + future regressions surface. --- ## 12. Risks & open questions +These are the original v0 design risks — still useful as a checklist +for new contributors. Phase 4k/4l/8c have exercised the routing / +hotplug / default-sink branches; the bullets below are unchanged +since several of them remain live concerns for non-NixOS distros +and multi-rate hardware. See [[headroom-project]] in team memory +for current status per risk. + - **WirePlumber re-linking on device hotplug.** When a Bluetooth headset connects, WP re-evaluates linking. Headroom must re-pin its routed streams. Tractable; the registry events surface this. From 03edb171801fbd66a50fe39f934a13cae0494d06 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 18:19:32 +1000 Subject: [PATCH 09/21] F6: honour compressor.enabled in the DSP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The profile schema accepted `[compressor] enabled = false` (and the `transparent` and `bypass-all` profiles set it) but the flag was parsed and dropped — `build_compressor_config()` never threaded it through to `CompressorConfig`, and `Compressor::process_frame` had no enable branch. Result: the "compressor and AGC bypassed" claim in `transparent.toml`'s description was a lie; the compressor ran on every sample regardless of the profile knob. Surfaced by Codex's review of the project. Changes - `headroom_dsp::CompressorConfig` gains `pub enabled: bool` (default true). `Compressor::process_frame` early-returns `(left, right)` and resets `last_gr_db = 0.0` when disabled, so bus meters / `gain_reduction_db()` report the truthful "compressor off" state instead of the stale last value. - `headroom_core::profile::Profile::build_compressor_config` threads `self.compressor.enabled` into the materialised `CompressorConfig`. Live profile reload picks this up automatically — the next `set_config` push from `setting.set` / `profile.use` flips the audio thread. - Regression unit test `disabled_compressor_passes_signal_through_unchanged`: drive a -6 dBFS sine that would compress hard with enabled + aggressive thresholds, assert output equals input exactly and GR is zero. What this does NOT change - **Limiter has no `enabled` flag** and intentionally remains always-on. It is the daemon's hard contract (the -0.1 dBTP ceiling on the processed route, advertised in the README and in PLAN §3). Users who don't want limiting should route bypass; the `bypass-all.toml` profile's own comment confirms the limiter is "still configured as a fail-safe in case a stream lands on the processed sink anyway." Verified 186 tests pass (+1 for the disable path); clippy clean at -D warnings --all-targets. Live A/B against `pw-cat /tmp/sine` (-6 dBFS sine into processed): default profile compresses at -4.5 dB GR; `headroom profile use transparent` flips to 0.00 dB GR exactly on the next meter tick. --- crates/headroom-core/src/profile.rs | 1 + crates/headroom-dsp/src/compressor.rs | 39 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/crates/headroom-core/src/profile.rs b/crates/headroom-core/src/profile.rs index d20a00f..c065860 100644 --- a/crates/headroom-core/src/profile.rs +++ b/crates/headroom-core/src/profile.rs @@ -150,6 +150,7 @@ impl Profile { MakeupGain::Db(v) => Some(v), }; CompressorConfig { + enabled: self.compressor.enabled, threshold_db: self.compressor.threshold_db, ratio: self.compressor.ratio, knee_db: self.compressor.knee_db, diff --git a/crates/headroom-dsp/src/compressor.rs b/crates/headroom-dsp/src/compressor.rs index db2a462..a38f08d 100644 --- a/crates/headroom-dsp/src/compressor.rs +++ b/crates/headroom-dsp/src/compressor.rs @@ -19,6 +19,14 @@ pub enum Detector { /// Compressor parameters. #[derive(Debug, Clone, Copy, PartialEq)] pub struct CompressorConfig { + /// Master enable. When `false`, [`Compressor::process_frame`] + /// returns the input unchanged and reports zero gain reduction. + /// The compressor's envelope state is *not* reset while disabled, + /// so a stale envelope can briefly affect the first few samples + /// after re-enabling — but with typical release time constants + /// (tens to hundreds of ms) any residual transient is below the + /// audibility threshold. + pub enabled: bool, /// Threshold in dBFS. Inputs above this start compressing. pub threshold_db: f32, /// Compression ratio (>= 1.0). @@ -40,6 +48,7 @@ pub struct CompressorConfig { impl Default for CompressorConfig { fn default() -> Self { Self { + enabled: true, threshold_db: -24.0, ratio: 2.5, knee_db: 6.0, @@ -120,6 +129,13 @@ impl Compressor { /// Process one stereo frame. pub fn process_frame(&mut self, left: f32, right: f32) -> (f32, f32) { + if !self.cfg.enabled { + // Pass through untouched and report no reduction, so the + // bus meters reflect "compressor off" rather than the + // last value before disable. + self.last_gr_db = 0.0; + return (left, right); + } let det_lin = match self.cfg.detector { Detector::Peak => left.abs().max(right.abs()), Detector::Rms => { @@ -256,6 +272,29 @@ mod tests { assert_eq!(cfg.ratio, 1.0); } + #[test] + fn disabled_compressor_passes_signal_through_unchanged() { + // Same hot input that would compress hard in the enabled + // test above. With `enabled: false`, output equals input + // exactly (no makeup gain, no reduction), and the reporter + // shows zero GR — so the `transparent` and `bypass-all` + // profiles actually do what their name claims. + let cfg = CompressorConfig { + enabled: false, + threshold_db: -20.0, + ratio: 4.0, + makeup_db: Some(12.0), + ..CompressorConfig::default() + }; + let mut c = Compressor::new(cfg, 48_000.0); + for _ in 0..1_000 { + let (l, r) = c.process_frame(0.5, 0.5); + assert_eq!(l, 0.5); + assert_eq!(r, 0.5); + } + assert_eq!(c.gain_reduction_db(), 0.0); + } + #[test] fn static_curve_at_threshold_with_soft_knee() { // At exactly threshold, soft knee contributes exactly half the From 3427ec56fcdb5576df2f676c5541892fc626fa5b Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 18:24:01 +1000 Subject: [PATCH 10/21] F3: force-bypass surround streams; generalise N-channel pairing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex flagged that routing was channel-blind and the explicit-link pairer hardcoded `take(2)`. For a 5.1 stream the consequences depended on the route decision: Route::Processed silently dropped the centre, LFE, and both surround channels (only FL/FR linked to the stereo processed sink); Route::Bypass to a 5.1-capable real sink had the destruction pass kill 4 of 6 links because they weren't in the 2-pair `want_set`. Either way the user lost channels. PLAN §12 already documented the intent ("anything >2ch is routed directly to the real sink, bypass behaviour, regardless of profile rule") but the code didn't honour it. This commit makes the contract load-bearing. Changes - `PwNodeInfo` gains `audio_channels: Option`, populated in `build_node_info` from the stream's `audio.channels` property. `None` for clients that don't advertise (older PW, odd toolkits) — those fall through to normal rule evaluation on the assumption they're stereo or mono. - `routing::evaluate` short-circuits to `Route(Bypass)` when `audio_channels > 2`, ahead of rule matching. The bus filter is F32 stereo by construction, so this is the only honest answer: forcing surround into the processed path either drops channels or invents an unrequested downmix. - `apply_pending_routes`' link pairing generalised from `take(2)` to `take(min(src_outs.len(), target_ins.len()))`. Stereo → stereo is unchanged (`min(2, 2) = 2`); 5.1 → 5.1 real sink now pairs all six channels; 5.1 → stereo real sink pairs two (PipeWire's source-side adapter does the downmix, which is its job, not ours). The destruction pass already only nukes links to *known sinks*, so taps + non-sink consumers stay untouched as before. - PLAN §12 updated: the surround bullet now describes enforced behaviour rather than aspirational documentation. Tests - `routing::tests::surround_streams_force_bypass_regardless_of_rule_match` — a 6-channel stream matching the default profile's "browser is processed" rule must still bypass. - `routing::tests::stereo_and_mono_streams_follow_normal_rules` — confirms the forcer only triggers for `>2ch` (None, Some(1), Some(2) all flow through to the rule). 188 tests pass; clippy clean at -D warnings --all-targets. Live regression check (stereo 1 kHz sine into processed): 51 non-floor meter ticks over 3 s, bus DSP path still flowing, integrated LUFS around -28. Stereo path unaffected by the generalised pairing. --- PLAN.md | 13 ++++++- crates/headroom-core/src/pw/registry.rs | 20 ++++++++-- crates/headroom-core/src/routing.rs | 51 +++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/PLAN.md b/PLAN.md index c2b5374..4cc64a1 100644 --- a/PLAN.md +++ b/PLAN.md @@ -1114,8 +1114,17 @@ for current status per risk. filter should source its rate from the real sink and convert on the capture side only. - **Surround content downmix vs. passthrough.** v0 punts: anything - >2ch is routed directly to the real sink (bypass behaviour) - regardless of profile rule. Documented behaviour. + `>2ch` is force-bypassed regardless of profile rule. The bus + filter is F32 stereo by construction and pulling a 5.1+ stream + into it would either drop the centre/LFE/surround channels (with + explicit links pairing only the first two ports) or run our DSP + on a downmix that wasn't asked for. The check fires in + `routing::evaluate` based on `PwNodeInfo.audio_channels` (parsed + from the stream's `audio.channels` property). The explicit-link + pairing in `apply_pending_routes` was generalised from `take(2)` + to `take(min(src, dst))` so wide bypass to a wide real sink links + all channels; narrower sinks let PipeWire's source-side adapter + handle downmix as usual. --- diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index 13c0790..6a0bdd4 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -969,8 +969,19 @@ impl RoutingState { tracing::debug!(node_id, target_node, "pending route: target has no input ports yet"); continue; }; - // Stereo v0 — pair by ordinal. - if src_outs.len() < 2 || target_ins.len() < 2 { + // Pair by ordinal up to whichever side has fewer + // channels. For stereo→stereo this is the original + // `take(2)`. For wider streams (surround) routed + // Bypass to a wide sink we pair all N channels — fixes + // the F3 bug where the old `take(2)` silently dropped + // the centre, LFE, and surround channels of a 5.1 + // stream. If the wider side has more channels than the + // narrower side, the extra ports are left unlinked + // here; the destruction pass below also leaves them + // alone (they don't land on the target sink at all). + // PipeWire's adapter is responsible for any downmix. + let pair_count = src_outs.len().min(target_ins.len()); + if pair_count < 2 { tracing::debug!( node_id, src_outs = src_outs.len(), @@ -981,8 +992,8 @@ impl RoutingState { } let want: Vec<(u32, u32)> = src_outs .iter() - .take(2) - .zip(target_ins.iter().take(2)) + .take(pair_count) + .zip(target_ins.iter().take(pair_count)) .map(|(o, i)| (o.port_id, i.port_id)) .collect(); let want_set: std::collections::HashSet<(u32, u32)> = want.iter().copied().collect(); @@ -1515,6 +1526,7 @@ fn build_node_info(node_id: u32, dict: &DictRef) -> PwNodeInfo { .map(str::to_owned), media_role: dict.get("media.role").map(str::to_owned), dont_move: dict.get("node.dont-move") == Some("true"), + audio_channels: dict.get("audio.channels").and_then(|s| s.parse::().ok()), } } diff --git a/crates/headroom-core/src/routing.rs b/crates/headroom-core/src/routing.rs index 7f412b9..a2c194b 100644 --- a/crates/headroom-core/src/routing.rs +++ b/crates/headroom-core/src/routing.rs @@ -34,6 +34,15 @@ pub struct PwNodeInfo { /// `node.dont-move` — if set true, the stream opted out of being /// rerouted. Honoured by skipping routing entirely. pub dont_move: bool, + /// `audio.channels` — the stream's declared channel count. + /// `None` if the property is absent (older PipeWire / odd + /// clients). Used to force `>2ch` streams onto the bypass + /// path: the bus filter is stereo-only by construction, so + /// pulling a 5.1 stream into the processed sink would either + /// drop four channels (if we used explicit links naively) or + /// produce a downmix that wasn't asked for. Either way the + /// safer default is "leave surround alone." + pub audio_channels: Option, } impl PwNodeInfo { @@ -64,6 +73,17 @@ pub fn evaluate(info: &PwNodeInfo, profile: &Profile) -> RoutingDecision { if !info.is_routable_playback() { return RoutingDecision::Skip; } + // Force-bypass anything wider than stereo. PLAN §3's surround + // contract: the bus filter is F32 stereo by construction, so + // pulling a 5.1+ stream into `headroom-processed` either drops + // channels (with explicit links) or produces an unrequested + // downmix (if WP's adapter gets involved). Routing it straight + // to the real sink preserves the user's intended layout. If + // the real sink isn't 5.1-capable PipeWire's source-side + // adapter handles the downmix — that's its job, not ours. + if matches!(info.audio_channels, Some(ch) if ch > 2) { + return RoutingDecision::Route(Route::Bypass); + } for rule in &profile.rules { if matches(info, &rule.match_) { return RoutingDecision::Route(rule.route); @@ -124,6 +144,37 @@ mod tests { assert_eq!(evaluate(&info, &profile), RoutingDecision::Skip); } + #[test] + fn surround_streams_force_bypass_regardless_of_rule_match() { + // The default profile routes `firefox` to processed. A 5.1 + // firefox stream (rare but valid — some browser content + // declares surround) must still bypass: the bus filter is + // stereo-only and the explicit-link path would otherwise + // drop FC/LFE/SL/SR. PLAN §3 surround contract. + let mut info = playback("firefox"); + info.audio_channels = Some(6); + let profile = Profile::default_v0(); + assert_eq!( + evaluate(&info, &profile), + RoutingDecision::Route(Route::Bypass) + ); + } + + #[test] + fn stereo_and_mono_streams_follow_normal_rules() { + // Sanity: the surround forcer only kicks in for >2ch. + let profile = Profile::default_v0(); + for ch in [None, Some(1), Some(2)] { + let mut info = playback("firefox"); + info.audio_channels = ch; + assert_eq!( + evaluate(&info, &profile), + RoutingDecision::Route(Route::Processed), + "channels={ch:?}" + ); + } + } + #[test] fn matches_bypass_rule_for_known_music_player() { let info = playback("mpv"); From e0c23ec459692c04b15043f9f778c6094b08daff Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 18:32:43 +1000 Subject: [PATCH 11/21] F1: make `bypass on` a real kill switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex flagged that `bypass.set` only flipped `bypass_global` in profile state and never touched the graph: `try_route_stream` returned Skip but the daemon kept re-asserting `default.audio.sink = headroom-processed`, so apps following default still landed in the processor, and already-managed streams kept their explicit links to the processed sink. The "kill switch" killed nothing. What the bypass now actually does Three coupled effects, applied atomically by a single `PwCommand::ReevaluateAll` post from the IPC handler: 1. **Routing decision flips.** `routing::evaluate` learned to short-circuit to `Route(Bypass)` for every routable playback stream when `bypass_global=true`. Surround's pre-existing `>2ch -> Bypass` rule still applies; both share the same output and pick up the same explicit-link machinery from 4k. 2. **Existing managed streams get re-routed.** A new `known_streams: HashMap` cache in `RoutingState` (populated on `try_route_stream`, cleared in `on_global_remove`) lets `reevaluate_all` iterate every stream we've ever seen and re-run the decision. The extracted `apply_bus_route` runs the same enqueue / unmanage logic the registry callback uses, so the live-arrival path and the bypass-toggle path stay in lockstep. 3. **`default.audio.sink` flips to the real sink.** Inside `reevaluate_all`, the daemon writes default to the real sink name under bypass, and back to `headroom-processed` when bypass clears. The `reassert_default_processed` rate-limiter is gated on bypass so we don't keep fighting WP for a sink we no longer want as default. Apps that route to "default" (which is most legacy code paths and a lot of GTK/Qt widgets) now actually skip the processor under bypass. Adjacent cleanups that fell out - `try_route_stream` no longer carries the bypass branch inline. The split — registry callback inserts cache + calls `apply_bus_route` + maybe spawns Layer A — keeps the re-evaluation path free of the `&GlobalObject` it doesn't have. Layer A spawning stays at first-see time as before; streams that arrived before the daemon doesn't get a retroactive tap, which is fine since Layer A is orthogonal to bus routing and tap creation requires the registry global. - `RoutingDecision::Skip` now properly tears down any prior bus state (`unmanage()` drops the Link proxies and removes the IPC-visible `state.streams` entry). - `PwCommand::ReevaluateAll` is a generic re-evaluation trigger; F2 will reuse it for profile / rule changes. Tests - `routing::evaluate` signature picked up a `bypass_global: bool` arg; 11 unit tests updated to pass `false`. - ops::tests' `let PwCommand::RouteStream { .. } = cmd;` is now `let ... else { panic!(..) }` (the enum is no longer single-variant). 188 tests pass; clippy clean. Live verification A/B/A against a 1 kHz sine `--target headroom-processed`: - bypass off (baseline): pw-cat → headroom-processed:playback; default.audio.sink = headroom-processed. - bypass on: pw-cat → Mbox:playback (the explicit link to processed is gone, a new explicit link to the real sink is in place); default.audio.sink = the Mbox. - bypass off (back): pw-cat → headroom-processed:playback; default.audio.sink = headroom-processed. - Layer A tap link stays attached through both transitions — orthogonal as designed. --- crates/headroom-core/src/ipc/ops.rs | 20 +++- crates/headroom-core/src/pw/command.rs | 8 ++ crates/headroom-core/src/pw/registry.rs | 149 ++++++++++++++++++++---- crates/headroom-core/src/routing.rs | 40 ++++--- 4 files changed, 179 insertions(+), 38 deletions(-) diff --git a/crates/headroom-core/src/ipc/ops.rs b/crates/headroom-core/src/ipc/ops.rs index ec1c222..1230e43 100644 --- a/crates/headroom-core/src/ipc/ops.rs +++ b/crates/headroom-core/src/ipc/ops.rs @@ -356,7 +356,22 @@ fn bypass_set(id: u64, enabled: bool, state: &SharedState) -> Response { match s.profiles.set_bypass(enabled) { Ok(()) => { tracing::info!(enabled, "bypass.set applied"); + let tx = s.pw_command_tx.clone(); drop(s); + // Make bypass an actual graph operation, not just a + // metadata flag. The registry thread re-runs + // `routing::evaluate` against every known stream (which + // now returns Route::Bypass under bypass_global=true), + // tears down the explicit links to the processed sink, + // and rebuilds them to the real sink. The + // `reassert_default_processed` path is also gated on + // bypass, so WP's choice of system default sticks for + // any apps that route to "default." + if let Some(tx) = tx { + if tx.send(PwCommand::ReevaluateAll).is_err() { + tracing::warn!("PipeWire command channel closed; bypass toggle had no graph effect"); + } + } ok(id, &Value::Null) } Err(e) => store_err_to_response(id, e), @@ -1049,7 +1064,10 @@ mod tests { node_id, to, app_label, - } = cmd; + } = cmd + else { + panic!("expected RouteStream, got {cmd:?}"); + }; assert_eq!(node_id, 42); assert_eq!(to, Route::Bypass); assert_eq!(app_label, "firefox"); diff --git a/crates/headroom-core/src/pw/command.rs b/crates/headroom-core/src/pw/command.rs index 6273639..93efde4 100644 --- a/crates/headroom-core/src/pw/command.rs +++ b/crates/headroom-core/src/pw/command.rs @@ -53,4 +53,12 @@ pub enum PwCommand { /// Cached app label for log lines / events. app_label: String, }, + /// Re-run `routing::evaluate` against every known stream and + /// enqueue routes where the decision changed since last time. + /// Posted by IPC handlers that mutate routing inputs — global + /// bypass toggle (F1), profile.use / profile.reload / route.set + /// / route.unset (F2). The handler reads current state (bypass, + /// effective profile, real sink) at apply time, not at post + /// time, so a stale command is harmless. + ReevaluateAll, } diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index 6a0bdd4..ecc19e2 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -199,6 +199,13 @@ pub struct RoutingState { /// `(window_start, attempts_in_window)`. See /// [`Self::reassert_default_processed`]. default_reassertion: Option<(std::time::Instant, u32)>, + /// Cache of `PwNodeInfo` for every routable playback stream + /// we've seen, keyed by node id. Lets the bypass toggle (F1) + /// and the profile/rule reapply path (F2) re-run + /// `routing::evaluate` without re-fetching properties from + /// PipeWire. Populated by `try_route_stream`, cleared in + /// `on_global_remove`. + known_streams: HashMap, } /// Per-stream Layer A bundle: the tap (audio path), the controller @@ -265,6 +272,7 @@ impl RoutingState { pending_routes: HashMap::new(), managed_route_links: HashMap::new(), default_reassertion: None, + known_streams: HashMap::new(), } } @@ -307,6 +315,9 @@ impl RoutingState { self.write_stream_target(node_id, &target_name, &app_label); self.enqueue_route(node_id, target_name, app_label, to); } + PwCommand::ReevaluateAll => { + self.reevaluate_all(); + } } } @@ -632,37 +643,55 @@ impl RoutingState { let info = build_node_info(global.id, dict); - // Hold the lock only long enough to clone what we need; never - // call out to PipeWire while locked. - let (decision, app_label, real_sink_name) = { + // Cache before routing so the bypass-toggle / profile-reapply + // paths can re-run `evaluate` on this stream later without + // re-reading PipeWire properties. The cache survives every + // routing decision (including Skip) and is cleaned up by + // `on_global_remove`. + self.known_streams.insert(info.node_id, info.clone()); + + let app_label = info_app_label(&info); + self.apply_bus_route(&info, &app_label); + + // Bus routing decision is in place; Layer A is orthogonal — + // it taps the source's output regardless of where the bus + // routes. Spawning here (not in `apply_bus_route`) keeps + // the re-evaluation path free of the `&GlobalObject` it + // doesn't have. + self.maybe_spawn_layer_a(global, &info, &app_label, back); + } + + /// Apply the current bus-routing decision for `info`. Reads + /// global bypass + active profile + real-sink name at call time + /// (so a stale snapshot can't bite), then either enqueues an + /// explicit-link route or unmanages the stream. No PipeWire + /// proxies touched while the daemon lock is held. + fn apply_bus_route(&mut self, info: &PwNodeInfo, app_label: &str) { + let (decision, real_sink_name) = { let s = self.daemon.lock(); - let label = info_app_label(&info); - if s.profiles.bypass_global() { - (RoutingDecision::Skip, label, s.real_sink.name.clone()) - } else { - let d = routing::evaluate(&info, s.profiles.effective()); - (d, label, s.real_sink.name.clone()) - } + let bypass = s.profiles.bypass_global(); + let d = routing::evaluate(info, s.profiles.effective(), bypass); + (d, s.real_sink.name.clone()) }; match decision { RoutingDecision::Route(Route::Processed) => { - self.write_stream_target(info.node_id, PROCESSED_SINK_NAME, &app_label); + self.write_stream_target(info.node_id, PROCESSED_SINK_NAME, app_label); self.enqueue_route( info.node_id, PROCESSED_SINK_NAME.to_owned(), - app_label.clone(), + app_label.to_owned(), Route::Processed, ); - self.record_route(info.node_id, app_label.clone(), Route::Processed); + self.record_route(info.node_id, app_label.to_owned(), Route::Processed); } RoutingDecision::Route(Route::Bypass) => { if let Some(name) = real_sink_name.as_deref() { - self.write_stream_target(info.node_id, name, &app_label); + self.write_stream_target(info.node_id, name, app_label); self.enqueue_route( info.node_id, name.to_owned(), - app_label.clone(), + app_label.to_owned(), Route::Bypass, ); } else { @@ -673,21 +702,84 @@ impl RoutingState { // there's nothing better we could target. tracing::warn!( node_id = info.node_id, - app = app_label.as_str(), + app = app_label, "bypass route with no known real sink — leaving stream at PipeWire default" ); } - self.record_route(info.node_id, app_label.clone(), Route::Bypass); + self.record_route(info.node_id, app_label.to_owned(), Route::Bypass); } RoutingDecision::Skip => { + // Stream isn't (or no longer is) a managed bus + // stream. Drop any explicit links + intent we'd + // built for it; leave Layer A alone. tracing::trace!(node_id = info.node_id, "skip (not routable)"); - return; + self.unmanage(info.node_id); + } + } + } + + /// Tear down bus-routing state for `node_id`: drops the explicit + /// `Link` proxies (which destroys the links via + /// `object.linger = "false"`), removes any pending route intent, + /// and removes the stream from the IPC-visible `state.streams`. + /// Layer A managed-stream entries are intentionally untouched — + /// they're keyed on the source node, not on the bus route, and + /// have their own lifecycle. + fn unmanage(&mut self, node_id: u32) { + self.pending_routes.remove(&node_id); + self.managed_route_links.remove(&node_id); + let mut s = self.daemon.lock(); + if s.streams.remove(&node_id).is_some() { + tracing::debug!(node_id, "bus route unmanaged"); + } + } + + /// Re-apply the current routing policy to every known stream. + /// Cheap: per-stream cost is one `routing::evaluate` call plus + /// (only when the decision changed) one `enqueue_route` or + /// `unmanage`. Called from `apply_pw_command` when the IPC layer + /// posts `PwCommand::ReevaluateAll` — global bypass toggle (F1), + /// profile.use / profile.reload / route.set / route.unset (F2). + /// + /// Also re-asserts `default.audio.sink` to the correct value for + /// the current bypass state: `headroom-processed` when off, the + /// real sink when on. This is what makes "bypass on" a real kill + /// switch — apps that don't speak `target.object` (or any new + /// stream that hasn't been routed yet) follow `default`, so + /// flipping it is the only way to redirect them. + fn reevaluate_all(&mut self) { + let (bypass, real_sink_name) = { + let s = self.daemon.lock(); + (s.profiles.bypass_global(), s.real_sink.name.clone()) + }; + match (bypass, real_sink_name.as_deref()) { + (true, Some(name)) => { + tracing::info!( + sink = name, + "bypass on: setting default.audio.sink to real sink" + ); + self.write_default_audio_sink(name); + } + (true, None) => { + tracing::warn!( + "bypass on but no real sink known yet — leaving default.audio.sink alone" + ); + } + (false, _) => { + // Use the unconditional write here rather than + // `reassert_default_processed`'s rate-limited path: + // we're handling an explicit operator action (bypass + // off, profile change, etc.), not a fight with WP. + self.write_default_audio_sink(PROCESSED_SINK_NAME); } } - // Bus routing decision is in place; Layer A is orthogonal. If - // the stream matches a `per_app` rule, spawn the analysis tap. - self.maybe_spawn_layer_a(global, &info, &app_label, back); + let snapshot: Vec = self.known_streams.values().cloned().collect(); + tracing::info!(streams = snapshot.len(), "reevaluating all known streams"); + for info in snapshot { + let app_label = info_app_label(&info); + self.apply_bus_route(&info, &app_label); + } } /// Spawn a Layer A tap + controller if the stream matches an @@ -1153,6 +1245,16 @@ impl RoutingState { /// 4k links continue to enforce routing for managed streams /// regardless of which side wins the default. fn reassert_default_processed(&mut self) { + // Under global bypass we deliberately stop fighting WP over + // the system default — letting it pick the user's + // configured sink means apps that don't speak target.object + // (and any new streams that arrive while bypassed) land at + // the real sink rather than at headroom-processed. Without + // this gate, "headroom bypass on" wouldn't actually bypass + // for those apps. + if self.daemon.lock().profiles.bypass_global() { + return; + } const WINDOW: std::time::Duration = std::time::Duration::from_secs(1); const MAX_PER_WINDOW: u32 = 10; let now = std::time::Instant::now(); @@ -1310,9 +1412,12 @@ impl RoutingState { self.links_by_id .retain(|_, info| info.output_node != node_id && info.input_node != node_id); - // Stream gone — drop pending intent + managed Link proxies. + // Stream gone — drop pending intent + managed Link proxies + // + the routing cache entry so re-evaluation passes don't + // try to apply a route to a node that no longer exists. self.pending_routes.remove(&node_id); self.managed_route_links.remove(&node_id); + self.known_streams.remove(&node_id); if self.filter_playback_id == Some(node_id) { tracing::debug!(node_id, "filter playback removed from registry"); diff --git a/crates/headroom-core/src/routing.rs b/crates/headroom-core/src/routing.rs index a2c194b..a5956aa 100644 --- a/crates/headroom-core/src/routing.rs +++ b/crates/headroom-core/src/routing.rs @@ -66,13 +66,23 @@ pub enum RoutingDecision { /// Evaluate a stream against the profile's routing rules. /// /// Returns [`RoutingDecision::Skip`] if the stream isn't a routable -/// playback stream. Otherwise returns the first-match route, or the -/// profile's `default_route` if no rule matches. +/// playback stream. When `bypass_global` is true, every routable +/// stream gets [`Route::Bypass`] regardless of rule match — the +/// global kill switch overrides everything. Otherwise returns the +/// first-match route, or the profile's `default_route` if no rule +/// matches. #[must_use] -pub fn evaluate(info: &PwNodeInfo, profile: &Profile) -> RoutingDecision { +pub fn evaluate(info: &PwNodeInfo, profile: &Profile, bypass_global: bool) -> RoutingDecision { if !info.is_routable_playback() { return RoutingDecision::Skip; } + // Global bypass: nothing reaches the processed sink. Implemented + // as a real graph operation (4k explicit links to the real sink) + // rather than just a metadata write — see PwCommand::ReevaluateAll + // and `set_global_bypass` in the registry. + if bypass_global { + return RoutingDecision::Route(Route::Bypass); + } // Force-bypass anything wider than stereo. PLAN §3's surround // contract: the bus filter is F32 stereo by construction, so // pulling a 5.1+ stream into `headroom-processed` either drops @@ -133,7 +143,7 @@ mod tests { let mut info = playback("firefox"); info.media_class = Some("Stream/Input/Audio".into()); let profile = Profile::default_v0(); - assert_eq!(evaluate(&info, &profile), RoutingDecision::Skip); + assert_eq!(evaluate(&info, &profile, false), RoutingDecision::Skip); } #[test] @@ -141,7 +151,7 @@ mod tests { let mut info = playback("firefox"); info.dont_move = true; let profile = Profile::default_v0(); - assert_eq!(evaluate(&info, &profile), RoutingDecision::Skip); + assert_eq!(evaluate(&info, &profile, false), RoutingDecision::Skip); } #[test] @@ -155,7 +165,7 @@ mod tests { info.audio_channels = Some(6); let profile = Profile::default_v0(); assert_eq!( - evaluate(&info, &profile), + evaluate(&info, &profile, false), RoutingDecision::Route(Route::Bypass) ); } @@ -168,7 +178,7 @@ mod tests { let mut info = playback("firefox"); info.audio_channels = ch; assert_eq!( - evaluate(&info, &profile), + evaluate(&info, &profile, false), RoutingDecision::Route(Route::Processed), "channels={ch:?}" ); @@ -180,7 +190,7 @@ mod tests { let info = playback("mpv"); let profile = Profile::default_v0(); assert_eq!( - evaluate(&info, &profile), + evaluate(&info, &profile, false), RoutingDecision::Route(Route::Bypass) ); } @@ -190,7 +200,7 @@ mod tests { let info = playback("firefox"); let profile = Profile::default_v0(); assert_eq!( - evaluate(&info, &profile), + evaluate(&info, &profile, false), RoutingDecision::Route(Route::Processed) ); } @@ -201,7 +211,7 @@ mod tests { let profile = Profile::default_v0(); // default_v0 has `default_route = Processed`. assert_eq!( - evaluate(&info, &profile), + evaluate(&info, &profile, false), RoutingDecision::Route(Route::Processed) ); } @@ -228,7 +238,7 @@ mod tests { }); let info = playback("firefox"); assert_eq!( - evaluate(&info, &profile), + evaluate(&info, &profile, false), RoutingDecision::Route(Route::Bypass) ); } @@ -243,7 +253,7 @@ mod tests { }); let info = playback("firefox"); assert_eq!( - evaluate(&info, &profile), + evaluate(&info, &profile, false), RoutingDecision::Route(Route::Bypass) ); } @@ -264,7 +274,7 @@ mod tests { // process_binary matches but media_role doesn't (None on info). let info = playback("firefox"); assert_ne!( - evaluate(&info, &profile), + evaluate(&info, &profile, false), RoutingDecision::Route(Route::Bypass) ); @@ -272,7 +282,7 @@ mod tests { let mut info2 = playback("firefox"); info2.media_role = Some("Communication".into()); assert_eq!( - evaluate(&info2, &profile), + evaluate(&info2, &profile, false), RoutingDecision::Route(Route::Bypass) ); } @@ -291,7 +301,7 @@ mod tests { let mut info = playback("DiscordWrapper"); info.portal_app_id = Some("com.discordapp.Discord".into()); assert_eq!( - evaluate(&info, &profile), + evaluate(&info, &profile, false), RoutingDecision::Route(Route::Processed) ); } From 0e718abe274d49856200124f3f81995176230554 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 18:40:02 +1000 Subject: [PATCH 12/21] F2: reapply routing on profile / rule changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex flagged that `profile use`, `profile reload`, `route set`, and `route unset` updated overlay state and (sometimes) propagated DSP configs but never asked the registry thread to re-route existing streams. The new policy only applied to *future* connections; anything already routed kept its old explicit links until the app disconnected. The plumbing was actually already in place from F1 — the bypass toggle posted `PwCommand::ReevaluateAll`, the registry handled it, and `reevaluate_all` iterated the `known_streams` cache. This commit is just the missing call sites: a `post_reevaluate(state)` helper that reads `state.pw_command_tx` and sends `ReevaluateAll`, called after each of the four mutating IPC ops. `execute_reload` (which the profile-watcher also calls) gets the post too, so editing a TOML on disk now re-routes live streams. Tests All 188 still pass; clippy clean. Live verification Sine flowing through `headroom-processed` while the daemon is on the `layer-a-test` profile (default_route = processed): - `headroom profile use bypass-all` → pw-cat's explicit link flips from processed → Mbox within ~50 ms (one drain tick). - `headroom profile use layer-a-test` → flips back to processed. - Layer A tap link survives both transitions (orthogonal, unaffected by bus rerouting — same invariant as F1). Adjacent issue noted (not in F2 scope) `headroom route set ` only writes the rule's `process_binary` field. Streams that don't advertise `application.process.binary` (pw-cat is one) can't be matched by this single-field rule even though they have an `application.name`. The fix is either to widen `route.set` into a smarter "match by app label" verb (which would either need a new OR-across-fields matcher kind or a CLI flag to pick which field) or to teach the materialiser to produce both process_binary AND application_name rules with the same name, with the matcher then OR'd. Either way it's a separate UX bug; filed as a follow-up. --- crates/headroom-core/src/ipc/ops.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/headroom-core/src/ipc/ops.rs b/crates/headroom-core/src/ipc/ops.rs index 1230e43..3b480b0 100644 --- a/crates/headroom-core/src/ipc/ops.rs +++ b/crates/headroom-core/src/ipc/ops.rs @@ -193,6 +193,7 @@ fn profile_use(id: u64, name: &str, state: &SharedState) -> Response { publish_profile_changed(&mut s, name); let control = s.filter_control.clone(); let snap = build_dsp_configs(&s); + post_reevaluate(&s); drop(s); push_dsp_update(control.as_ref(), snap); ok(id, &json!({ "name": name })) @@ -234,6 +235,7 @@ pub(crate) fn execute_reload( publish_profile_reloaded(&mut s, &report.loaded); let control = s.filter_control.clone(); let snap = build_dsp_configs(&s); + post_reevaluate(&s); drop(s); push_dsp_update(control.as_ref(), snap); Ok(report) @@ -245,6 +247,7 @@ fn route_set(id: u64, app: &str, to: Route, state: &SharedState) -> Response { Ok(()) => { tracing::info!(app, ?to, "route.set applied"); publish_rule_changed(&mut s); + post_reevaluate(&s); drop(s); ok(id, &Value::Null) } @@ -258,6 +261,7 @@ fn route_unset(id: u64, app: &str, state: &SharedState) -> Response { Ok(()) => { tracing::info!(app, "route.unset applied"); publish_rule_changed(&mut s); + post_reevaluate(&s); drop(s); ok(id, &Value::Null) } @@ -271,6 +275,25 @@ fn publish_rule_changed(state: &mut crate::state::DaemonState) { } } +/// Ask the PipeWire main loop to re-run `routing::evaluate` against +/// every known stream. Called after any IPC mutation that changes +/// the inputs to that decision: active profile, profile contents +/// reloaded from disk, or a `route.set` / `route.unset` overlay +/// edit. Without this, the new policy only applies to *future* +/// streams; everything already routed keeps its old links until the +/// app reconnects. A stale or duplicate post is harmless — the +/// handler reads current state at apply time and is idempotent +/// when nothing changed. +fn post_reevaluate(state: &crate::state::DaemonState) { + let Some(tx) = state.pw_command_tx.as_ref() else { + tracing::debug!("no PipeWire command channel; reevaluation skipped (test mode)"); + return; + }; + if tx.send(PwCommand::ReevaluateAll).is_err() { + tracing::warn!("PipeWire command channel closed; reevaluation lost"); + } +} + fn publish_profile_changed(state: &mut crate::state::DaemonState, name: &str) { if let Ok(event) = Event::new(Topic::Profile, "used", &json!({ "name": name })) { state.broadcaster.publish(Topic::Profile, event); From 04a005e1cd820e3525616694e62dcf017adeb6b9 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 18:43:02 +1000 Subject: [PATCH 13/21] F4: real-sink discovery fallback closes the cold-boot race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex flagged a real-but-rare race in `try_bind_default_metadata`: the daemon installs the metadata listener then immediately writes `default.audio.sink = headroom-processed`, relying on PipeWire to deliver the prior value to the listener before our write. In practice this works (pw-metadata replays current state to a freshly-installed listener), but two failure modes leak through: 1. **Prior daemon left the world dirty.** If a previous daemon run set default to `headroom-processed` and didn't restore before exiting, the listener replays "headroom-processed" — `on_metadata_property` recognises that as our own promotion and returns early. `real_sink.name` is never captured. Bypass routes log "no real sink known" forever. 2. **No replay event.** If the listener doesn't fire for any reason — broken PipeWire, an event-bus hiccup, pipewire-rs's `add_listener_local` racing with our write — same outcome. Fix: instead of trying to win the listener race (pipewire-rs has no synchronous metadata getter, and `Core::sync` needs an async done-callback we don't have plumbing for), make `try_capture_real_sink` self-heal. When `real_sink.name` is still `None` and we see *any* non-processed `Audio/Sink` on the registry, adopt it as the fallback real sink. A subsequent `default.audio.sink` event will refine the choice via the existing `adopt_new_real_sink` path if WP picks something else. This is a belt-and-braces patch — the listener path stays the primary mechanism, the fallback only kicks in when that path hasn't produced a name. In steady-state (the common case where listener replay works) it changes nothing. Verified Cold start with PipeWire's `default.audio.sink` set to the Mbox: daemon logs `preferred_real_sink updated sink=Mbox` via the listener path; the fallback's `adopting first available Audio/Sink as fallback` log does not fire. No regression for the steady state. 188 tests pass; clippy clean at -D warnings --all-targets. --- crates/headroom-core/src/pw/registry.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index ecc19e2..32631e8 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -496,6 +496,17 @@ impl RoutingState { /// `sinks_by_name`. If the captured name matches the active /// `preferred_real_sink`, populate `real_sink.node_id` so `status` /// reports it and downstream ops can route by id. + /// + /// **Cold-boot fallback (F4).** If we haven't captured a real-sink + /// name yet — either because the metadata listener's initial + /// replay hasn't fired, or because `default.audio.sink` was + /// already `headroom-processed` from a prior daemon run that + /// didn't clean up — adopt the first non-processed Audio/Sink we + /// see as the real sink. This avoids the failure mode Codex + /// flagged where `real_sink.name = None` indefinitely and bypass + /// routes log "no real sink known" forever. A subsequent + /// `default.audio.sink` event will refine the choice via + /// `adopt_new_real_sink` if the user/WP picks a different one. fn try_capture_real_sink(&mut self, global: &GlobalObject<&DictRef>) { let Some(props) = &global.props else { return }; let dict: &DictRef = props; @@ -510,7 +521,15 @@ impl RoutingState { } self.sinks_by_name.insert(name.to_owned(), global.id); let mut s = self.daemon.lock(); - if s.real_sink.name.as_deref() == Some(name) && s.real_sink.node_id != Some(global.id) { + if s.real_sink.name.is_none() { + tracing::info!( + node_id = global.id, + name, + "no preferred_real_sink yet; adopting first available Audio/Sink as fallback" + ); + s.real_sink.name = Some(name.to_owned()); + s.real_sink.node_id = Some(global.id); + } else if s.real_sink.name.as_deref() == Some(name) && s.real_sink.node_id != Some(global.id) { tracing::info!( node_id = global.id, name, From 5143c07c9931ce29fa8734183c368c99658eaa65 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 18:43:58 +1000 Subject: [PATCH 14/21] =?UTF-8?q?F5:=20document=20the=20limiter's=20rate-l?= =?UTF-8?q?eakage=20caveat=20in=20PLAN=20=C2=A73.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex's review of the v0 design correctly pointed out that PLAN's "hard −0.1 dBTP ceiling" claim only holds at the filter's output, not at the speaker, when the real sink runs at a non-48 kHz native rate. PipeWire inserts a resampler at the `filter.playback → real-sink` edge, and any polynomial / windowed-sinc reconstruction can elevate inter-sample peaks slightly through its own math. The elevation is small in practice (a few tenths of a dB for a clean band-limited resampler) and the contract still holds where headroom is in control of the graph, but the README and §3 had been silent on the leak. This commit only edits docs: - §3.1 grows a "Contract scope (caveat)" paragraph next to the "Never bypassed, never disabled" hard-tier description, naming the leak, its magnitude, and the fix-for-real (filter rate matching). - §11 picks up a new tracked follow-up entry alongside the other dormant items. Scope: dynamic `FILTER_SAMPLE_RATE`, kernel rebuild on real-sink change, Layer A's block-period constant goes dynamic too. Gated on a multi-rate hardware test bench — no point shipping the refactor without something to validate it against. **v1 scope.** No code changes; no tests; clippy and tests are unaffected. --- PLAN.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/PLAN.md b/PLAN.md index 4cc64a1..4fedb21 100644 --- a/PLAN.md +++ b/PLAN.md @@ -233,6 +233,23 @@ downsampling) guarantee the contract numerically — the envelope can misbehave and the contract still holds. Never bypassed, never disabled. +**Contract scope (caveat).** The ≤ −0.1 dBTP guarantee holds at the +*filter's output*, not at the speaker. The bus filter is hardcoded +F32 stereo @ 48 kHz (`headroom-dsp::limiter`'s 4× oversampler is +sized for 48 k); when the real sink negotiates a different rate +(44.1 kHz, 96 kHz, 192 kHz), PipeWire inserts a downstream +resampler between `filter.playback` and the sink. Polynomial / +windowed-sinc resamplers can elevate inter-sample peaks slightly +through their own reconstruction, so the limiter's true-peak +guarantee leaks across that resampling stage. In practice the +elevation is small (a few tenths of a dB worst case for a clean +band-limited resampler), and the contract still holds at the bus +output where headroom is in control. **For the contract to hold +end-to-end the filter would need to match the real sink's rate +and rebuild its DSP coefficients on rate-change** — that's the +v1 work tracked as PLAN §11 "filter rate matching" (deferred from +8d, gated on a multi-rate hardware test bench). + **Soft tier — the comfort cap.** Targets a *dynamic* ceiling computed as `program_lufs + max_psr_db`. Smooth attack/release envelope so the gain reduction sounds like volume riding, not a slap. Pulls transients @@ -903,6 +920,14 @@ lost. Pick up by name when the trigger that gates them fires. for simplicity. Revisit if real users ask for it; the store-level change is a flag on the setter methods. **Dormant** — no user has asked through Phase 8. +- **Filter rate matching to the real sink.** *(F5 follow-up.)* §3.1 + documents the contract leak when the real sink runs at a + non-48 kHz native rate. Closing it requires dynamic + `FILTER_SAMPLE_RATE`, kernel rebuild on real-sink change + (compressor + limiter coefficients are rate-dependent), and + Layer A's `LAYER_A_BLOCK_DT_S` constant becoming dynamic too. + Gated on a multi-rate hardware test bench — no point shipping + the refactor without something to validate it against. **v1 scope.** - ~~**Filter playback BUSY spikes (periodic, ~10 s cadence).**~~ **Closed in 8e (`d52cd6d`).** The instrumentation added by 8e did not reproduce the ~8×-baseline outlier pattern in a ~3 min From 5c769a1226ea71b887562ceecef1d7925ed62360 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 19:42:12 +1000 Subject: [PATCH 15/21] fix: close audio-gap on unchanged ReevaluateAll; reset compressor on enable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two self-review follow-ups from the F1/F2 commits surfaced by an audio-correctness pass over `244367c..HEAD`. Both are low-risk, high-signal fixes — the kind that prevent users complaining about "weird little blips" when changing profiles or unmuting the compressor. ## Audio-gap on `PwCommand::ReevaluateAll` `enqueue_route` used to unconditionally drop `managed_route_links[node_id]` before the next 50 ms drain tick rebuilt. With `object.linger="false"` on the link-factory props, dropping the `Link` proxy destroys the actual graph link immediately. Result: every profile reload / route set / route unset / bypass toggle caused a 21–42 ms audio dropout on every already-correctly-routed stream — even when nothing about the stream's routing had actually changed. `managed_route_links` now carries the target sink name alongside the `Link` proxies (new `ManagedRoute` struct: `target_sink_name` + `links`). `enqueue_route` only drops when the target name differs from the stored one; the unchanged case leaves the live links intact, and `apply_pending_routes`' destroy/create loop sees its `want_set` already satisfied and exits as a no-op. Live verification: pw-cat /tmp/sine streaming through processed, issue `route set firefox bypass` (rule that doesn't touch pw-cat). Before this fix the link IDs would flip; after, link IDs 83 + 122 stayed identical across `reevaluating all known streams streams=1` in the daemon log. Listener-visible gap goes from one quantum to zero. The path that *does* change target (real bypass toggle, real-sink hot-swap, a rule edit that flipped the stream's decision) still drops + rebuilds — the gap there is unavoidable without a core-sync barrier or a "transition through both old and new links" choreography. That's acceptable: the user explicitly asked for the route change in those cases. ## Compressor envelope reset across `enabled` transition F6 made `compressor.enabled = false` actually skip processing, but didn't touch the envelope or RMS state — which kept ticking forward during enabled periods, sat stale during disabled periods, and then bled out via release on the first re-enable. With long release times this meant up to ~100 ms of artificial gain reduction after switching from a `transparent` profile back to a compressing one, for no acoustic reason. `Compressor::set_config` now detects the `disabled → enabled` transition and resets `envelope_db`, `rms_state`, and `last_gr_db` so the compressor starts from a clean state — same behaviour as a freshly-constructed `Compressor::new(...)`. Same-enabled transitions (parameter tweaks while enabled, or no-op `set_config` while disabled) leave the envelope alone, so live tweaks still don't pop. Regression test `compressor::tests::enable_transition_resets_stale_envelope` winds the envelope hot, toggles disable+enable via two `set_config` calls, then asserts the next quiet sample produces zero GR. Without the reset that assertion would fail by ~5+ dB. ## Verified 190 tests pass (+1 for the envelope reset; +0 for the link fix — exercised by live-smoke since it's about side-effect timing not value); clippy clean at `-D warnings --all-targets`. --- crates/headroom-core/src/pw/registry.rs | 55 ++++++++++++++++++--- crates/headroom-dsp/src/compressor.rs | 66 ++++++++++++++++++++++++- 2 files changed, 111 insertions(+), 10 deletions(-) diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index 32631e8..c271c60 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -127,6 +127,18 @@ struct PendingRoute { route: Route, } +/// The daemon-owned routing links for one stream, paired with the +/// target sink name they were built for. Stored in +/// `RoutingState::managed_route_links` so the bypass / profile +/// re-evaluation paths can ask "is the stream already routed to +/// this sink?" without losing the existing live links to find out +/// — keeping them alive avoids the audio gap the destroy+rebuild +/// cycle otherwise introduces. +struct ManagedRoute { + target_sink_name: String, + links: Vec, +} + /// Subject id passed to `set_property` for keys that aren't bound to /// a specific node (system-wide settings like `default.audio.sink`). const METADATA_SUBJECT_GLOBAL: u32 = 0; @@ -192,8 +204,14 @@ pub struct RoutingState { pending_routes: HashMap, /// Explicit `link-factory` `Link` proxies owned by the daemon, /// keyed by source stream node id. Kept alive so the links - /// persist; dropped on stream removal or route change. - managed_route_links: HashMap>, + /// persist; dropped on stream removal or *target-changing* + /// route change. The accompanying `target_sink_name` lets + /// `enqueue_route` skip the drop when a re-evaluation arrives + /// at the same target (the common case for profile.reload + /// / route.unset on an unaffected stream) — avoiding the + /// 21-42 ms audio gap that an unconditional drop+rebuild + /// would cause. + managed_route_links: HashMap, /// Window-based limit for `default.audio.sink` re-assertions /// so a hostile WP policy can't pull us into a hot loop. /// `(window_start, attempts_in_window)`. See @@ -1023,11 +1041,25 @@ impl RoutingState { app_label: String, route: Route, ) { - // Replacing intent: drop any old managed links for this - // stream so apply_pending_routes can rebuild against the new - // target. Dropping the proxies destroys the links via - // `object.linger = "false"`. - self.managed_route_links.remove(&node_id); + // If we already have live links for this stream pointing + // at the *same* sink, leave them alone. `apply_pending_routes` + // will run its idempotent destroy/create pass on the next + // drain tick and find the want_set already satisfied — a + // no-op. This is the common case for profile.reload / + // route.set / route.unset / profile.use when the change + // doesn't move *this* stream, and skipping the drop avoids + // the 21-42 ms audio gap an unconditional rebuild would + // cost. When the target *did* change (bypass toggle, + // real-sink hot-swap, or rule edit that flipped this + // stream's decision), drop the proxies so the old links + // tear down before the new ones come up. + let already_at_target = self + .managed_route_links + .get(&node_id) + .is_some_and(|m| m.target_sink_name == target_sink_name); + if !already_at_target { + self.managed_route_links.remove(&node_id); + } self.pending_routes.insert( node_id, PendingRoute { @@ -1156,6 +1188,7 @@ impl RoutingState { let mut created: Vec = self .managed_route_links .remove(&node_id) + .map(|m| m.links) .unwrap_or_default(); let mut all_ok = true; for (out_port, in_port) in &want { @@ -1179,7 +1212,13 @@ impl RoutingState { } } if !created.is_empty() { - self.managed_route_links.insert(node_id, created); + self.managed_route_links.insert( + node_id, + ManagedRoute { + target_sink_name: intent.target_sink_name.clone(), + links: created, + }, + ); } if all_ok { tracing::info!( diff --git a/crates/headroom-dsp/src/compressor.rs b/crates/headroom-dsp/src/compressor.rs index a38f08d..906fd96 100644 --- a/crates/headroom-dsp/src/compressor.rs +++ b/crates/headroom-dsp/src/compressor.rs @@ -117,11 +117,23 @@ impl Compressor { self.last_gr_db } - /// Update parameters. Recomputes alphas. Envelope state is kept, - /// so live tweaks don't pop. + /// Update parameters. Recomputes alphas. Envelope state is kept + /// across same-enabled transitions so live tweaks don't pop, but + /// reset on a `disabled → enabled` transition so a stale + /// envelope from before the disable doesn't bleed out at the + /// release time-constant when processing resumes (otherwise + /// switching from a `transparent` profile back to a compressing + /// one would briefly duck on the first ~100 ms of audio for no + /// reason). pub fn set_config(&mut self, cfg: CompressorConfig) { let cfg = cfg.sanitized(); + let was_disabled = !self.cfg.enabled; self.cfg = cfg; + if was_disabled && self.cfg.enabled { + self.envelope_db = -200.0; + self.rms_state = 0.0; + self.last_gr_db = 0.0; + } self.attack_alpha = time_to_alpha(cfg.attack_ms, self.sample_rate); self.release_alpha = time_to_alpha(cfg.release_ms, self.sample_rate); self.rms_alpha = time_to_alpha(cfg.rms_window_ms, self.sample_rate); @@ -295,6 +307,56 @@ mod tests { assert_eq!(c.gain_reduction_db(), 0.0); } + #[test] + fn enable_transition_resets_stale_envelope() { + // Run a loud signal through an enabled compressor to wind + // the envelope up, then disable + re-enable via set_config. + // The first sample after re-enable must NOT see the stale + // envelope (which would otherwise duck the signal until + // release_ms wound it down). Concretely: with a quiet input + // after re-enable, the envelope should be at the floor, so + // GR is zero — same as a freshly-constructed compressor. + let loud_cfg = CompressorConfig { + enabled: true, + threshold_db: -20.0, + ratio: 4.0, + attack_ms: 0.1, + release_ms: 1000.0, // slow release so stale state would otherwise stick + knee_db: 0.0, + makeup_db: Some(0.0), + ..CompressorConfig::default() + }; + let mut c = Compressor::new(loud_cfg, 48_000.0); + // Drive hot signal to wind envelope up. + for _ in 0..2_000 { + c.process_frame(0.5, 0.5); + } + assert!( + c.gain_reduction_db() < -5.0, + "precondition: envelope should be wound up; gr={}", + c.gain_reduction_db() + ); + + // Disable, then re-enable — should reset. + let mut disabled_cfg = loud_cfg; + disabled_cfg.enabled = false; + c.set_config(disabled_cfg); + c.set_config(loud_cfg); + + // Now drive a quiet signal. With reset envelope, GR should + // ride near zero; without reset, the stale envelope would + // bleed gain reduction out over ~release_ms. + let (l, r) = c.process_frame(0.001, 0.001); + assert!( + c.gain_reduction_db().abs() < 0.01, + "envelope didn't reset across enable transition; gr={}", + c.gain_reduction_db() + ); + // Output should be quiet (within makeup-applied scale). + assert!(l.abs() < 0.01); + assert!(r.abs() < 0.01); + } + #[test] fn static_curve_at_threshold_with_soft_knee() { // At exactly threshold, soft knee contributes exactly half the From ec4920666021648d0eac24d822892fee1b32919d Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 19:44:49 +1000 Subject: [PATCH 16/21] fix(F4): clear real_sink.name when the adopted sink departs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex audit of the F1-F6 sweep flagged this. The fallback path in `try_capture_real_sink` adopts the first non-processed Audio/Sink when no real sink is known; if that sink later disconnects (USB DAC pulled, Bluetooth peer drops), `on_global_remove`'s `sinks_by_name.retain` was clearing `s.real_sink.node_id` but leaving `s.real_sink.name` set to the departed name. Symptoms: - `apply_pending_routes` then logs "target sink not yet on registry" for every bypass route and queues forever — `name` no longer resolves through `sinks_by_name`. - `adopt_new_real_sink` from the metadata listener would normally rescue this, but WP only re-fires `default.audio.sink` on actual changes; if the user's default is unchanged in WP's view (because the departed sink wasn't WP's pick), no event arrives. The retain-callback now clears both `name` and `node_id` when the removed node's name matches `real_sink.name`. The F4 fallback will then pick a replacement from the next non-processed Audio/Sink the registry surfaces, or a fresh metadata event will set a specific choice — either path recovers cleanly. Codex's other finding (theoretical duplicate-link creation in multi-channel apply_pending_routes when the link listener lags behind the drain timer within a single tick) is real-but-unlikely: the listener fires within microseconds on the same event loop; drain ticks are 50 ms apart, so the listener always catches up before the next drain. The unchanged-target gap-mitigation from `5c769a1` also reduces exposure — most ReevaluateAll passes don't hit the create loop at all now. Filed as a "watch but don't fix" note inline in the routing follow-up memory. 190 tests pass; clippy clean. --- crates/headroom-core/src/pw/registry.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index c271c60..c4f5904 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -1485,7 +1485,20 @@ impl RoutingState { if id == node_id { tracing::debug!(node_id, name, "real sink removed from registry"); let mut s = self.daemon.lock(); - if s.real_sink.node_id == Some(node_id) { + // Clear BOTH name and node_id when the departing + // sink is our preferred real sink. Just nulling + // node_id (the previous behaviour) left the name + // pinned to a sink that no longer exists, so + // `apply_pending_routes` would queue every bypass + // route forever against a stale target name. The + // F4 fallback or a fresh `default.audio.sink` event + // can then pick a replacement. + if s.real_sink.name.as_deref() == Some(name.as_str()) { + s.real_sink.name = None; + s.real_sink.node_id = None; + } else if s.real_sink.node_id == Some(node_id) { + // Defensive: id matched but name didn't (sinks + // shouldn't double-register). Null the id. s.real_sink.node_id = None; } false From 4a80a16d7914b587a5912ecd29d3ac33314a6c1b Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 19:50:11 +1000 Subject: [PATCH 17/21] docs: explain why mono streams aren't link-enforced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `pair_count < 2` early-return in `apply_pending_routes` looked arbitrary from the outside (and Codex+self-review both flagged it as a possible bug). It's actually a deliberate choice: WP's source-side upmix adapter handles mono → stereo cleanly today, and broadcasting one source port to N target ports via link-factory fanout requires the limiter's stereo-link semantics and the BS.1770 multichannel weights to make sense for N=1 — neither generalises trivially. The proper fix lives in the v1 multichannel pipeline. Replaces the old "PipeWire's adapter is responsible for any downmix" comment with the actual reasoning + the contract caveat (`route.set` on a mono app won't move it; the metadata write is a hint, not enforcement) so a future contributor doesn't accidentally "fix" it without weighing the trade-offs. No code change beyond the comment + the debug-log message. --- crates/headroom-core/src/pw/registry.rs | 29 ++++++++++++++++--------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index c4f5904..13dee0f 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -1114,22 +1114,31 @@ impl RoutingState { }; // Pair by ordinal up to whichever side has fewer // channels. For stereo→stereo this is the original - // `take(2)`. For wider streams (surround) routed - // Bypass to a wide sink we pair all N channels — fixes - // the F3 bug where the old `take(2)` silently dropped - // the centre, LFE, and surround channels of a 5.1 - // stream. If the wider side has more channels than the - // narrower side, the extra ports are left unlinked - // here; the destruction pass below also leaves them - // alone (they don't land on the target sink at all). - // PipeWire's adapter is responsible for any downmix. + // `take(2)`. For wider streams (surround) routed Bypass + // to a wide sink we pair all N channels — fixes the F3 + // bug where the old `take(2)` silently dropped the + // centre, LFE, and surround channels of a 5.1 stream. + // + // **Mono streams are intentionally not enforced.** A + // single-port source needs broadcast (1→N fanout) to + // play on both channels of a stereo sink, and the + // limiter's stereo-link semantics + the BS.1770 + // multichannel weights don't generalise cleanly to + // `N=1`. WP's source-side upmix adapter handles mono + // → stereo correctly today, so we let it. The cost is + // a small contract leak (`route.set` on a mono app + // won't actually move it; the metadata write is a + // hint, not enforcement) — acceptable for v0, the + // proper fix is part of the v1 multichannel pipeline. + // See PLAN §11 "Filter rate matching" and the + // multichannel-deferral memory. let pair_count = src_outs.len().min(target_ins.len()); if pair_count < 2 { tracing::debug!( node_id, src_outs = src_outs.len(), target_ins = target_ins.len(), - "pending route: not enough ports yet" + "pending route: not enough ports for stereo+ pairing (mono left to WP)" ); continue; } From 86d00c43d12500a46cfc9a94e8f44383762f0ef1 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 20:43:55 +1000 Subject: [PATCH 18/21] filter rate matching A+B: runtime-parameterised rate at boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drops the FILTER_SAMPLE_RATE const dependency from the filter's creation path so the audio thread can run at whatever rate the real sink negotiates, not unconditionally 48 kHz. Closes one of the two output-edge resamples PLAN §3.1's F5 caveat called out — content matching the real-sink rate now passes through the limiter without an output-side resample elevating its true peaks. Phase A (foundation) - `Filter::create(core, init, sample_rate)` takes the rate as a runtime parameter. `DEFAULT_SAMPLE_RATE` keeps 48 kHz as the fallback constant; `FILTER_SAMPLE_RATE` is kept as a back-compat alias. - `build_format_pod_bytes(sample_rate)` parameterised so the SPA Format the filter advertises matches the chosen rate. - `FilterBundle.sample_rate` exposed so the AGC controller and `runtime` can size their own state. - New `LimiterConfig::sanitize_for_rate(sample_rate)` caps the oversample factor so the internal (post-upsample) rate stays ≤ 192 kHz: 48 k base → 4× = 192 k; 96 k → 2× = 192 k; 192 k → 1× = 192 k. Keeps the FIR cost from doubling each time the base rate doubles, with negligible loss of true-peak detection quality at high base rates (the signal already has plenty of bandwidth). Two regression tests lock the math in. Phase B (data plumbing) - `SinkInfo` (wire-level) gains an optional `sample_rate` field. `headroom status` now reports the processed sink's running rate and the real sink's native rate — useful for debugging "did the daemon actually match my hardware?" without resorting to `pw-link`. - `state::RealSink.sample_rate` populated by the registry watcher from two sources: - The `audio.rate` property (many virtual sinks expose it). - A `Format`-param listener bound to the real sink's `Node` proxy (ALSA sinks only expose the rate in the negotiated Format, not in their property dict). New `install_real_sink_format_listener` mirrors the channelVolumes-listener pattern Layer A already uses. Listener cleaned up in `on_global_remove` when the real sink departs. - `state::DaemonState.filter_sample_rate` mirrors the bus filter's currently-running rate; surfaced in `status`. - Layer A's block-period constant becomes a runtime function (`layer_a_block_dt_s(sample_rate)`) so 96 k / 192 k hardware gets correctly-scaled controller time-constants. Known gap: filter created at boot uses whatever rate is known at that moment. For ALSA sinks the Format listener fires ~tens of ms *after* the registry capture — by which time the filter is already created at the fallback rate. The next commit (Phase C) rebuilds the filter when the listener delivers a rate different from what the filter is running at. Verified - 191 tests pass (was 189; +2 for the new `sanitize_for_rate` cases); clippy clean at -D warnings --all-targets. - Live: cold-boot against a 48 kHz Mbox shows `status.sinks.processed.sample_rate = 48000` + `status.sinks.real.sample_rate = 48000`, daemon log records "creating filter at real-sink-matched rate initial_rate=48000" and "real sink Format negotiated; updating sample_rate new_rate=48000" within ~55 ms of each other. For sinks where `audio.rate` IS in props (some virtual sinks) the rate is captured before filter creation. --- crates/headroom-core/src/ipc/ops.rs | 5 + crates/headroom-core/src/pw/filter.rs | 52 +++++-- crates/headroom-core/src/pw/registry.rs | 186 +++++++++++++++++++++--- crates/headroom-core/src/runtime.rs | 24 ++- crates/headroom-core/src/state.rs | 17 ++- crates/headroom-dsp/src/limiter.rs | 69 ++++++++- crates/headroom-ipc/src/proto.rs | 7 + 7 files changed, 318 insertions(+), 42 deletions(-) diff --git a/crates/headroom-core/src/ipc/ops.rs b/crates/headroom-core/src/ipc/ops.rs index 3b480b0..9ba2081 100644 --- a/crates/headroom-core/src/ipc/ops.rs +++ b/crates/headroom-core/src/ipc/ops.rs @@ -66,6 +66,11 @@ fn status(id: u64, state: &SharedState) -> Response { node_id: s.processed_sink_id, name: Some(crate::pw::sink::NODE_NAME.to_owned()), ready: s.processed_sink_id.is_some(), + // The processed sink advertises whatever rate the + // filter is currently running at (rate-matched to + // the real sink). `None` only during very early + // boot before `Filter::create` lands. + sample_rate: s.filter_sample_rate, }, real: s.real_sink.clone(), }, diff --git a/crates/headroom-core/src/pw/filter.rs b/crates/headroom-core/src/pw/filter.rs index acf3580..cb41460 100644 --- a/crates/headroom-core/src/pw/filter.rs +++ b/crates/headroom-core/src/pw/filter.rs @@ -60,7 +60,19 @@ use crate::pw::sink::NODE_NAME as PROCESSED_SINK_NAME; /// constructed for this rate; if PipeWire negotiates a different /// rate the filter logs a warning and the DSP may sound slightly off /// in time-based parameters until Phase 4 wires rate updates. -pub const FILTER_SAMPLE_RATE: u32 = 48_000; +/// Sample rate the filter uses when no real sink is yet known +/// (cold boot, or `default.audio.sink` hasn't resolved). The +/// runtime overrides this via [`Filter::create`]'s `sample_rate` +/// argument once a real-sink rate is captured from the registry. +/// 48 kHz matches the PipeWire graph default; nothing else is +/// load-bearing at this number. +pub const DEFAULT_SAMPLE_RATE: u32 = 48_000; + +/// Backward-compatibility alias for the old const name. Internal +/// callers should take the rate as a parameter; this exists so +/// out-of-tree code (`headroom-core` doc readers, downstream +/// experiments) doesn't break on the rename. +pub const FILTER_SAMPLE_RATE: u32 = DEFAULT_SAMPLE_RATE; /// Number of channels the filter operates on (stereo only in v0). pub const CHANNELS: u32 = 2; @@ -270,6 +282,11 @@ pub struct FilterBundle { /// Playback callback timing stats. Updated lock-free from the /// audio thread; sampled by the AGC controller's slow tick. pub timing: SharedPlaybackTiming, + /// The sample rate the filter is running at — read from the + /// real sink at construction time, or [`DEFAULT_SAMPLE_RATE`] + /// if no real sink was known yet. Callers (runtime, + /// AgcController) need it to size their own state. + pub sample_rate: u32, } impl Filter { @@ -285,7 +302,11 @@ impl Filter { /// # Errors /// [`DaemonError::PipeWire`] if stream creation or connection /// fails. - pub fn create(core: &Core, init: FilterInit) -> Result { + pub fn create( + core: &Core, + init: FilterInit, + sample_rate: u32, + ) -> Result { let (producer, consumer) = RingBuffer::::new(RING_CAPACITY); let (cmd_producer, cmd_consumer) = RingBuffer::::new(CMD_RING_CAPACITY); let (measurement_producer, measurement_consumer) = @@ -296,9 +317,15 @@ impl Filter { let bus_metrics = crate::meters::shared(); let timing = crate::meters::shared_timing(); - let compressor = Compressor::new(init.compressor, FILTER_SAMPLE_RATE as f32); - let limiter = Limiter::new(init.limiter, FILTER_SAMPLE_RATE as f32); - let mut agc = AgcGain::new(init.agc, FILTER_SAMPLE_RATE as f32); + // The limiter's `sanitized()` caps the *internal* (post- + // oversample) rate, so a 96 kHz base + the default 4× + // oversample auto-drops to 2× → 192 kHz internal rather + // than 384 kHz. Keeps the FIR cost bounded as we follow + // higher real-sink rates. + let limiter_cfg = init.limiter.sanitize_for_rate(sample_rate as f32); + let compressor = Compressor::new(init.compressor, sample_rate as f32); + let limiter = Limiter::new(limiter_cfg, sample_rate as f32); + let mut agc = AgcGain::new(init.agc, sample_rate as f32); agc.set_enabled(init.agc_enabled); let capture = build_capture_stream(core)?; @@ -330,9 +357,9 @@ impl Filter { .map_err(|e| DaemonError::pipewire(format!("playback register: {e}")))?; // One format POD, two connects. Both streams want the same - // interpretation (F32LE stereo at FILTER_SAMPLE_RATE) and the + // interpretation (F32LE stereo at `sample_rate`) and the // POD bytes live on this stack for the duration of both calls. - let format_bytes = build_format_pod_bytes()?; + let format_bytes = build_format_pod_bytes(sample_rate)?; let format_pod = Pod::from_bytes(&format_bytes).ok_or_else(|| DaemonError::pipewire("Pod::from_bytes"))?; @@ -357,7 +384,7 @@ impl Filter { .map_err(|e| DaemonError::pipewire(format!("playback connect: {e}")))?; tracing::info!( - sample_rate = FILTER_SAMPLE_RATE, + sample_rate, channels = CHANNELS, ring_capacity = RING_CAPACITY, "filter streams created and connected" @@ -374,6 +401,7 @@ impl Filter { measurement_consumer, bus_metrics, timing, + sample_rate, }) } } @@ -419,12 +447,12 @@ fn build_playback_stream(core: &Core) -> Result { .map_err(|e| DaemonError::pipewire(format!("playback Stream::new: {e}"))) } -/// Serialize our preferred audio format (F32LE stereo at -/// [`FILTER_SAMPLE_RATE`]) into a SPA POD byte buffer. -fn build_format_pod_bytes() -> Result, DaemonError> { +/// Serialize our preferred audio format (F32LE stereo at the +/// runtime-supplied `sample_rate`) into a SPA POD byte buffer. +fn build_format_pod_bytes(sample_rate: u32) -> Result, DaemonError> { let mut info = AudioInfoRaw::new(); info.set_format(AudioFormat::F32LE); - info.set_rate(FILTER_SAMPLE_RATE); + info.set_rate(sample_rate); info.set_channels(CHANNELS); let obj = Object { diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index 13dee0f..61bce95 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -60,11 +60,25 @@ use crate::pw::tap::{MeasurementSample, StreamTap}; use crate::routing::{self, PwNodeInfo, RoutingDecision}; use crate::state::{RoutedStream, SharedState}; -/// Assumed audio-thread block period for Layer A controllers. Matches -/// the filter's hardcoded 1024-frame quantum at 48 kHz. Future work -/// reads the negotiated quantum from PipeWire when constructing each -/// controller. -const LAYER_A_BLOCK_DT_S: f32 = 1024.0 / 48_000.0; +/// Assumed audio-thread quantum for Layer A's block-rate +/// controllers. PipeWire's default is 1024 frames; nodes may +/// negotiate something different, but the controller's smoothing +/// constants are tolerant of small mismatches (the 30 ms +/// anti-bounce smoother stays in the right order of magnitude +/// from 512 to 2048 frames at any reasonable sample rate). The +/// actual `dt_s = QUANTUM_FRAMES / sample_rate` is computed at +/// controller-spawn time from the daemon's known real-sink rate +/// so 96 kHz / 192 kHz hardware gets correctly-scaled +/// time-constants without changing the filter quantum. +const LAYER_A_QUANTUM_FRAMES: f32 = 1024.0; + +/// Compute Layer A's block-period (seconds) for the active +/// sample rate. Falls back to the 48 kHz reference when no real +/// sink rate is known yet. +fn layer_a_block_dt_s(sample_rate: Option) -> f32 { + let sr = sample_rate.unwrap_or(crate::pw::filter::DEFAULT_SAMPLE_RATE); + LAYER_A_QUANTUM_FRAMES / (sr as f32) +} /// Lightweight view of a `Port` registry global. We track these to /// enable explicit port-level linking for Layer A taps: PipeWire's @@ -224,6 +238,13 @@ pub struct RoutingState { /// PipeWire. Populated by `try_route_stream`, cleared in /// `on_global_remove`. known_streams: HashMap, + /// Node proxy + Format-param listener for the current real + /// sink, used to capture its negotiated `audio.rate` (ALSA + /// sinks don't expose this in their property dict; it only + /// appears in the Format param). `Some` for the lifetime of + /// a real sink; replaced whenever `real_sink.node_id` + /// changes, dropped on removal. + real_sink_format_listener: Option<(u32, Node, NodeListener)>, } /// Per-stream Layer A bundle: the tap (audio path), the controller @@ -291,6 +312,7 @@ impl RoutingState { managed_route_links: HashMap::new(), default_reassertion: None, known_streams: HashMap::new(), + real_sink_format_listener: None, } } @@ -537,24 +559,110 @@ impl RoutingState { if name == PROCESSED_SINK_NAME { return; // tracked elsewhere } + let rate = dict.get("audio.rate").and_then(|s| s.parse::().ok()); self.sinks_by_name.insert(name.to_owned(), global.id); - let mut s = self.daemon.lock(); - if s.real_sink.name.is_none() { - tracing::info!( - node_id = global.id, - name, - "no preferred_real_sink yet; adopting first available Audio/Sink as fallback" - ); - s.real_sink.name = Some(name.to_owned()); - s.real_sink.node_id = Some(global.id); - } else if s.real_sink.name.as_deref() == Some(name) && s.real_sink.node_id != Some(global.id) { - tracing::info!( - node_id = global.id, - name, - "resolved preferred_real_sink node id" - ); - s.real_sink.node_id = Some(global.id); + let mut became_real_sink = false; + { + let mut s = self.daemon.lock(); + if s.real_sink.name.is_none() { + tracing::info!( + node_id = global.id, + name, + ?rate, + "no preferred_real_sink yet; adopting first available Audio/Sink as fallback" + ); + s.real_sink.name = Some(name.to_owned()); + s.real_sink.node_id = Some(global.id); + s.real_sink.sample_rate = rate; + became_real_sink = true; + } else if s.real_sink.name.as_deref() == Some(name) { + if s.real_sink.node_id != Some(global.id) { + tracing::info!( + node_id = global.id, + name, + "resolved preferred_real_sink node id" + ); + s.real_sink.node_id = Some(global.id); + became_real_sink = true; + } + // Update rate every time we (re-)see the sink: a + // first-registration without `audio.rate` (common + // for ALSA sinks) gets filled in by the Format + // listener installed below. + if rate.is_some() && s.real_sink.sample_rate != rate { + tracing::info!( + node_id = global.id, + name, + old_rate = ?s.real_sink.sample_rate, + new_rate = ?rate, + "real sink rate updated" + ); + s.real_sink.sample_rate = rate; + } + } } + // ALSA sinks don't carry `audio.rate` in their property + // dict — it lives in the negotiated Format param. Bind the + // node and subscribe to Format events so we can pull the + // rate from the next param callback. Only do this for the + // current real sink (the one filter routing actually + // matters for) so we don't accumulate proxies for sinks + // we'll never touch. + if became_real_sink { + self.install_real_sink_format_listener(global); + } + } + + /// Bind the node behind `sink_global` and subscribe to its + /// `Format` param so changes (including the initial + /// negotiated value PipeWire replays on subscribe) update + /// `real_sink.sample_rate`. Replaces any previously-installed + /// listener for a different node. + fn install_real_sink_format_listener(&mut self, sink_global: &GlobalObject<&DictRef>) { + let node_id = sink_global.id; + if let Some((prev_id, _, _)) = &self.real_sink_format_listener { + if *prev_id == node_id { + return; // already bound, nothing to do + } + } + let node = match self.registry.bind::(sink_global) { + Ok(n) => n, + Err(e) => { + tracing::warn!( + node_id, + error = %e, + "failed to bind real sink Node proxy; sample rate will fall back to default" + ); + self.real_sink_format_listener = None; + return; + } + }; + let daemon = self.daemon.clone(); + let listener = node + .add_listener_local() + .param(move |_seq, id, _index, _next, param_opt| { + if id != ParamType::Format { + return; + } + let Some(param) = param_opt else { return }; + let Some(rate) = extract_audio_rate(param) else { + return; + }; + let mut s = daemon.lock(); + if s.real_sink.sample_rate == Some(rate) { + return; + } + tracing::info!( + node_id, + old_rate = ?s.real_sink.sample_rate, + new_rate = rate, + "real sink Format negotiated; updating sample_rate" + ); + s.real_sink.sample_rate = Some(rate); + }) + .register(); + node.subscribe_params(&[ParamType::Format]); + self.real_sink_format_listener = Some((node_id, node, listener)); } /// Capture the global id of `headroom-filter.playback` when the @@ -837,9 +945,10 @@ impl RoutingState { app_level::evaluate(info, &s.profiles.effective().per_app) }; let Some(rule) = rule else { return }; + let block_dt_s = layer_a_block_dt_s(self.daemon.lock().real_sink.sample_rate); match StreamTap::start(&self.core, info.node_id) { Ok((tap, consumer)) => { - let controller = AppLevelController::new(rule, LAYER_A_BLOCK_DT_S); + let controller = AppLevelController::new(rule, block_dt_s); // Bind a Node proxy so 6d can write // `Props.channelVolumes`. If this fails we still spawn // the tap — the controller runs and we'll log when it @@ -1505,6 +1614,15 @@ impl RoutingState { if s.real_sink.name.as_deref() == Some(name.as_str()) { s.real_sink.name = None; s.real_sink.node_id = None; + s.real_sink.sample_rate = None; + drop(s); + // Drop the Format-param listener too; it points + // at a node that no longer exists. + if let Some((prev_id, _, _)) = &self.real_sink_format_listener { + if *prev_id == node_id { + self.real_sink_format_listener = None; + } + } } else if s.real_sink.node_id == Some(node_id) { // Defensive: id matched but name didn't (sinks // shouldn't double-register). Null the id. @@ -1583,6 +1701,30 @@ fn install_param_listener( .register() } +/// Parse a `Format` POD looking for `SPA_FORMAT_AUDIO_rate` and +/// return its integer value. ALSA sinks don't expose `audio.rate` +/// in their property dict — the rate only appears in the +/// negotiated Format param. Returns `None` if the pod isn't a +/// Format object or doesn't carry the rate field. +fn extract_audio_rate(pod: &Pod) -> Option { + let bytes = pod.as_bytes(); + let (_, value) = PodDeserializer::deserialize_any_from(bytes).ok()?; + let Value::Object(obj) = value else { return None }; + if obj.id != ParamType::Format.as_raw() { + return None; + } + for prop in obj.properties { + if prop.key == libspa_sys::SPA_FORMAT_AUDIO_rate { + if let Value::Int(rate) = prop.value { + if rate > 0 { + return Some(rate as u32); + } + } + } + } + None +} + /// Parse a `Props` POD looking for `SPA_PROP_channelVolumes` and /// return the first channel's value. Returns `None` if the pod isn't /// a Props object, or doesn't carry channelVolumes, or carries it in diff --git a/crates/headroom-core/src/runtime.rs b/crates/headroom-core/src/runtime.rs index 8c4a85e..0093c25 100644 --- a/crates/headroom-core/src/runtime.rs +++ b/crates/headroom-core/src/runtime.rs @@ -103,14 +103,32 @@ pub fn run(profiles: ProfileStore) -> Result<(), DaemonError> { agc_enabled: effective.agc.enabled, } }; + // Read the real sink's native rate (captured during the brief + // window the registry watcher has been running) so the filter + // can match it and skip the output-edge resample for content + // at that rate. Falls back to PipeWire's 48 kHz default if the + // real sink hasn't surfaced yet — Phase C will rebuild the + // filter when the rate later resolves to something else. + let initial_rate = daemon_state + .lock() + .real_sink + .sample_rate + .unwrap_or(crate::pw::filter::DEFAULT_SAMPLE_RATE); + tracing::info!(initial_rate, "creating filter at real-sink-matched rate"); + let FilterBundle { filter: _filter, control: filter_control, measurement_consumer, bus_metrics, timing, - } = Filter::create(pw.core(), filter_init)?; - daemon_state.lock().filter_control = Some(filter_control.clone()); + sample_rate: filter_rate, + } = Filter::create(pw.core(), filter_init, initial_rate)?; + { + let mut s = daemon_state.lock(); + s.filter_control = Some(filter_control.clone()); + s.filter_sample_rate = Some(filter_rate); + } // Spin up the slow AGC controller. Ticks on the PipeWire main // loop via a timer source; reads the active profile's [agc] @@ -120,7 +138,7 @@ pub fn run(profiles: ProfileStore) -> Result<(), DaemonError> { // `profile.meters.publish_hz` (capped at 20 Hz, the AGC tick // rate) — 4g. let agc_controller = AgcController::new( - crate::pw::filter::FILTER_SAMPLE_RATE, + filter_rate, crate::pw::filter::CHANNELS, measurement_consumer, filter_control, diff --git a/crates/headroom-core/src/state.rs b/crates/headroom-core/src/state.rs index f87074c..c62249a 100644 --- a/crates/headroom-core/src/state.rs +++ b/crates/headroom-core/src/state.rs @@ -55,6 +55,13 @@ pub struct DaemonState { /// PipeWire global id of `headroom-processed`, captured when the /// registry surfaces it. `None` until then. pub processed_sink_id: Option, + /// Sample rate the filter is currently running at, in Hz. + /// `None` until `Filter::create` has been called (very early + /// boot only). Matches the real sink's native rate at the time + /// the filter was last (re)built. Used to populate the + /// processed sink's `sample_rate` field in `status` and to + /// drive Layer A's block-period. + pub filter_sample_rate: Option, /// Snapshot of the user's preferred hardware sink. Phase 4h /// keeps this fresh from `default.audio.sink`. pub real_sink: SinkInfo, @@ -89,6 +96,7 @@ impl DaemonState { started_at: Instant::now(), profiles, processed_sink_id: None, + filter_sample_rate: None, real_sink: SinkInfo::default(), streams: HashMap::new(), broadcaster: Broadcaster::new(), @@ -111,13 +119,14 @@ impl DaemonState { return None; } self.real_sink = SinkInfo { - // node_id stays unknown for now — Headroom routes by name - // via `target.object = {"name":"…"}`, which is what - // WirePlumber expects. 4i may resolve the id when ad-hoc - // per-stream overrides need it. + // node_id + sample_rate stay unknown for now — + // registry's `try_capture_real_sink` resolves both + // when it sees the matching `Audio/Sink` global. The + // 4i routing path operates on name alone. node_id: None, name: Some(new_name.to_owned()), ready: true, + sample_rate: None, }; Some( self.streams diff --git a/crates/headroom-dsp/src/limiter.rs b/crates/headroom-dsp/src/limiter.rs index 617f7d2..3e61f0a 100644 --- a/crates/headroom-dsp/src/limiter.rs +++ b/crates/headroom-dsp/src/limiter.rs @@ -140,10 +140,22 @@ impl Default for LimiterConfig { } } +/// Internal-rate cap (Hz). The limiter's true-peak detector +/// upsamples to `sample_rate × oversample`. Above ~192 kHz the +/// FIR cost rises linearly with effectively no gain — at base +/// rates ≥ 96 kHz the signal already has plenty of bandwidth +/// for inter-sample-peak detection. We cap the *effective* +/// internal rate here and drop the oversample factor on high +/// base rates accordingly. +pub const MAX_INTERNAL_RATE_HZ: f32 = 192_000.0; + impl LimiterConfig { /// Sanitize a user-supplied configuration: clamp ceiling, /// oversample factor, ensure odd FIR length, sanitize the soft - /// tier if present. + /// tier if present. Rate-agnostic — callers that know the + /// audio thread's sample rate should prefer + /// [`Self::sanitize_for_rate`] so the oversample factor scales + /// down on high-rate inputs. #[must_use] pub fn sanitized(mut self) -> Self { if self.ceiling_dbtp > 0.0 { @@ -162,6 +174,27 @@ impl LimiterConfig { self } + /// Sanitize and additionally cap the oversample factor so the + /// post-upsample internal rate stays ≤ [`MAX_INTERNAL_RATE_HZ`]. + /// Examples at the default `oversample = 4`: + /// 44.1 kHz → 4× → 176.4 kHz (under cap, untouched) + /// 48 kHz → 4× → 192 kHz (at cap, untouched) + /// 96 kHz → 2× → 192 kHz (cap engaged, dropped from 4) + /// 192 kHz → 1× → 192 kHz (cap engaged, no oversampling) + /// Always returns at least `oversample = 1`. + #[must_use] + pub fn sanitize_for_rate(self, sample_rate: f32) -> Self { + let mut s = self.sanitized(); + if sample_rate > 0.0 { + let max_os = + (MAX_INTERNAL_RATE_HZ / sample_rate).floor().max(1.0) as usize; + if s.oversample > max_os { + s.oversample = max_os; + } + } + s + } + /// Convenience: brickwall only (no soft tier). #[must_use] pub fn brickwall_only() -> Self { @@ -615,6 +648,40 @@ mod tests { use super::*; use std::f32::consts::PI; + // ---------------------------------------------------------------- + // sanitize_for_rate: oversample factor scales down so the + // internal (post-upsample) rate stays bounded. + // ---------------------------------------------------------------- + + #[test] + fn sanitize_for_rate_caps_oversample_at_internal_192k() { + // Default config has oversample = 4. + let default = LimiterConfig::default(); + assert_eq!(default.oversample, 4); + + // At 48 kHz: 4× = 192 kHz, at the cap, untouched. + assert_eq!(default.sanitize_for_rate(48_000.0).oversample, 4); + // At 44.1 kHz: 4× = 176.4 kHz, under the cap. + assert_eq!(default.sanitize_for_rate(44_100.0).oversample, 4); + // At 96 kHz: 4× = 384 kHz, exceeds; drop to 2× = 192 kHz. + assert_eq!(default.sanitize_for_rate(96_000.0).oversample, 2); + // At 192 kHz: cap forces oversample = 1. + assert_eq!(default.sanitize_for_rate(192_000.0).oversample, 1); + // Pathological rate above the cap still leaves at least 1. + assert_eq!(default.sanitize_for_rate(384_000.0).oversample, 1); + } + + #[test] + fn sanitize_for_rate_preserves_user_lower_oversample() { + // User who explicitly set oversample = 2 at 48 kHz should + // keep it; the rate cap doesn't push the value *up*. + let cfg = LimiterConfig { + oversample: 2, + ..LimiterConfig::default() + }; + assert_eq!(cfg.sanitize_for_rate(48_000.0).oversample, 2); + } + // ---------------------------------------------------------------- // try_set_config: scalar updates apply in place, structural // changes are rejected. diff --git a/crates/headroom-ipc/src/proto.rs b/crates/headroom-ipc/src/proto.rs index cbe761c..a4114c6 100644 --- a/crates/headroom-ipc/src/proto.rs +++ b/crates/headroom-ipc/src/proto.rs @@ -391,6 +391,13 @@ pub struct SinkInfo { /// True if the sink is currently linked and accepting audio. #[serde(default)] pub ready: bool, + /// Sink's native sample rate (Hz), when known. The filter + /// matches the *real* sink's rate to skip the output-edge + /// resample; the processed sink advertises whatever rate the + /// filter is currently running at. Older clients that don't + /// understand the field treat it as absent (serde `default`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub sample_rate: Option, } /// One playback stream and where it's routed. From ab02df23fe67fc5863ec510f5b47bbb28df74031 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 20:51:11 +1000 Subject: [PATCH 19/21] filter rate matching C: live rebuild when real-sink rate changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the cold-boot + hot-swap gap A+B left open. When the real sink's Format-param listener fires with a rate that doesn't match the filter's currently-running rate, the daemon now rebuilds the filter atomically and rebinds the slow AGC controller to the new measurement ring + FilterControl. What triggers a rebuild - Cold-boot against an ALSA sink. `audio.rate` isn't in the props dict, so the registry-capture path falls back to 48 kHz and creates the filter at that rate. Tens of ms later the Format listener fires with the real rate (say 96 kHz). If different from the filter's current rate, post `PwCommand::RebuildFilter`. - Hot-swap. User runs `wpctl set-default ` and the new sink has a different native rate. `adopt_new_real_sink` swaps the Format listener; the next param event from the new node's negotiated Format triggers the same rebuild path. What the rebuild does - Snapshots `FilterInit` from the active profile under the daemon lock, then drops the lock before touching PipeWire. - Drops the old `Filter` (RAII tears down the two pw_streams + their listeners), then calls `Filter::create` at the new rate. ~50–100 ms audio gap on the processed path during the swap. - Updates `daemon.filter_control` + `daemon.filter_sample_rate` under the lock. - `AgcController::rebind(new_consumer, new_control, new_rate)` swaps the AGC's view atomically and rebuilds its `ebur128` instance at the new rate. - Runs `reevaluate_all` so any explicit links anchored at the old filter's now-gone ports get re-pinned to the new processed-sink ports on the next drain tick. Plumbing - New `PwCommand::RebuildFilter { sample_rate }`. - `RoutingState` gains `bus_filter: Option` (filter ownership moves from `runtime::run`'s local into routing state so the registry thread can swap it) and `agc_controller: Option>>` so the rebuild can call `rebind` on the slow loop. - `RoutingState::install_filter_rebuild_handles` is called once from `runtime` after `start_routing` + `AgcController::new`. - `PwContext::routing_state()` accessor exposes the `Rc>` so runtime can install the handles without threading them through `start_routing`'s signature. - The Format listener computes `need_rebuild = filter_sample_rate != Some(new_rate)` under the daemon lock, then sends the `RebuildFilter` command on `daemon.pw_command_tx` if needed. What doesn't change - Steady-state: when the daemon boots and the rate hasn't moved, no rebuild fires. The no-rebuild path is the common case for users whose hardware is 48 kHz native; nothing about their setup gets touched. - Layer A taps: orthogonal to the bus path. The rebuild doesn't touch `managed_streams`; existing taps keep their links. Verified - 191 tests still pass; clippy clean. - Cold-boot against the dev Mbox (48 kHz native): filter creates at 48 k, Format listener fires ~22 ms later detecting 48 k → `need_rebuild = false` → no rebuild posted. Status reports `processed.sample_rate = 48000`. The no-rebuild path is the one most users will hit. - Live rebuild against a non-48 kHz sink: not exercised in this commit (I can't reliably fabricate a non-48 kHz null sink via `pw-cli load-module` in the shell — same limitation 8d hit). The user's 96 kHz motherboard, once they activate its card profile and set it as default, is the next test target. --- crates/headroom-core/src/agc.rs | 21 ++++ crates/headroom-core/src/pw/command.rs | 12 ++ crates/headroom-core/src/pw/mod.rs | 9 ++ crates/headroom-core/src/pw/registry.rs | 156 ++++++++++++++++++++++-- crates/headroom-core/src/runtime.rs | 21 +++- 5 files changed, 209 insertions(+), 10 deletions(-) diff --git a/crates/headroom-core/src/agc.rs b/crates/headroom-core/src/agc.rs index 31b5cca..d541f59 100644 --- a/crates/headroom-core/src/agc.rs +++ b/crates/headroom-core/src/agc.rs @@ -330,6 +330,27 @@ impl AgcController { } self.filter_control.set_agc_target_db(0.0); } + + /// Rebind the controller to a freshly-built filter (Phase C of + /// the filter rate-matching work). The old `measurement_consumer` + /// and `filter_control` point at rtrbs whose producers were just + /// dropped — every send on them would now fail — so we swap in + /// the new bundle's handles and rebuild `ebur128` at the new + /// sample rate. Resets the smoother + the LUFS sentinel so the + /// controller starts clean on the new audio path; the brief + /// post-rebuild silence (~50–100 ms of dropped audio) is + /// inaudible compared to the rate-change event itself. + pub fn rebind( + &mut self, + measurement_consumer: rtrb::Consumer, + filter_control: FilterControl, + sample_rate: u32, + ) { + self.measurement_consumer = measurement_consumer; + self.filter_control = filter_control; + self.sample_rate = sample_rate; + self.reset(); + } } /// Coerce a possibly-non-finite LUFS measurement into a finite value diff --git a/crates/headroom-core/src/pw/command.rs b/crates/headroom-core/src/pw/command.rs index 93efde4..502b83b 100644 --- a/crates/headroom-core/src/pw/command.rs +++ b/crates/headroom-core/src/pw/command.rs @@ -61,4 +61,16 @@ pub enum PwCommand { /// effective profile, real sink) at apply time, not at post /// time, so a stale command is harmless. ReevaluateAll, + /// Rebuild the bus filter at a new sample rate. Posted when + /// the real sink's Format-param listener detects a rate that + /// doesn't match what the filter is currently running at — + /// either at cold boot (ALSA sinks only publish their rate + /// via Format, not in their props dict, so the initial filter + /// is created at the fallback rate before the Format event + /// fires) or on a sink hot-swap that changed the rate. + /// Causes a ~50–100 ms audio dropout during the swap. + RebuildFilter { + /// New filter sample rate in Hz. + sample_rate: u32, + }, } diff --git a/crates/headroom-core/src/pw/mod.rs b/crates/headroom-core/src/pw/mod.rs index 7e362db..efa69f6 100644 --- a/crates/headroom-core/src/pw/mod.rs +++ b/crates/headroom-core/src/pw/mod.rs @@ -157,6 +157,15 @@ impl PwContext { &self.core } + /// Borrow the routing state's `Rc>`, if + /// the routing engine has been started. Lets `runtime` install + /// the filter-rebuild handles after `start_routing` without + /// having to thread them through that method's signature. + #[must_use] + pub fn routing_state(&self) -> Option>> { + self.routing.borrow().as_ref().map(|w| w.state().clone()) + } + /// Create `headroom-processed` and do a roundtrip to confirm it /// landed on the server. /// diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index 61bce95..44db1f4 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -245,6 +245,18 @@ pub struct RoutingState { /// a real sink; replaced whenever `real_sink.node_id` /// changes, dropped on removal. real_sink_format_listener: Option<(u32, Node, NodeListener)>, + /// The bus filter (`Filter::create` output). Held here rather + /// than in `runtime` so `PwCommand::RebuildFilter` (issued by + /// the Format listener when the real sink's rate changes) can + /// swap the streams atomically. `None` only on cold-boot + /// before runtime calls `install_filter_rebuild_handles`. + bus_filter: Option, + /// Handle to the slow AGC controller so a rebuild can hand it + /// the new `measurement_consumer` + `filter_control` via + /// [`AgcController::rebind`]. The Rc is also held by the AGC + /// timer in `runtime`; only one of the two can borrow_mut at + /// once — the main loop serialises both. + agc_controller: Option>>, } /// Per-stream Layer A bundle: the tap (audio path), the controller @@ -313,9 +325,24 @@ impl RoutingState { default_reassertion: None, known_streams: HashMap::new(), real_sink_format_listener: None, + bus_filter: None, + agc_controller: None, } } + /// Take ownership of the bus filter + a handle to the slow AGC + /// controller so the registry thread can rebuild + rebind both + /// atomically when the real sink's rate changes. Called once + /// from `runtime` after the initial filter + AGC come up. + pub fn install_filter_rebuild_handles( + &mut self, + filter: crate::pw::filter::Filter, + agc: Rc>, + ) { + self.bus_filter = Some(filter); + self.agc_controller = Some(agc); + } + /// Drain any [`PwCommand`]s the IPC threads posted while we /// weren't looking, then run a pass of routing-link enforcement. /// Called by the 50 ms timer source installed in @@ -358,9 +385,96 @@ impl RoutingState { PwCommand::ReevaluateAll => { self.reevaluate_all(); } + PwCommand::RebuildFilter { sample_rate } => { + self.rebuild_filter(sample_rate); + } } } + /// Tear down + recreate the bus filter at `new_sample_rate`, + /// then rebind the slow AGC controller to the new measurement + /// ring + FilterControl. Posted by the Format listener when + /// it detects a real-sink rate that doesn't match what the + /// filter is currently running at. Causes a ~50–100 ms audio + /// gap on the processed path during the swap — acceptable on + /// a rate-change event since the user typically just plugged + /// a different DAC in. + fn rebuild_filter(&mut self, new_sample_rate: u32) { + let Some(agc) = self.agc_controller.clone() else { + tracing::warn!( + new_sample_rate, + "filter rebuild requested but agc handle not installed yet" + ); + return; + }; + let current_rate = self.daemon.lock().filter_sample_rate; + if current_rate == Some(new_sample_rate) { + tracing::debug!( + new_sample_rate, + "filter rebuild requested but rate is already current — no-op" + ); + return; + } + // Snapshot the DSP config from the active profile under + // the daemon lock; rebuild then runs against PipeWire + // without holding the lock. + let filter_init = { + let s = self.daemon.lock(); + let effective = s.profiles.effective(); + crate::pw::filter::FilterInit { + compressor: effective.build_compressor_config(), + limiter: effective.build_limiter_config(), + agc: headroom_dsp::AgcGainConfig::default(), + agc_enabled: effective.agc.enabled, + } + }; + tracing::info!( + old_rate = ?current_rate, + new_rate = new_sample_rate, + "rebuilding bus filter at new sample rate" + ); + // Drop the old filter BEFORE creating the new one so the + // streams come down cleanly and we don't briefly carry + // two copies. The user will hear a short silence here. + self.bus_filter = None; + let bundle = match crate::pw::filter::Filter::create( + &self.core, + filter_init, + new_sample_rate, + ) { + Ok(b) => b, + Err(e) => { + tracing::error!( + error = %e, + new_sample_rate, + "filter rebuild failed; daemon will run without a filter until the next rate change" + ); + return; + } + }; + // Update shared state under the lock. + { + let mut s = self.daemon.lock(); + s.filter_control = Some(bundle.control.clone()); + s.filter_sample_rate = Some(bundle.sample_rate); + } + // Rebind AGC to the new measurement ring + control. + agc.borrow_mut().rebind( + bundle.measurement_consumer, + bundle.control, + bundle.sample_rate, + ); + // Install the new filter; old is already dropped. + self.bus_filter = Some(bundle.filter); + // Existing managed-route links were anchored at ports on + // the *old* filter's processed-sink monitor / playback + // ports. Re-running the routing pass picks up the new + // processed sink's ports as they appear; any links whose + // target ports just disappeared get destroyed by the + // listener-driven `outbound_links_by_node` cleanup. + self.reevaluate_all(); + } + /// True iff the default metadata has been bound. #[must_use] pub fn has_default_metadata(&self) -> bool { @@ -648,17 +762,41 @@ impl RoutingState { let Some(rate) = extract_audio_rate(param) else { return; }; - let mut s = daemon.lock(); - if s.real_sink.sample_rate == Some(rate) { + let (need_rebuild, tx) = { + let mut s = daemon.lock(); + if s.real_sink.sample_rate == Some(rate) { + return; + } + tracing::info!( + node_id, + old_rate = ?s.real_sink.sample_rate, + new_rate = rate, + "real sink Format negotiated; updating sample_rate" + ); + s.real_sink.sample_rate = Some(rate); + // If the filter is running at a different rate + // (cold-boot fallback, or hot-swap), ask the + // registry thread to rebuild. + let need = s.filter_sample_rate != Some(rate); + (need, s.pw_command_tx.clone()) + }; + if !need_rebuild { return; } - tracing::info!( - node_id, - old_rate = ?s.real_sink.sample_rate, - new_rate = rate, - "real sink Format negotiated; updating sample_rate" - ); - s.real_sink.sample_rate = Some(rate); + let Some(tx) = tx else { + tracing::debug!( + "no PipeWire command channel; filter rebuild deferred (test mode?)" + ); + return; + }; + if tx + .send(PwCommand::RebuildFilter { sample_rate: rate }) + .is_err() + { + tracing::warn!( + "PipeWire command channel closed; filter rate-match lost" + ); + } }) .register(); node.subscribe_params(&[ParamType::Format]); diff --git a/crates/headroom-core/src/runtime.rs b/crates/headroom-core/src/runtime.rs index 0093c25..22d0721 100644 --- a/crates/headroom-core/src/runtime.rs +++ b/crates/headroom-core/src/runtime.rs @@ -117,7 +117,7 @@ pub fn run(profiles: ProfileStore) -> Result<(), DaemonError> { tracing::info!(initial_rate, "creating filter at real-sink-matched rate"); let FilterBundle { - filter: _filter, + filter, control: filter_control, measurement_consumer, bus_metrics, @@ -165,6 +165,25 @@ pub fn run(profiles: ProfileStore) -> Result<(), DaemonError> { // mechanism (see 4h). pw.start_routing(daemon_state.clone())?; + // Hand the filter + an AGC handle to the routing state so the + // Format-param listener (registered when the real sink resolves + // its negotiated audio.rate) can ask the registry thread to + // rebuild the filter at a new rate via + // `PwCommand::RebuildFilter`. Filter ownership moves here: + // RoutingState now drops it on daemon shutdown via PwContext's + // drop order. The Filter is `Some(filter)` here unconditionally + // — `install_filter_rebuild_handles` overwrites whatever's in + // the slot. + if let Some(routing_state) = pw.routing_state() { + routing_state + .borrow_mut() + .install_filter_rebuild_handles(filter, agc_controller.clone()); + } else { + // start_routing succeeded above so this branch shouldn't + // fire; keep the filter alive defensively if it ever does. + tracing::warn!("routing_state unavailable post-start_routing; keeping filter local"); + } + publish_daemon_started(&daemon_state, &pending_warnings, active_missing.as_deref()); pw.run_until_signal()?; From 4c39ecd5d20010cb14f9b9227b86869a626de252 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 21:12:23 +1000 Subject: [PATCH 20/21] fix: route.set matches application_name too, not just process_binary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `route.set ` overlay used to emit a single `RouteRule` keyed on `process_binary`. The matcher ANDs across non-empty fields, so a stream that didn't advertise `application.process.binary` would miss the rule even though its `application.name` was a perfect match. pw-cat is the canonical hit — it sets `application.name = "pw-cat"` and `node.name = "pw-cat"` but leaves `process.binary` unset entirely. The same goes for several Electron and Flatpak wrappers where the wrapping process eats the binary name. `apply_route_overrides` now emits TWO rules per override, one keyed on each identity field, with the same route. PipeWire iterates rules in order and returns on first match, so the effect is an OR across `process_binary` and `application_name` for the single override — exactly the "match by whatever name the stream advertises" intent of the CLI verb. Why two rules and not "loosen the matcher to OR these two fields": the matcher's AND-across-fields is load-bearing for profile-author rules like `{process_binary: ["firefox"], media_role: ["voice"]}` (match firefox-with-voice-role only). Loosening the matcher would silently break those. Two single-field rules with the same route preserve the original semantics and add zero risk. `is_single_app_rule_for_any` (the retain pre-pass that drops old override rules before re-emitting) extends to recognise the application_name-only variant too, so re-setting or unsetting an override leaves no residual rules. Tests - `profile_store::tests::set_route_emits_both_process_binary_and_application_name_rules` asserts both variants exist after `set_route`. - `profile_store::tests::set_route_then_unset_leaves_no_residual_rules` catches the matching retain-pre-pass regression that would have leaked rules on unset. - `routing::tests::application_name_only_rule_matches_stream_with_no_process_binary` proves a stream with `application.name = "pw-cat"` and no `process.binary` actually matches the application_name-keyed rule path. 194 tests pass (was 191; +3 for the new coverage); clippy clean. Live verification Daemon up, pw-cat → headroom-processed (default rule). `headroom route set pw-cat bypass`: pw-cat's link snaps to `Mbox:playback_FL` within one drain tick (~50 ms); status reports `route: bypass`. Layer A tap survives the transition intact. `headroom route unset pw-cat`: snaps back to `headroom-processed:playback_FL`. Both transitions are audibly clean against the F2 audio-gap mitigation from `5c769a1`. --- crates/headroom-core/src/profile_store.rs | 112 +++++++++++++++++++--- crates/headroom-core/src/routing.rs | 34 +++++++ 2 files changed, 135 insertions(+), 11 deletions(-) diff --git a/crates/headroom-core/src/profile_store.rs b/crates/headroom-core/src/profile_store.rs index df3c509..d8b5b7e 100644 --- a/crates/headroom-core/src/profile_store.rs +++ b/crates/headroom-core/src/profile_store.rs @@ -638,34 +638,76 @@ fn materialize_skipping( fn apply_route_overrides(profile: &mut Profile, overrides: &BTreeMap) { // Drop any existing single-app user rule matching an override, then - // prepend the overrides as one rule per app at the top of the list. + // prepend two rules per app so the match catches whichever + // identity field the stream actually advertises. + // + // Why two rules and not one with both fields set? The matcher + // ANDs across non-empty fields, so a rule with both + // `process_binary` *and* `application_name` populated would + // only match a stream that has *both* properties set to the + // same string. Many CLI tools (pw-cat being the canonical + // case, plus various Electron / Flatpak wrappers) only set + // `application.name` and leave `application.process.binary` + // unset — they'd miss the AND-shape rule despite the user's + // clear intent. + // + // Two single-field rules with the same route effectively form + // an OR across the identity fields. PipeWire iterates rules in + // order and returns on first match, so emitting both is cheap + // (constant per override) and correct in either case. let override_apps: std::collections::HashSet<&String> = overrides.keys().collect(); profile .rules .retain(|r| !is_single_app_rule_for_any(r, &override_apps)); - let mut new_rules: Vec = overrides - .iter() - .map(|(app, route)| RouteRule { + let mut new_rules: Vec = Vec::with_capacity(overrides.len() * 2); + for (app, route) in overrides { + new_rules.push(RouteRule { match_: RouteRuleMatch { process_binary: vec![app.clone()], ..Default::default() }, route: *route, - }) - .collect(); + }); + new_rules.push(RouteRule { + match_: RouteRuleMatch { + application_name: vec![app.clone()], + ..Default::default() + }, + route: *route, + }); + } new_rules.extend(std::mem::take(&mut profile.rules)); profile.rules = new_rules; } +/// True iff `rule` is one of the single-app override rules +/// generated by [`apply_route_overrides`] for *any* of `apps`. +/// Used by the retain pre-pass so re-applying an overlay doesn't +/// double-stack the per-app rules from a prior materialise. fn is_single_app_rule_for_any( rule: &RouteRule, apps: &std::collections::HashSet<&String>, ) -> bool { - rule.match_.process_binary.len() == 1 - && apps.contains(&rule.match_.process_binary[0]) - && rule.match_.application_name.is_empty() - && rule.match_.portal_app_id.is_empty() - && rule.match_.media_role.is_empty() + let m = &rule.match_; + if !m.portal_app_id.is_empty() || !m.media_role.is_empty() { + return false; + } + // process_binary-only single-app rule + if m.process_binary.len() == 1 + && apps.contains(&m.process_binary[0]) + && m.application_name.is_empty() + { + return true; + } + // application_name-only single-app rule (added 2026-05-21 so + // CLI tools that don't advertise `process.binary` match too) + if m.application_name.len() == 1 + && apps.contains(&m.application_name[0]) + && m.process_binary.is_empty() + { + return true; + } + false } // --------------------------------------------------------------------------- @@ -940,6 +982,54 @@ mod tests { assert_eq!(rule.route, Route::Bypass); } + #[test] + fn set_route_emits_both_process_binary_and_application_name_rules() { + // The route.set CLI verb accepts a single app identifier + // but streams can advertise themselves via either + // `application.process.binary` or `application.name` + // (or neither — those go through default_route). The + // overlay materialises BOTH single-field rules so each + // possible identity field is covered. + let (paths, _g) = tmp_paths(); + let mut s = ProfileStore::load(&paths).unwrap(); + s.set_route("pw-cat", Route::Bypass).unwrap(); + let rules = &s.effective().rules; + let proc_rule = rules + .iter() + .find(|r| r.match_.process_binary == vec!["pw-cat".to_string()]) + .expect("process_binary rule"); + assert_eq!(proc_rule.route, Route::Bypass); + assert!(proc_rule.match_.application_name.is_empty()); + let name_rule = rules + .iter() + .find(|r| r.match_.application_name == vec!["pw-cat".to_string()]) + .expect("application_name rule"); + assert_eq!(name_rule.route, Route::Bypass); + assert!(name_rule.match_.process_binary.is_empty()); + } + + #[test] + fn set_route_then_unset_leaves_no_residual_rules() { + // Both the process_binary and application_name variants + // of a single-app override must clear on unset; otherwise + // a re-add would stack rules and the matcher would carry + // dead entries indefinitely. + let (paths, _g) = tmp_paths(); + let mut s = ProfileStore::load(&paths).unwrap(); + s.set_route("pw-cat", Route::Bypass).unwrap(); + s.unset_route("pw-cat").unwrap(); + let residual: Vec<_> = s + .effective() + .rules + .iter() + .filter(|r| { + r.match_.process_binary == vec!["pw-cat".to_string()] + || r.match_.application_name == vec!["pw-cat".to_string()] + }) + .collect(); + assert!(residual.is_empty(), "leftover override rules: {residual:#?}"); + } + #[test] fn set_route_updates_existing_override() { let (paths, _g) = tmp_paths(); diff --git a/crates/headroom-core/src/routing.rs b/crates/headroom-core/src/routing.rs index a5956aa..99e0be3 100644 --- a/crates/headroom-core/src/routing.rs +++ b/crates/headroom-core/src/routing.rs @@ -185,6 +185,40 @@ mod tests { } } + #[test] + fn application_name_only_rule_matches_stream_with_no_process_binary() { + // The shape `route set` emits when expanded into an + // `application_name`-keyed override. Verifies that a + // stream missing `application.process.binary` (typical + // of pw-cat, many CLI tools, some Flatpak wrappers) is + // still matched by the user's intent. + use headroom_ipc::{RouteRule, RouteRuleMatch}; + let mut profile = Profile::default_v0(); + // Override at the top of the rule list. + profile.rules.insert( + 0, + RouteRule { + match_: RouteRuleMatch { + application_name: vec!["pw-cat".into()], + ..Default::default() + }, + route: Route::Bypass, + }, + ); + // Stream advertises only application.name = "pw-cat". + let info = PwNodeInfo { + node_id: 9, + media_class: Some("Stream/Output/Audio".into()), + application_process_binary: None, + application_name: Some("pw-cat".into()), + ..Default::default() + }; + assert_eq!( + evaluate(&info, &profile, false), + RoutingDecision::Route(Route::Bypass) + ); + } + #[test] fn matches_bypass_rule_for_known_music_player() { let info = playback("mpv"); From 716290c3bfcb114f744ab5db17921c192e2a014a Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 21:30:06 +1000 Subject: [PATCH 21/21] fix(Q5): drop retain pre-pass in apply_route_overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex audit of 4c39ecd flagged that the `retain` step would silently prune any base-profile rule whose single-field shape coincided with the overlay's emit pattern. `materialize` is already stateless — it reserialises the base profile fresh from `pick_base` on every call, so overlay rules can't accumulate across consecutive `set_route` calls. The retain bought nothing and risked dropping user-authored rules; prepending alone makes the overlay win first-match iteration. Adds a regression test that loads a user profile carrying a rule whose shape matches what the overlay emits, then sets an override for the same app and asserts the user rule survives alongside the prepended overlay rules. --- crates/headroom-core/src/profile_store.rs | 93 ++++++++++++++--------- 1 file changed, 56 insertions(+), 37 deletions(-) diff --git a/crates/headroom-core/src/profile_store.rs b/crates/headroom-core/src/profile_store.rs index d8b5b7e..2b6d0c1 100644 --- a/crates/headroom-core/src/profile_store.rs +++ b/crates/headroom-core/src/profile_store.rs @@ -637,9 +637,8 @@ fn materialize_skipping( } fn apply_route_overrides(profile: &mut Profile, overrides: &BTreeMap) { - // Drop any existing single-app user rule matching an override, then - // prepend two rules per app so the match catches whichever - // identity field the stream actually advertises. + // Prepend two rules per overlay entry so the match catches + // whichever identity field the stream actually advertises. // // Why two rules and not one with both fields set? The matcher // ANDs across non-empty fields, so a rule with both @@ -655,10 +654,18 @@ fn apply_route_overrides(profile: &mut Profile, overrides: &BTreeMap = overrides.keys().collect(); - profile - .rules - .retain(|r| !is_single_app_rule_for_any(r, &override_apps)); + // + // No retain pre-pass: `materialize` is stateless (it + // serializes the base profile fresh from `pick_base` each + // call), so overlay rules can't accumulate across consecutive + // `set_route` calls. A retain pre-pass would only deduplicate + // rules whose *base profile* TOML coincidentally has the same + // shape — silently removing a user-authored rule that was + // never an overlay artefact. The prepended order means + // overlay rules win first-match iteration over any genuinely + // duplicate base-profile rule anyway, so no correctness gain; + // dropping the retain closes the data-loss surface Codex + // flagged in its audit of the route.set match-by-name change. let mut new_rules: Vec = Vec::with_capacity(overrides.len() * 2); for (app, route) in overrides { new_rules.push(RouteRule { @@ -680,36 +687,6 @@ fn apply_route_overrides(profile: &mut Profile, overrides: &BTreeMap, -) -> bool { - let m = &rule.match_; - if !m.portal_app_id.is_empty() || !m.media_role.is_empty() { - return false; - } - // process_binary-only single-app rule - if m.process_binary.len() == 1 - && apps.contains(&m.process_binary[0]) - && m.application_name.is_empty() - { - return true; - } - // application_name-only single-app rule (added 2026-05-21 so - // CLI tools that don't advertise `process.binary` match too) - if m.application_name.len() == 1 - && apps.contains(&m.application_name[0]) - && m.process_binary.is_empty() - { - return true; - } - false -} - // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -1030,6 +1007,48 @@ mod tests { assert!(residual.is_empty(), "leftover override rules: {residual:#?}"); } + #[test] + fn user_rule_with_overlay_shape_survives_set_route_for_same_app() { + // Regression for Codex audit Q5: an earlier retain pre-pass in + // `apply_route_overrides` would silently drop any base-profile + // rule whose single-field shape coincided with the overlay's + // emit pattern. The fix is to delete the retain entirely — + // prepending already makes the overlay win first-match + // iteration, and removing the retain closes the data-loss + // surface. This test pins the surviving-rule behaviour so a + // future refactor can't quietly reintroduce the prune. + let (paths, _g) = tmp_paths(); + fs::write( + paths.config_dir.join("profiles/custom.toml"), + r#" +name = "custom" +description = "user custom" +default_route = { route = "processed" } +[[rules]] +match = { process_binary = ["obs"] } +route = "processed" +"#, + ) + .unwrap(); + let mut s = ProfileStore::load(&paths).unwrap(); + s.use_profile("custom").unwrap(); + // Sanity: user rule is loaded once. + assert_eq!(s.effective().rules.len(), 1); + + s.set_route("obs", Route::Bypass).unwrap(); + + let rules = &s.effective().rules; + // Two overlay rules (process_binary + application_name) plus + // the preserved user rule. + assert_eq!(rules.len(), 3, "rules: {rules:#?}"); + assert_eq!(rules[0].route, Route::Bypass); + assert_eq!(rules[0].match_.process_binary, vec!["obs".to_string()]); + assert_eq!(rules[1].route, Route::Bypass); + assert_eq!(rules[1].match_.application_name, vec!["obs".to_string()]); + assert_eq!(rules[2].route, Route::Processed); + assert_eq!(rules[2].match_.process_binary, vec!["obs".to_string()]); + } + #[test] fn set_route_updates_existing_override() { let (paths, _g) = tmp_paths();