diff --git a/src/host/audio.rs b/src/host/audio.rs index 8eaa342..b62229e 100644 --- a/src/host/audio.rs +++ b/src/host/audio.rs @@ -365,6 +365,9 @@ fn load_module(args: &[&str]) -> Result { .context("pactl returned non-UTF-8")? .trim() .to_string(); + // Genuinely 32-bit, unlike `object.serial`: this is a PulseAudio module + // index (`pa_module.index`, `uint32_t`), which `pactl unload-module` takes + // back verbatim. Do not widen it. id_str .parse::() .with_context(|| format!("pactl returned unexpected module ID: {id_str:?}")) @@ -532,13 +535,21 @@ fn run_router( return; }; if props.get("node.name") == Some(sink_name_owned.as_str()) { - if let Some(serial) = props - .get("object.serial") - .and_then(|s| s.parse::().ok()) - { - state_for_reg.borrow_mut().sink_serial = Some(serial); - tracing::info!(serial, "audio routing: pixelpass sink registered"); - try_flush(&state_for_reg, &event_tx_for_reg); + match props.get("object.serial").and_then(parse_object_serial) { + Some(serial) => { + state_for_reg.borrow_mut().sink_serial = Some(serial); + tracing::info!(serial, "audio routing: pixelpass sink registered"); + try_flush(&state_for_reg, &event_tx_for_reg); + } + // Never silently: without a serial `try_flush` can + // never route anything, so the whole app-filter mode + // is dead and the only symptom is missing audio. + None => tracing::warn!( + node_id = obj.id, + serial = props.get("object.serial").unwrap_or(""), + "audio routing: pixelpass sink has no usable object.serial; \ + stream rerouting disabled" + ), } return; } @@ -591,8 +602,29 @@ fn run_router( Ok(()) } +/// Parse a PipeWire `object.serial` property value. +/// +/// `object.serial` is a **64-bit** monotonically-increasing counter +/// (`pw_global`'s serial is `uint64_t`); it is *not* a `pw` object id +/// (those are `u32` and get recycled — the serial exists precisely so +/// that recycled ids can be disambiguated). Parsing it as `u32` silently +/// yields `None` past `u32::MAX`, which on a long-lived daemon means the +/// sink is never registered and no stream is ever routed. +/// +/// Strict on purpose: PipeWire emits a bare decimal, so anything else +/// (empty, signed, padded, non-numeric) is a property we do not +/// understand and must not guess at. +fn parse_object_serial(raw: &str) -> Option { + if raw.is_empty() || !raw.bytes().all(|b| b.is_ascii_digit()) { + return None; + } + raw.parse::().ok() +} + struct RouterState { - sink_serial: Option, + /// See [`parse_object_serial`] — 64-bit, and not interchangeable with + /// the `u32` node ids in `routed_node_ids` / `pending`. + sink_serial: Option, default_metadata: Option, routed_node_ids: Vec, pending: Vec, @@ -654,3 +686,62 @@ fn try_flush( let _ = event_tx.send(Event::FirstRoutedStream); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn object_serial_parses_past_u32() { + // The regression this fix exists for: a serial one past `u32::MAX` + // used to parse as `None` and silently disable rerouting. + let beyond = u64::from(u32::MAX) + 1; + assert_eq!(parse_object_serial(&beyond.to_string()), Some(beyond)); + assert_eq!( + parse_object_serial(&u64::MAX.to_string()), + Some(u64::MAX), + "the full 64-bit range must round-trip" + ); + } + + #[test] + fn object_serial_boundary_values() { + assert_eq!(parse_object_serial("0"), Some(0)); + assert_eq!(parse_object_serial("1"), Some(1)); + let max32 = u64::from(u32::MAX); + assert_eq!(parse_object_serial(&max32.to_string()), Some(max32)); + assert_eq!( + parse_object_serial(&(max32 - 1).to_string()), + Some(max32 - 1) + ); + } + + #[test] + fn object_serial_round_trips_through_the_metadata_string() { + // `try_flush` writes the serial back out as a decimal string for + // `target.object`; widening must not introduce a formatting change. + for raw in ["0", "4294967296", "18446744073709551615"] { + let parsed = parse_object_serial(raw).expect("valid serial"); + assert_eq!(parsed.to_string(), raw); + } + } + + #[test] + fn object_serial_rejects_malformed() { + for raw in [ + "", + " 12", + "12 ", + "+12", + "-1", + "1.0", + "0x10", + "12a", + "abc", + // u64::MAX + 1 — overflow must be rejected, not wrapped. + "18446744073709551616", + ] { + assert_eq!(parse_object_serial(raw), None, "should reject {raw:?}"); + } + } +}