From 8af6dff98d042435a59c5d14a10b4e508bd5b298 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 15:58:18 +1000 Subject: [PATCH] 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(