audio/ownership: apply the merge rule to the ALSA carrier too
Verification round on round 10's own fixes, not on the next layer.
R10-5 preserved a user's PULSE_PROP and PIPEWIRE_PROPS but
tag_this_process_alsa_audio still clobbered their PIPEWIRE_ALSA, which is
the same kind of routing policy and deserves the same treatment. Both it
and tag_child now merge.
MEASURED, rather than assumed, because "our pairs go last so they win"
was load-bearing for the whole merge design and was never checked:
PIPEWIRE_PROPS='{ "node.name"="theirs_first", "media.role"="music",
"node.name"="ours_last" }' on pw-play
-> node.name=ours_last, media.role preserved.
The PULSE_PROP equivalent on paplay -> the same.
So last-wins holds on both grammars: a user who already sets node.name
cannot silently untag us, and their other keys survive.
That also makes tag_child's ALSA carrier merge from the inherited value
safely: in production main has already put this process's `clip` tag
there, and the child's own role now overrides it by coming last. The
existing row could not see this — the test binary never runs main, so it
only ever exercised the merge-into-nothing case. Added a row that drives
the real shape directly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+63
-18
@@ -173,16 +173,22 @@ fn pipewire_props_value(node_name: &str) -> String {
|
|||||||
/// ALSA output. Setting the ones the child ignores costs nothing.
|
/// ALSA output. Setting the ones the child ignores costs nothing.
|
||||||
///
|
///
|
||||||
/// An inherited value is **merged** rather than replaced (round 10, R10-5).
|
/// An inherited value is **merged** rather than replaced (round 10, R10-5).
|
||||||
/// Both variables can legitimately carry a user's own routing policy —
|
/// All three can legitimately carry a user's own routing policy —
|
||||||
/// `media.role`, a target sink — and clobbering it changes where the user's
|
/// `media.role`, a target sink — and clobbering it changes where the user's
|
||||||
/// audio goes as a side effect of a tagging mechanism that is supposed to be
|
/// audio goes as a side effect of a tagging mechanism that is supposed to be
|
||||||
/// behaviourally invisible. Our pairs go **last**, so they win any duplicate
|
/// behaviourally invisible. An inherited value that does not match the
|
||||||
/// key. An inherited value that does not match the expected shape is logged
|
/// expected shape is logged and overwritten: a half-merged string that fails
|
||||||
/// and overwritten: a half-merged string that fails to parse would drop the
|
/// to parse would drop the tag silently, which is the one outcome worse than
|
||||||
/// tag silently, which is the one outcome worse than losing the user's
|
/// losing the user's routing preference.
|
||||||
/// routing preference.
|
|
||||||
///
|
///
|
||||||
/// ⚠️ Reachability, measured 2026-07-25: neither variable is set anywhere in
|
/// ✅ **Our pairs go last, and last wins — measured 2026-07-25**, not assumed.
|
||||||
|
/// `PIPEWIRE_PROPS='{ "node.name" = "theirs_first", "media.role" = "music",
|
||||||
|
/// "node.name" = "ours_last" }'` on `pw-play` produced `node.name=ours_last`
|
||||||
|
/// with `media.role` preserved, and the `PULSE_PROP` equivalent on `paplay`
|
||||||
|
/// did the same. So a user who already sets `node.name` cannot silently
|
||||||
|
/// untag us, and their other keys survive.
|
||||||
|
///
|
||||||
|
/// ⚠️ Reachability, measured 2026-07-25: none of the three is set anywhere in
|
||||||
/// this user's environment or configuration, so this is a correctness
|
/// this user's environment or configuration, so this is a correctness
|
||||||
/// property with no live consumer today.
|
/// property with no live consumer today.
|
||||||
pub fn tag_child(command: &mut Command, role: &str) {
|
pub fn tag_child(command: &mut Command, role: &str) {
|
||||||
@@ -191,13 +197,23 @@ pub fn tag_child(command: &mut Command, role: &str) {
|
|||||||
PULSE_PROP_ENV,
|
PULSE_PROP_ENV,
|
||||||
merge_pulse_prop(inherited(PULSE_PROP_ENV).as_deref(), &node_name),
|
merge_pulse_prop(inherited(PULSE_PROP_ENV).as_deref(), &node_name),
|
||||||
);
|
);
|
||||||
let pipewire_props = merge_pipewire_props(inherited(PIPEWIRE_PROPS_ENV).as_deref(), &node_name);
|
command.env(
|
||||||
|
PIPEWIRE_PROPS_ENV,
|
||||||
|
merge_pipewire_props(inherited(PIPEWIRE_PROPS_ENV).as_deref(), &node_name),
|
||||||
|
);
|
||||||
// The ALSA carrier takes the same SPA-JSON grammar and is set here too, so
|
// The ALSA carrier takes the same SPA-JSON grammar and is set here too, so
|
||||||
// a child configured for ALSA output — which neither of the other two
|
// a child configured for ALSA output — which neither of the other two
|
||||||
// variables reaches — is tagged with *its own* role rather than inheriting
|
// variables reaches — is tagged at all, and with *its own* role.
|
||||||
// this process's `clip` tag from `tag_this_process_alsa_audio`.
|
//
|
||||||
command.env(PIPEWIRE_ALSA_ENV, &pipewire_props);
|
// Merged from the inherited `PIPEWIRE_ALSA`, which by now holds this
|
||||||
command.env(PIPEWIRE_PROPS_ENV, pipewire_props);
|
// process's own `clip` tag from [`tag_this_process_alsa_audio`] plus
|
||||||
|
// whatever the user set. The duplicate `node.name` that produces is
|
||||||
|
// resolved in our favour by the measured last-wins rule above, so the
|
||||||
|
// child gets its own role rather than inheriting `clip`.
|
||||||
|
command.env(
|
||||||
|
PIPEWIRE_ALSA_ENV,
|
||||||
|
merge_pipewire_props(inherited(PIPEWIRE_ALSA_ENV).as_deref(), &node_name),
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// This process's value for `key`, which is what a child would inherit.
|
/// This process's value for `key`, which is what a child would inherit.
|
||||||
@@ -301,18 +317,20 @@ fn merge_pipewire_props(inherited: Option<&str>, node_name: &str) -> String {
|
|||||||
/// of `main`, before anything is spawned; that is also correct on the merits,
|
/// of `main`, before anything is spawned; that is also correct on the merits,
|
||||||
/// since the plugin reads the variable when a stream is opened.
|
/// since the plugin reads the variable when a stream is opened.
|
||||||
///
|
///
|
||||||
/// Inherited values are **replaced**. Unlike the `Command` carriers (R10-5),
|
/// An inherited value is **merged**, on the same reasoning as [`tag_child`]'s
|
||||||
/// there is nothing to preserve: no user sets `PIPEWIRE_ALSA` to route
|
/// (R10-5): a user's `PIPEWIRE_ALSA` can carry routing policy, and this
|
||||||
/// peerspeak's own clip playback, and honouring one would defeat the tag.
|
/// function is not entitled to move their audio as a side effect of tagging
|
||||||
|
/// ours. Ours goes last and last wins (measured — see [`tag_child`]).
|
||||||
///
|
///
|
||||||
/// # Safety
|
/// # Safety
|
||||||
///
|
///
|
||||||
/// The caller must guarantee no other thread exists in this process.
|
/// The caller must guarantee no other thread exists in this process.
|
||||||
pub unsafe fn tag_this_process_alsa_audio() {
|
pub unsafe fn tag_this_process_alsa_audio() {
|
||||||
let node_name = owned_node_name(CLIP_ROLE);
|
let node_name = owned_node_name(CLIP_ROLE);
|
||||||
|
let value = merge_pipewire_props(inherited(PIPEWIRE_ALSA_ENV).as_deref(), &node_name);
|
||||||
// SAFETY: the caller's obligation, discharged by calling this at the top
|
// SAFETY: the caller's obligation, discharged by calling this at the top
|
||||||
// of `main` before any thread is spawned.
|
// of `main` before any thread is spawned.
|
||||||
unsafe { std::env::set_var(PIPEWIRE_ALSA_ENV, pipewire_props_value(&node_name)) };
|
unsafe { std::env::set_var(PIPEWIRE_ALSA_ENV, value) };
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Live-test support for the phase-1 exit gate, shared by the two modules
|
/// Live-test support for the phase-1 exit gate, shared by the two modules
|
||||||
@@ -717,12 +735,39 @@ mod tests {
|
|||||||
let alsa = env.get(PIPEWIRE_ALSA_ENV).expect("PIPEWIRE_ALSA set");
|
let alsa = env.get(PIPEWIRE_ALSA_ENV).expect("PIPEWIRE_ALSA set");
|
||||||
assert!(
|
assert!(
|
||||||
alsa.contains(&format!("\"node.name\" = \"{}\"", owned_node_name("mpv"))),
|
alsa.contains(&format!("\"node.name\" = \"{}\"", owned_node_name("mpv"))),
|
||||||
"the child's own role, not this process's clip role: {alsa}"
|
"the child's own role: {alsa}"
|
||||||
);
|
);
|
||||||
assert!(!alsa.contains(&format!("_{CLIP_ROLE}_")), "{alsa}");
|
|
||||||
assert!(alsa.contains(&format!("\"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\"")));
|
assert!(alsa.contains(&format!("\"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\"")));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// ⚠️ In production `main` has already set `PIPEWIRE_ALSA` to this
|
||||||
|
/// process's `clip` tag by the time a player is spawned, so `tag_child`
|
||||||
|
/// merges into *that*, not into nothing. The test binary has no such
|
||||||
|
/// value, so the row above never exercises the real shape — this one does,
|
||||||
|
/// on the merge function directly.
|
||||||
|
///
|
||||||
|
/// The child's `node.name` must come last, because last wins (measured):
|
||||||
|
/// otherwise a spawned mpv would report itself as `clip` and the audit
|
||||||
|
/// could not tell the incoming screenshare's audio from a chat clip.
|
||||||
|
#[test]
|
||||||
|
fn a_childs_role_overrides_the_inherited_clip_tag_in_the_alsa_carrier() {
|
||||||
|
let clip = pipewire_props_value(&owned_node_name(CLIP_ROLE));
|
||||||
|
let child = owned_node_name("mpv");
|
||||||
|
let merged = merge_pipewire_props(Some(&clip), &child);
|
||||||
|
|
||||||
|
let last_name = merged
|
||||||
|
.rfind("\"node.name\" = ")
|
||||||
|
.expect("a node.name pair in the merged value");
|
||||||
|
assert!(
|
||||||
|
merged[last_name..].contains(&child),
|
||||||
|
"the child's role must be the last node.name: {merged}"
|
||||||
|
);
|
||||||
|
// Both are still peerspeak-owned, so the duplicate is harmless.
|
||||||
|
assert!(merged.contains(&format!("\"{OWNED_PROP_KEY}\" = \"{OWNED_PROP_VALUE}\"")));
|
||||||
|
assert!(merged.starts_with('{') && merged.ends_with('}'), "{merged}");
|
||||||
|
assert_eq!(merged.matches('{').count(), 1, "one object only: {merged}");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn every_generated_name_matches_the_prefix_pixelpass_looks_for() {
|
fn every_generated_name_matches_the_prefix_pixelpass_looks_for() {
|
||||||
for role in ["call", "mpv", "vlc", "notify"] {
|
for role in ["call", "mpv", "vlc", "notify"] {
|
||||||
|
|||||||
Reference in New Issue
Block a user