From ff2533c95d9a15b5e9e815f0408a1ae6a0a04ad9 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Sat, 22 Aug 2026 02:41:31 -0400 Subject: [PATCH] fix: reap viewers and clear resolved share warnings --- src/app/mod.rs | 116 +++++++++++++++++++++++++++++++++-------- src/core/mod.rs | 20 +++++++ src/screenshare/mod.rs | 37 +++++++++++-- 3 files changed, 148 insertions(+), 25 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 60becc4..653d12e 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -1175,8 +1175,10 @@ pub struct AppState { /// Whether the current share is the desktop-excluding mode. This gates its /// status events so late notices cannot affect another share mode. share_desktop_excluding_active: bool, - /// Persistent explanation from PixelPass while desktop exclusion is active. - share_audio_exclusion_warning: Option, + /// The latest still-active warning from PixelPass while desktop exclusion + /// is active. Keeping the typed status preserves a stream serial so an + /// additive `stream_status_cleared` event can clear only its own warning. + share_audio_exclusion_warning: Option, /// Whether the resolved pixelpass supports `--strict-audio` (per-app audio). /// `false` ⇒ the picker offers whole-desktop only, because a per-app share /// would pass a flag an older pixelpass rejects (audit P2). Optimistic `true` @@ -2614,9 +2616,27 @@ fn update(state: &mut AppState, message: AppMessage) -> Task { ShareAudioSelection::DesktopExcluding ); if state.share_desktop_excluding_active || excluding_starting { - let message = audio_exclusion_status_message(&status); - state.status_message = message.clone(); - state.share_audio_exclusion_warning = Some(message); + match status { + AudioExclusionStatus::StreamStatusCleared { stream_serial } => { + let clears_visible = matches!( + state.share_audio_exclusion_warning.as_ref(), + Some(AudioExclusionStatus::StreamUnsupported { + stream_serial: visible_serial, + .. + }) if *visible_serial == stream_serial + ); + if clears_visible { + state.share_audio_exclusion_warning = None; + state.status_message = "Sharing your screen".to_string(); + } + } + warning => { + if let Some(message) = audio_exclusion_status_message(&warning) { + state.status_message = message; + state.share_audio_exclusion_warning = Some(warning); + } + } + } } } UiEvent::ClockSkewWarning { @@ -6723,7 +6743,11 @@ fn view(state: &AppState) -> Element<'_, AppMessage> { .color(color_yellow), ); } - if let Some(warning) = &state.share_audio_exclusion_warning { + if let Some(warning) = state + .share_audio_exclusion_warning + .as_ref() + .and_then(audio_exclusion_status_message) + { details = details.push(text(warning).size(11).color(color_yellow)); } details.into() @@ -8814,19 +8838,20 @@ fn with_pixelpass_help<'a>( .into() } -fn audio_exclusion_status_message(status: &AudioExclusionStatus) -> String { +fn audio_exclusion_status_message(status: &AudioExclusionStatus) -> Option { match status { - AudioExclusionStatus::StreamUnsupported { reason, .. } => format!( + AudioExclusionStatus::StreamUnsupported { reason, .. } => Some(format!( "Some system audio could not be shared safely ({reason}); it was left out." - ), + )), + AudioExclusionStatus::StreamStatusCleared { .. } => None, AudioExclusionStatus::AecFailed { .. } => - "Audio exclusion could not verify PeerSpeak's echo canceller; viewers hear silence." - .to_string(), + Some("Audio exclusion could not verify PeerSpeak's echo canceller; viewers hear silence." + .to_string()), AudioExclusionStatus::AecRevoked { .. } => - "Audio exclusion stopped because PeerSpeak's echo canceller disappeared; viewers hear silence." - .to_string(), + Some("Audio exclusion stopped because PeerSpeak's echo canceller disappeared; viewers hear silence." + .to_string()), AudioExclusionStatus::ForeignAecWarning { .. } => - "Another echo-cancel stream was kept out of the screen share.".to_string(), + Some("Another echo-cancel stream was kept out of the screen share.".to_string()), } } @@ -9760,11 +9785,12 @@ mod tests { use super::{ AppConfig, AppMessage, AppState, AttachmentCache, AttachmentState, AudioExclusionStatus, CLOCK_SKEW_WARNING_VISIBLE_SECS, ChatEntry, ClockSkewBanner, GateMeter, METER_MAX, Screen, - ScreenBounds, ShareAudioSelection, UiEvent, attachment_default_name, clamp_window_position, - clear_expired_clock_skew_warning, format_clock_skew_duration, format_duration, - format_relative_ago, friend_presence_notification, initial_window_position, - now_playing_label, reconnect_attempt_chime, reconnected_chime, selected_wav_path, - set_peer_gate_config, set_peer_volume_config, show_clock_skew_warning, update, + ScreenBounds, ShareAudioSelection, UiEvent, attachment_default_name, + audio_exclusion_status_message, clamp_window_position, clear_expired_clock_skew_warning, + format_clock_skew_duration, format_duration, format_relative_ago, + friend_presence_notification, initial_window_position, now_playing_label, + reconnect_attempt_chime, reconnected_chime, selected_wav_path, set_peer_gate_config, + set_peer_volume_config, show_clock_skew_warning, update, }; use iroh::SecretKey; use std::collections::VecDeque; @@ -10065,7 +10091,8 @@ mod tests { state.share_audio_dropped = true; state.share_audio_app_active = true; state.share_desktop_excluding_active = true; - state.share_audio_exclusion_warning = Some("warning".to_string()); + state.share_audio_exclusion_warning = + Some(AudioExclusionStatus::AecFailed { module_index: 77 }); state.share_app_audio_supported = false; state.share_desktop_audio_exclusion_supported = true; state.clock_skew_warning = Some(ClockSkewBanner { @@ -10436,7 +10463,8 @@ mod tests { assert!( state .share_audio_exclusion_warning - .as_deref() + .as_ref() + .and_then(audio_exclusion_status_message) .is_some_and(|message| message.contains("viewers hear silence")) ); @@ -10464,6 +10492,52 @@ mod tests { assert!(state.share_audio_exclusion_warning.is_none()); } + #[test] + fn desktop_exclusion_clears_only_the_matching_stream_warning() { + let mut state = AppState { + self_sharing: true, + share_desktop_excluding_active: true, + ..Default::default() + }; + + let _ = update( + &mut state, + AppMessage::UiEventReceived(UiEvent::ShareAudioExclusionStatus( + AudioExclusionStatus::StreamUnsupported { + stream_serial: 41, + reason: "unidentified-channel".to_string(), + }, + )), + ); + assert!(matches!( + state.share_audio_exclusion_warning.as_ref(), + Some(AudioExclusionStatus::StreamUnsupported { + stream_serial: 41, + .. + }) + )); + + let _ = update( + &mut state, + AppMessage::UiEventReceived(UiEvent::ShareAudioExclusionStatus( + AudioExclusionStatus::StreamStatusCleared { stream_serial: 99 }, + )), + ); + assert!( + state.share_audio_exclusion_warning.is_some(), + "another stream's recovery must not clear the visible warning" + ); + + let _ = update( + &mut state, + AppMessage::UiEventReceived(UiEvent::ShareAudioExclusionStatus( + AudioExclusionStatus::StreamStatusCleared { stream_serial: 41 }, + )), + ); + assert!(state.share_audio_exclusion_warning.is_none()); + assert_eq!(state.status_message, "Sharing your screen"); + } + #[test] fn old_pixelpass_picker_offers_whole_desktop_only() { // P2 (version skew): when the resolved pixelpass lacks --strict-audio, the diff --git a/src/core/mod.rs b/src/core/mod.rs index 8fa1e02..3c56240 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -1241,6 +1241,11 @@ const PING_INTERVAL: Duration = Duration::from_secs(15); /// Delay before the FIRST presence pass, so the endpoint's background `online()` /// has a moment to finish (otherwise the first probes fail and friends flash offline). const PING_STARTUP_DELAY: Duration = Duration::from_secs(3); +/// How quickly the core polls owned PixelPass viewer children for natural exit. +/// A viewer normally exits when the remote host stops sharing; without this +/// independent tick it remains an unreaped zombie until another Watch click or +/// the entire call ends. +const VIEWER_REAP_INTERVAL: Duration = Duration::from_millis(500); /// One outbound presence-refresh pass (W7 B2): probe every friend and emit a /// *definitive* status for each, so the UI self-heals every pass instead of only @@ -1565,6 +1570,11 @@ async fn run_core_loop( PING_INTERVAL, ); ping_interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); + let mut viewer_reap_interval = tokio::time::interval_at( + tokio::time::Instant::now() + VIEWER_REAP_INTERVAL, + VIEWER_REAP_INTERVAL, + ); + viewer_reap_interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); loop { let cmd = tokio::select! { biased; @@ -1611,6 +1621,12 @@ async fn run_core_loop( Some(generation) = host_fault_rx.recv() => { CoreCommand::ScreenShareHostFault { generation } } + _ = viewer_reap_interval.tick() => { + if let Some(session) = &mut active_session { + session.teardown.sweep_exited_viewers(); + } + continue; + } game_change = next_game_change(&mut game_rx) => { // The detector worker published a new debounced game (or `None`). let Some(detected) = game_change else { @@ -3801,6 +3817,7 @@ mod tests { let lines = [ r#"{"event":"stream_unsupported","version":1,"stream_serial":4294967303,"reason":"port-exclusive"}"#, + r#"{"event":"stream_status_cleared","version":1,"stream_serial":4294967303}"#, r#"{"event":"aec_failed","version":1,"module_index":536870919}"#, r#"{"event":"aec_revoked","version":1,"module_index":536870919}"#, r#"{"event":"foreign_aec_warning","version":1,"link_group":"echo-cancel-9999-13"}"#, @@ -3833,6 +3850,9 @@ mod tests { stream_serial: 4_294_967_303, reason: "port-exclusive".to_string(), }, + AudioExclusionStatus::StreamStatusCleared { + stream_serial: 4_294_967_303, + }, AudioExclusionStatus::AecFailed { module_index: 536_870_919, }, diff --git a/src/screenshare/mod.rs b/src/screenshare/mod.rs index 4f9620c..98f109a 100644 --- a/src/screenshare/mod.rs +++ b/src/screenshare/mod.rs @@ -82,10 +82,24 @@ pub enum ShareAudioSelection { /// define a second matching enum that could silently drift from the wire. #[derive(Debug, Clone, PartialEq, Eq)] pub enum AudioExclusionStatus { - StreamUnsupported { stream_serial: u64, reason: String }, - AecFailed { module_index: u64 }, - AecRevoked { module_index: u64 }, - ForeignAecWarning { link_group: String }, + StreamUnsupported { + stream_serial: u64, + reason: String, + }, + /// PixelPass has cleared the previously reported status for this exact + /// stream because it became capturable or left the graph. + StreamStatusCleared { + stream_serial: u64, + }, + AecFailed { + module_index: u64, + }, + AecRevoked { + module_index: u64, + }, + ForeignAecWarning { + link_group: String, + }, } /// One parsed line from pixelpass's `--output json` stdout stream. Mirrors the @@ -177,6 +191,11 @@ pub fn parse_pixelpass_event(line: &str) -> Option { reason: v.get("reason")?.as_str()?.to_string(), }) } + "stream_status_cleared" if json_u64(&v, "version") == Some(1) => { + PixelpassEvent::AudioExclusion(AudioExclusionStatus::StreamStatusCleared { + stream_serial: json_u64(&v, "stream_serial")?, + }) + } "aec_failed" if json_u64(&v, "version") == Some(1) => { PixelpassEvent::AudioExclusion(AudioExclusionStatus::AecFailed { module_index: json_u64(&v, "module_index")?, @@ -1679,6 +1698,16 @@ Install hint: sudo apt install gstreamer1.0-plugins-bad } )) ); + assert_eq!( + parse_pixelpass_event( + r#"{"event":"stream_status_cleared","version":1,"stream_serial":4294967303}"#, + ), + Some(PixelpassEvent::AudioExclusion( + AudioExclusionStatus::StreamStatusCleared { + stream_serial: 4_294_967_303, + } + )) + ); assert_eq!( parse_pixelpass_event(r#"{"event":"aec_failed","version":1,"module_index":536870919}"#,), Some(PixelpassEvent::AudioExclusion(