F3: force-bypass surround streams; generalise N-channel pairing
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<u32>`, 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.
This commit is contained in:
parent
03edb17180
commit
3427ec56fc
3 changed files with 78 additions and 6 deletions
13
PLAN.md
13
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.
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -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::<u32>().ok()),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<u32>,
|
||||
}
|
||||
|
||||
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");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue