feat(screenshare): pass --strict-audio + surface app_audio drop warning (A23 P2)
Consumes the new pixelpass --strict-audio mode + app_audio events (pixelpass
85fdebe) to close Codex's A23 P2: the per-app pick alone was best-effort —
pixelpass would fall back to whole-desktop audio before the app routed and again
if it stopped, both reintroducing the call echo.
- host_args appends --strict-audio alongside --app=<name>, so pixelpass never
mirrors the desktop for our share: viewers hear only the chosen app or silence.
- Parse the app_audio JSON event (routed/lost) into PixelpassEvent; spawn_host
takes an optional notices channel and the stdout drain forwards events on it.
- Core spawns a forwarder (only when an app is selected) mapping routed/lost to
UiEvent::ShareAudioActive(bool); the channel/task self-terminate on host EOF.
- App tracks share_audio_dropped and shows a transient warning under the "Sharing
your screen" badge when the chosen app's audio stops ("viewers hear silence
until it plays again"). Reset on start/stop/room-leave.
In-process mpsc only — no wire/GOSSIP_PROTO change. 435 lib tests (+2: app_audio
parse + the dropped-flag state machine), clippy --all-targets + release clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+75
-4
@@ -588,6 +588,11 @@ pub struct AppState {
|
||||
/// the picker (and re-confirming) during that startup window. Cleared on
|
||||
/// `ScreenShareStarted`, `ScreenShareStopped`, or any `Error`.
|
||||
share_starting: bool,
|
||||
/// While sharing a specific app's audio (A23 strict mode): `true` when that
|
||||
/// app's audio has stopped, so viewers currently hear silence. Drives a
|
||||
/// transient warning. Always `false` for whole-desktop shares (pixelpass
|
||||
/// emits no `app_audio` events then) and when not sharing.
|
||||
share_audio_dropped: bool,
|
||||
/// Whether the Chat drawer is open (drawer layout only).
|
||||
drawer_chat_open: bool,
|
||||
/// Raw mic level (normalized RMS, `0.0..=1.0`) for the settings meter.
|
||||
@@ -661,6 +666,7 @@ impl AppState {
|
||||
self.share_audio_apps.clear();
|
||||
self.share_audio_selection = None;
|
||||
self.share_starting = false;
|
||||
self.share_audio_dropped = false;
|
||||
}
|
||||
|
||||
fn custom_sound_path(&self, sound: Sound) -> &str {
|
||||
@@ -786,6 +792,7 @@ impl Default for AppState {
|
||||
share_audio_apps: Vec::new(),
|
||||
share_audio_selection: None,
|
||||
share_starting: false,
|
||||
share_audio_dropped: false,
|
||||
drawer_chat_open: false,
|
||||
mic_level: 0.0,
|
||||
mic_test_active: false,
|
||||
@@ -1375,6 +1382,7 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
|
||||
UiEvent::ScreenShareStarted => {
|
||||
state.self_sharing = true;
|
||||
state.share_starting = false;
|
||||
state.share_audio_dropped = false;
|
||||
// Defensive: ensure no picker lingers across a successful start.
|
||||
state.share_picker_open = false;
|
||||
state.status_message = "Sharing your screen".to_string();
|
||||
@@ -1382,8 +1390,13 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
|
||||
UiEvent::ScreenShareStopped => {
|
||||
state.self_sharing = false;
|
||||
state.share_starting = false;
|
||||
state.share_audio_dropped = false;
|
||||
state.status_message = "Screen share stopped".to_string();
|
||||
}
|
||||
UiEvent::ShareAudioActive(active) => {
|
||||
// Per-app audio routed/lost (only while sharing a chosen app).
|
||||
state.share_audio_dropped = !active;
|
||||
}
|
||||
UiEvent::IdentityStatus { node_id, persisted, error } => {
|
||||
state.self_node_id = Some(node_id);
|
||||
state.identity_persisted = persisted;
|
||||
@@ -4069,16 +4082,32 @@ fn view(state: &AppState) -> Element<'_, AppMessage> {
|
||||
};
|
||||
el
|
||||
},
|
||||
// Live "you're sharing" badge — only present while sharing.
|
||||
// Live "you're sharing" badge — only present while sharing. When
|
||||
// sharing a specific app whose audio has dropped (A23 strict
|
||||
// mode), a warning line is added: viewers hear silence, not the
|
||||
// call, until that app plays again.
|
||||
{
|
||||
let el: Element<'_, AppMessage> = if state.self_sharing {
|
||||
row![
|
||||
let badge = row![
|
||||
icon(IconKind::Live, 14.0, color_red),
|
||||
text("Sharing your screen").size(13).color(color_red),
|
||||
]
|
||||
.spacing(6)
|
||||
.align_y(iced::alignment::Vertical::Center)
|
||||
.into()
|
||||
.align_y(iced::alignment::Vertical::Center);
|
||||
if state.share_audio_dropped {
|
||||
column![
|
||||
badge,
|
||||
text(
|
||||
"⚠ Shared app's audio stopped — viewers hear silence until it plays again"
|
||||
)
|
||||
.size(11)
|
||||
.color(color_yellow),
|
||||
]
|
||||
.spacing(3)
|
||||
.into()
|
||||
} else {
|
||||
badge.into()
|
||||
}
|
||||
} else {
|
||||
iced::widget::Space::new().width(0.0).height(0.0).into()
|
||||
};
|
||||
@@ -6317,6 +6346,7 @@ mod tests {
|
||||
state.share_audio_apps = vec!["Firefox".to_string()];
|
||||
state.share_audio_selection = Some("Firefox".to_string());
|
||||
state.share_starting = true;
|
||||
state.share_audio_dropped = true;
|
||||
state.clip_status.lock().unwrap().playing_id = Some(attachment_id);
|
||||
|
||||
state.reset_room_state();
|
||||
@@ -6341,6 +6371,7 @@ mod tests {
|
||||
assert!(state.share_audio_apps.is_empty());
|
||||
assert!(state.share_audio_selection.is_none());
|
||||
assert!(!state.share_starting);
|
||||
assert!(!state.share_audio_dropped);
|
||||
|
||||
for _ in 0..50 {
|
||||
if crate::audio::clip_player::status_snapshot(&state.clip_status).playing_id.is_none() {
|
||||
@@ -6409,6 +6440,46 @@ mod tests {
|
||||
assert!(state.share_picker_open);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn share_audio_dropped_tracks_app_audio_events() {
|
||||
// While sharing a specific app, app_audio lost/routed toggles the warning
|
||||
// flag; start and stop both reset it so it can't linger across sessions.
|
||||
let mut state = AppState::default();
|
||||
|
||||
// Start sharing — flag is clear.
|
||||
let _ = update(
|
||||
&mut state,
|
||||
AppMessage::UiEventReceived(UiEvent::ScreenShareStarted),
|
||||
);
|
||||
assert!(!state.share_audio_dropped);
|
||||
|
||||
// The chosen app's audio stops → warning on.
|
||||
let _ = update(
|
||||
&mut state,
|
||||
AppMessage::UiEventReceived(UiEvent::ShareAudioActive(false)),
|
||||
);
|
||||
assert!(state.share_audio_dropped);
|
||||
|
||||
// It plays again → warning off.
|
||||
let _ = update(
|
||||
&mut state,
|
||||
AppMessage::UiEventReceived(UiEvent::ShareAudioActive(true)),
|
||||
);
|
||||
assert!(!state.share_audio_dropped);
|
||||
|
||||
// Drop again, then stop sharing → flag reset regardless.
|
||||
let _ = update(
|
||||
&mut state,
|
||||
AppMessage::UiEventReceived(UiEvent::ShareAudioActive(false)),
|
||||
);
|
||||
assert!(state.share_audio_dropped);
|
||||
let _ = update(
|
||||
&mut state,
|
||||
AppMessage::UiEventReceived(UiEvent::ScreenShareStopped),
|
||||
);
|
||||
assert!(!state.share_audio_dropped);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn peer_gate_persists_when_on_and_clears_when_off() {
|
||||
let mut config = AppConfig::default();
|
||||
|
||||
@@ -155,6 +155,11 @@ pub enum UiEvent {
|
||||
ScreenShareStarted,
|
||||
/// Our own screen share stopped (or failed to start).
|
||||
ScreenShareStopped,
|
||||
/// Per-app screen-share audio routing state (A23). `true` = the app we chose
|
||||
/// is now reaching viewers; `false` = its audio stopped, so under our strict
|
||||
/// run viewers currently hear silence. The UI shows a transient warning while
|
||||
/// `false`. Only meaningful while sharing a specific app (not whole-desktop).
|
||||
ShareAudioActive(bool),
|
||||
/// Our node identity (W7): the current node id string, and whether it is
|
||||
/// PERSISTED to disk. Sent once at startup and again after a regenerate.
|
||||
/// `persisted = false` means the key file couldn't be read/written and we're
|
||||
|
||||
+27
-1
@@ -2660,7 +2660,33 @@ async fn run_core_loop(
|
||||
continue;
|
||||
}
|
||||
};
|
||||
match crate::screenshare::spawn_host(&bin, audio_app.as_deref()).await {
|
||||
// Forward pixelpass `app_audio` events (only emitted when an app
|
||||
// is selected) to the UI so it can warn when the chosen app's
|
||||
// audio drops. The channel closes when the host dies (drain hits
|
||||
// EOF), ending the forwarder task on its own.
|
||||
let notices = audio_app.as_deref().map(|_| {
|
||||
let (tx, mut rx) =
|
||||
tokio::sync::mpsc::unbounded_channel::<crate::screenshare::PixelpassEvent>();
|
||||
let ui_tx_notices = ui_tx.clone();
|
||||
tokio::spawn(async move {
|
||||
while let Some(ev) = rx.recv().await {
|
||||
let active = match ev {
|
||||
crate::screenshare::PixelpassEvent::AppAudioRouted => true,
|
||||
crate::screenshare::PixelpassEvent::AppAudioLost => false,
|
||||
_ => continue,
|
||||
};
|
||||
if ui_tx_notices
|
||||
.send(UiEvent::ShareAudioActive(active))
|
||||
.await
|
||||
.is_err()
|
||||
{
|
||||
break;
|
||||
}
|
||||
}
|
||||
});
|
||||
tx
|
||||
});
|
||||
match crate::screenshare::spawn_host(&bin, audio_app.as_deref(), notices).await {
|
||||
Ok((child, ticket)) => {
|
||||
crate::log_msg("Screen share host started");
|
||||
session.screenshare_host = Some(child);
|
||||
|
||||
+74
-16
@@ -68,6 +68,12 @@ pub enum PixelpassEvent {
|
||||
CaptureStarted,
|
||||
/// Host: capture pipeline torn down (on last viewer).
|
||||
CaptureStopped,
|
||||
/// Host (per-app audio): the chosen app's audio is now reaching viewers.
|
||||
AppAudioRouted,
|
||||
/// Host (per-app audio): the chosen app's last audio stream went away. Under
|
||||
/// our `--strict-audio` run this means viewers now hear silence (not the call
|
||||
/// echo) until the app produces audio again — we surface it as a warning.
|
||||
AppAudioLost,
|
||||
/// A recognized event we don't act on (e.g. `host_info`).
|
||||
Other,
|
||||
}
|
||||
@@ -102,6 +108,11 @@ pub fn parse_pixelpass_event(line: &str) -> Option<PixelpassEvent> {
|
||||
Some("stopped") => PixelpassEvent::CaptureStopped,
|
||||
_ => PixelpassEvent::Other,
|
||||
},
|
||||
"app_audio" => match v.get("state").and_then(|s| s.as_str()) {
|
||||
Some("routed") => PixelpassEvent::AppAudioRouted,
|
||||
Some("lost") => PixelpassEvent::AppAudioLost,
|
||||
_ => PixelpassEvent::Other,
|
||||
},
|
||||
_ => PixelpassEvent::Other,
|
||||
};
|
||||
Some(ev)
|
||||
@@ -112,9 +123,16 @@ fn json_u32(v: &serde_json::Value, key: &str) -> u32 {
|
||||
}
|
||||
|
||||
/// Build the argv for a pixelpass *host*. Always `--host --output json`; when
|
||||
/// `audio_app` is `Some`, append `--app=<name>` so pixelpass captures only that
|
||||
/// app's audio instead of the whole desktop sink monitor (which contains our own
|
||||
/// call playout → the viewer would hear themselves echoed back, backlog A23).
|
||||
/// `audio_app` is `Some`, append `--app=<name> --strict-audio` so pixelpass
|
||||
/// captures only that app's audio instead of the whole desktop sink monitor
|
||||
/// (which contains our own call playout → the viewer would hear themselves
|
||||
/// echoed back, backlog A23).
|
||||
///
|
||||
/// `--strict-audio` is what makes the fix a guarantee rather than best-effort:
|
||||
/// without it, pixelpass falls back to the whole-desktop loopback before the
|
||||
/// app's first stream routes and again if the app's audio later stops — both of
|
||||
/// which reintroduce the echo. With it, the viewer hears only the chosen app (or
|
||||
/// silence), and pixelpass emits `app_audio` events we surface as a warning.
|
||||
///
|
||||
/// The name is passed in the single-token `--app=<name>` form so a value that
|
||||
/// happens to begin with `-` can never be reparsed as a pixelpass flag (clap
|
||||
@@ -129,6 +147,7 @@ pub fn host_args(audio_app: Option<&str>) -> Vec<String> {
|
||||
];
|
||||
if let Some(name) = audio_app.and_then(sanitize_app_name) {
|
||||
args.push(format!("--app={name}"));
|
||||
args.push("--strict-audio".to_string());
|
||||
}
|
||||
args
|
||||
}
|
||||
@@ -265,7 +284,11 @@ pub fn is_available(config_override: Option<&str>) -> bool {
|
||||
/// drained in a background task so a full pipe can't stall the host. We do
|
||||
/// **not** pass `--max-viewers`: pixelpass bandwidth-measures its own safe cap,
|
||||
/// protecting the sharer's uplink, and refuses extras with `viewer_refused`.
|
||||
pub async fn spawn_host(bin: &Path, audio_app: Option<&str>) -> std::io::Result<(Child, String)> {
|
||||
pub async fn spawn_host(
|
||||
bin: &Path,
|
||||
audio_app: Option<&str>,
|
||||
notices: Option<tokio::sync::mpsc::UnboundedSender<PixelpassEvent>>,
|
||||
) -> std::io::Result<(Child, String)> {
|
||||
let mut child = Command::new(bin)
|
||||
.args(host_args(audio_app))
|
||||
.stdin(Stdio::null())
|
||||
@@ -299,7 +322,7 @@ pub async fn spawn_host(bin: &Path, audio_app: Option<&str>) -> std::io::Result<
|
||||
}
|
||||
};
|
||||
|
||||
drain_in_background(lines, "host");
|
||||
drain_in_background(lines, "host", notices);
|
||||
Ok((child, ticket))
|
||||
}
|
||||
|
||||
@@ -346,7 +369,7 @@ pub async fn spawn_viewer(bin: &Path, ticket: &str) -> std::io::Result<Child> {
|
||||
return Err(e);
|
||||
}
|
||||
|
||||
drain_in_background(lines, "viewer");
|
||||
drain_in_background(lines, "viewer", None);
|
||||
Ok(child)
|
||||
}
|
||||
|
||||
@@ -381,15 +404,24 @@ where
|
||||
}
|
||||
|
||||
/// Keep reading the child's stdout to EOF in the background so a full pipe can't
|
||||
/// stall it; log notable events for diagnostics.
|
||||
fn drain_in_background<R>(mut lines: tokio::io::Lines<BufReader<R>>, role: &'static str)
|
||||
where
|
||||
/// stall it; log notable events for diagnostics. When `notices` is `Some`, each
|
||||
/// parsed event is also forwarded to the caller (the core, which translates the
|
||||
/// `app_audio` ones into a UI warning); a send failure (receiver dropped) just
|
||||
/// stops forwarding, draining continues. The task ends on EOF (child exited).
|
||||
fn drain_in_background<R>(
|
||||
mut lines: tokio::io::Lines<BufReader<R>>,
|
||||
role: &'static str,
|
||||
notices: Option<tokio::sync::mpsc::UnboundedSender<PixelpassEvent>>,
|
||||
) where
|
||||
R: tokio::io::AsyncRead + Unpin + Send + 'static,
|
||||
{
|
||||
tokio::spawn(async move {
|
||||
while let Ok(Some(line)) = lines.next_line().await {
|
||||
if let Some(ev) = parse_pixelpass_event(&line) {
|
||||
crate::log_msg(&format!("pixelpass {role}: {}", event_for_log(&ev)));
|
||||
if let Some(tx) = ¬ices {
|
||||
let _ = tx.send(ev);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
@@ -408,6 +440,8 @@ fn event_for_log(ev: &PixelpassEvent) -> String {
|
||||
PixelpassEvent::Refused(reason) => format!("viewer_refused reason={reason:?}"),
|
||||
PixelpassEvent::CaptureStarted => "capture_started".to_string(),
|
||||
PixelpassEvent::CaptureStopped => "capture_stopped".to_string(),
|
||||
PixelpassEvent::AppAudioRouted => "app_audio_routed".to_string(),
|
||||
PixelpassEvent::AppAudioLost => "app_audio_lost".to_string(),
|
||||
PixelpassEvent::Other => "other".to_string(),
|
||||
}
|
||||
}
|
||||
@@ -489,16 +523,17 @@ mod tests {
|
||||
#[test]
|
||||
fn host_args_with_app_appends_single_token_flag() {
|
||||
// The chosen app rides in the `--app=<name>` single-token form so a
|
||||
// name beginning with `-` can never be reparsed as a flag (A23).
|
||||
// name beginning with `-` can never be reparsed as a flag (A23), plus
|
||||
// `--strict-audio` so pixelpass never falls back to whole-desktop audio.
|
||||
assert_eq!(
|
||||
host_args(Some("Firefox")),
|
||||
vec!["--host", "--output", "json", "--app=Firefox"]
|
||||
);
|
||||
assert_eq!(
|
||||
host_args(Some("-rm -rf")).last().unwrap(),
|
||||
"--app=-rm -rf",
|
||||
"hyphen-leading name stays a single token bound to --app"
|
||||
vec!["--host", "--output", "json", "--app=Firefox", "--strict-audio"]
|
||||
);
|
||||
// The hyphen-leading name is still bound to --app as a single token;
|
||||
// --strict-audio is the trailing flag.
|
||||
let args = host_args(Some("-rm -rf"));
|
||||
assert_eq!(args[3], "--app=-rm -rf");
|
||||
assert_eq!(args[4], "--strict-audio");
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -645,6 +680,29 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_app_audio_states() {
|
||||
// The wire contract from pixelpass's --strict-audio run (A23): routed =
|
||||
// the chosen app's audio is live; lost = it stopped (viewers now silent).
|
||||
assert_eq!(
|
||||
parse_pixelpass_event(r#"{"event":"app_audio","state":"routed"}"#),
|
||||
Some(PixelpassEvent::AppAudioRouted)
|
||||
);
|
||||
assert_eq!(
|
||||
parse_pixelpass_event(r#"{"event":"app_audio","state":"lost"}"#),
|
||||
Some(PixelpassEvent::AppAudioLost)
|
||||
);
|
||||
// Unknown / missing state is recognized-but-unused, not a parse failure.
|
||||
assert_eq!(
|
||||
parse_pixelpass_event(r#"{"event":"app_audio","state":"weird"}"#),
|
||||
Some(PixelpassEvent::Other)
|
||||
);
|
||||
assert_eq!(
|
||||
parse_pixelpass_event(r#"{"event":"app_audio"}"#),
|
||||
Some(PixelpassEvent::Other)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recognized_but_unused_event_is_other() {
|
||||
assert_eq!(
|
||||
|
||||
Reference in New Issue
Block a user