fix: reap viewers and clear resolved share warnings
This commit is contained in:
+95
-21
@@ -1175,8 +1175,10 @@ pub struct AppState {
|
|||||||
/// Whether the current share is the desktop-excluding mode. This gates its
|
/// Whether the current share is the desktop-excluding mode. This gates its
|
||||||
/// status events so late notices cannot affect another share mode.
|
/// status events so late notices cannot affect another share mode.
|
||||||
share_desktop_excluding_active: bool,
|
share_desktop_excluding_active: bool,
|
||||||
/// Persistent explanation from PixelPass while desktop exclusion is active.
|
/// The latest still-active warning from PixelPass while desktop exclusion
|
||||||
share_audio_exclusion_warning: Option<String>,
|
/// 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<AudioExclusionStatus>,
|
||||||
/// Whether the resolved pixelpass supports `--strict-audio` (per-app audio).
|
/// Whether the resolved pixelpass supports `--strict-audio` (per-app audio).
|
||||||
/// `false` ⇒ the picker offers whole-desktop only, because a per-app share
|
/// `false` ⇒ the picker offers whole-desktop only, because a per-app share
|
||||||
/// would pass a flag an older pixelpass rejects (audit P2). Optimistic `true`
|
/// would pass a flag an older pixelpass rejects (audit P2). Optimistic `true`
|
||||||
@@ -2614,9 +2616,27 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
|
|||||||
ShareAudioSelection::DesktopExcluding
|
ShareAudioSelection::DesktopExcluding
|
||||||
);
|
);
|
||||||
if state.share_desktop_excluding_active || excluding_starting {
|
if state.share_desktop_excluding_active || excluding_starting {
|
||||||
let message = audio_exclusion_status_message(&status);
|
match status {
|
||||||
state.status_message = message.clone();
|
AudioExclusionStatus::StreamStatusCleared { stream_serial } => {
|
||||||
state.share_audio_exclusion_warning = Some(message);
|
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 {
|
UiEvent::ClockSkewWarning {
|
||||||
@@ -6723,7 +6743,11 @@ fn view(state: &AppState) -> Element<'_, AppMessage> {
|
|||||||
.color(color_yellow),
|
.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 = details.push(text(warning).size(11).color(color_yellow));
|
||||||
}
|
}
|
||||||
details.into()
|
details.into()
|
||||||
@@ -8814,19 +8838,20 @@ fn with_pixelpass_help<'a>(
|
|||||||
.into()
|
.into()
|
||||||
}
|
}
|
||||||
|
|
||||||
fn audio_exclusion_status_message(status: &AudioExclusionStatus) -> String {
|
fn audio_exclusion_status_message(status: &AudioExclusionStatus) -> Option<String> {
|
||||||
match status {
|
match status {
|
||||||
AudioExclusionStatus::StreamUnsupported { reason, .. } => format!(
|
AudioExclusionStatus::StreamUnsupported { reason, .. } => Some(format!(
|
||||||
"Some system audio could not be shared safely ({reason}); it was left out."
|
"Some system audio could not be shared safely ({reason}); it was left out."
|
||||||
),
|
)),
|
||||||
|
AudioExclusionStatus::StreamStatusCleared { .. } => None,
|
||||||
AudioExclusionStatus::AecFailed { .. } =>
|
AudioExclusionStatus::AecFailed { .. } =>
|
||||||
"Audio exclusion could not verify PeerSpeak's echo canceller; viewers hear silence."
|
Some("Audio exclusion could not verify PeerSpeak's echo canceller; viewers hear silence."
|
||||||
.to_string(),
|
.to_string()),
|
||||||
AudioExclusionStatus::AecRevoked { .. } =>
|
AudioExclusionStatus::AecRevoked { .. } =>
|
||||||
"Audio exclusion stopped because PeerSpeak's echo canceller disappeared; viewers hear silence."
|
Some("Audio exclusion stopped because PeerSpeak's echo canceller disappeared; viewers hear silence."
|
||||||
.to_string(),
|
.to_string()),
|
||||||
AudioExclusionStatus::ForeignAecWarning { .. } =>
|
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::{
|
use super::{
|
||||||
AppConfig, AppMessage, AppState, AttachmentCache, AttachmentState, AudioExclusionStatus,
|
AppConfig, AppMessage, AppState, AttachmentCache, AttachmentState, AudioExclusionStatus,
|
||||||
CLOCK_SKEW_WARNING_VISIBLE_SECS, ChatEntry, ClockSkewBanner, GateMeter, METER_MAX, Screen,
|
CLOCK_SKEW_WARNING_VISIBLE_SECS, ChatEntry, ClockSkewBanner, GateMeter, METER_MAX, Screen,
|
||||||
ScreenBounds, ShareAudioSelection, UiEvent, attachment_default_name, clamp_window_position,
|
ScreenBounds, ShareAudioSelection, UiEvent, attachment_default_name,
|
||||||
clear_expired_clock_skew_warning, format_clock_skew_duration, format_duration,
|
audio_exclusion_status_message, clamp_window_position, clear_expired_clock_skew_warning,
|
||||||
format_relative_ago, friend_presence_notification, initial_window_position,
|
format_clock_skew_duration, format_duration, format_relative_ago,
|
||||||
now_playing_label, reconnect_attempt_chime, reconnected_chime, selected_wav_path,
|
friend_presence_notification, initial_window_position, now_playing_label,
|
||||||
set_peer_gate_config, set_peer_volume_config, show_clock_skew_warning, update,
|
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 iroh::SecretKey;
|
||||||
use std::collections::VecDeque;
|
use std::collections::VecDeque;
|
||||||
@@ -10065,7 +10091,8 @@ mod tests {
|
|||||||
state.share_audio_dropped = true;
|
state.share_audio_dropped = true;
|
||||||
state.share_audio_app_active = true;
|
state.share_audio_app_active = true;
|
||||||
state.share_desktop_excluding_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_app_audio_supported = false;
|
||||||
state.share_desktop_audio_exclusion_supported = true;
|
state.share_desktop_audio_exclusion_supported = true;
|
||||||
state.clock_skew_warning = Some(ClockSkewBanner {
|
state.clock_skew_warning = Some(ClockSkewBanner {
|
||||||
@@ -10436,7 +10463,8 @@ mod tests {
|
|||||||
assert!(
|
assert!(
|
||||||
state
|
state
|
||||||
.share_audio_exclusion_warning
|
.share_audio_exclusion_warning
|
||||||
.as_deref()
|
.as_ref()
|
||||||
|
.and_then(audio_exclusion_status_message)
|
||||||
.is_some_and(|message| message.contains("viewers hear silence"))
|
.is_some_and(|message| message.contains("viewers hear silence"))
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -10464,6 +10492,52 @@ mod tests {
|
|||||||
assert!(state.share_audio_exclusion_warning.is_none());
|
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]
|
#[test]
|
||||||
fn old_pixelpass_picker_offers_whole_desktop_only() {
|
fn old_pixelpass_picker_offers_whole_desktop_only() {
|
||||||
// P2 (version skew): when the resolved pixelpass lacks --strict-audio, the
|
// P2 (version skew): when the resolved pixelpass lacks --strict-audio, the
|
||||||
|
|||||||
@@ -1241,6 +1241,11 @@ const PING_INTERVAL: Duration = Duration::from_secs(15);
|
|||||||
/// Delay before the FIRST presence pass, so the endpoint's background `online()`
|
/// 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).
|
/// has a moment to finish (otherwise the first probes fail and friends flash offline).
|
||||||
const PING_STARTUP_DELAY: Duration = Duration::from_secs(3);
|
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
|
/// 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
|
/// *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,
|
||||||
);
|
);
|
||||||
ping_interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip);
|
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 {
|
loop {
|
||||||
let cmd = tokio::select! {
|
let cmd = tokio::select! {
|
||||||
biased;
|
biased;
|
||||||
@@ -1611,6 +1621,12 @@ async fn run_core_loop(
|
|||||||
Some(generation) = host_fault_rx.recv() => {
|
Some(generation) = host_fault_rx.recv() => {
|
||||||
CoreCommand::ScreenShareHostFault { generation }
|
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) => {
|
game_change = next_game_change(&mut game_rx) => {
|
||||||
// The detector worker published a new debounced game (or `None`).
|
// The detector worker published a new debounced game (or `None`).
|
||||||
let Some(detected) = game_change else {
|
let Some(detected) = game_change else {
|
||||||
@@ -3801,6 +3817,7 @@ mod tests {
|
|||||||
|
|
||||||
let lines = [
|
let lines = [
|
||||||
r#"{"event":"stream_unsupported","version":1,"stream_serial":4294967303,"reason":"port-exclusive"}"#,
|
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_failed","version":1,"module_index":536870919}"#,
|
||||||
r#"{"event":"aec_revoked","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"}"#,
|
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,
|
stream_serial: 4_294_967_303,
|
||||||
reason: "port-exclusive".to_string(),
|
reason: "port-exclusive".to_string(),
|
||||||
},
|
},
|
||||||
|
AudioExclusionStatus::StreamStatusCleared {
|
||||||
|
stream_serial: 4_294_967_303,
|
||||||
|
},
|
||||||
AudioExclusionStatus::AecFailed {
|
AudioExclusionStatus::AecFailed {
|
||||||
module_index: 536_870_919,
|
module_index: 536_870_919,
|
||||||
},
|
},
|
||||||
|
|||||||
+33
-4
@@ -82,10 +82,24 @@ pub enum ShareAudioSelection {
|
|||||||
/// define a second matching enum that could silently drift from the wire.
|
/// define a second matching enum that could silently drift from the wire.
|
||||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||||
pub enum AudioExclusionStatus {
|
pub enum AudioExclusionStatus {
|
||||||
StreamUnsupported { stream_serial: u64, reason: String },
|
StreamUnsupported {
|
||||||
AecFailed { module_index: u64 },
|
stream_serial: u64,
|
||||||
AecRevoked { module_index: u64 },
|
reason: String,
|
||||||
ForeignAecWarning { link_group: 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
|
/// One parsed line from pixelpass's `--output json` stdout stream. Mirrors the
|
||||||
@@ -177,6 +191,11 @@ pub fn parse_pixelpass_event(line: &str) -> Option<PixelpassEvent> {
|
|||||||
reason: v.get("reason")?.as_str()?.to_string(),
|
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) => {
|
"aec_failed" if json_u64(&v, "version") == Some(1) => {
|
||||||
PixelpassEvent::AudioExclusion(AudioExclusionStatus::AecFailed {
|
PixelpassEvent::AudioExclusion(AudioExclusionStatus::AecFailed {
|
||||||
module_index: json_u64(&v, "module_index")?,
|
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!(
|
assert_eq!(
|
||||||
parse_pixelpass_event(r#"{"event":"aec_failed","version":1,"module_index":536870919}"#,),
|
parse_pixelpass_event(r#"{"event":"aec_failed","version":1,"module_index":536870919}"#,),
|
||||||
Some(PixelpassEvent::AudioExclusion(
|
Some(PixelpassEvent::AudioExclusion(
|
||||||
|
|||||||
Reference in New Issue
Block a user