diff --git a/src/core/mod.rs b/src/core/mod.rs index 235b5cc..148a47a 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -662,9 +662,11 @@ struct ActiveSession { /// Our pixelpass screen-share host child while sharing (`kill_on_drop`, so it /// also dies if the session is dropped without an explicit stop). screenshare_host: Option, - /// pixelpass viewer children we spawned to watch peers' shares; killed on - /// session teardown (each also self-exits when its player window closes). - screenshare_viewers: Vec, + /// pixelpass viewer children we spawned to watch peers' shares, each paired + /// with the share ticket it's viewing so a re-watch of the same share can + /// replace (not stack) its player. Killed on session teardown (each also + /// self-exits when its player window closes). + screenshare_viewers: Vec<(String, tokio::process::Child)>, } impl ActiveSession { @@ -676,7 +678,7 @@ impl ActiveSession { if let Some(mut host) = self.screenshare_host.take() { let _ = host.kill().await; } - for mut viewer in self.screenshare_viewers.drain(..) { + for (_, mut viewer) in self.screenshare_viewers.drain(..) { let _ = viewer.kill().await; } self.datagram_task.abort(); @@ -2497,7 +2499,7 @@ async fn run_core_loop( #[cfg(target_os = "linux")] echo_cancel: echo_cancel_guard, screenshare_host: None, - screenshare_viewers: Vec::new(), + screenshare_viewers: Vec::<(String, tokio::process::Child)>::new(), }; let self_id = endpoint.id().to_string(); @@ -3221,11 +3223,27 @@ async fn run_core_loop( continue; } }; + if let Some(session) = &mut active_session { + // Drop viewers whose player window has already closed so the + // list only tracks live players. + session + .screenshare_viewers + .retain_mut(|(_, child)| !matches!(child.try_wait(), Ok(Some(_)))); + // One player per share: a second Watch click on a share we're + // already viewing is a retry (usually because the first window + // froze), so replace the existing player rather than stacking a + // second mpv — two players would double the shared audio. + if let Some(pos) = replace_viewer_index(&session.screenshare_viewers, &ticket) { + let (_, mut old) = session.screenshare_viewers.remove(pos); + let _ = old.kill().await; + crate::log_msg("Screen share viewer replaced (re-watch)"); + } + } match crate::screenshare::spawn_viewer(&bin, &ticket).await { Ok(child) => { crate::log_msg("Screen share viewer started"); if let Some(session) = &mut active_session { - session.screenshare_viewers.push(child); + session.screenshare_viewers.push((ticket, child)); } } Err(e) => { @@ -3241,14 +3259,23 @@ async fn run_core_loop( Ok(()) } +/// Index of an existing viewer for `ticket` in the live-viewers list, if any. +/// A re-watch of the same share replaces that player instead of stacking a +/// second one — two players decoding the same stream would double the shared +/// audio. Generic over the child value so the dedup rule is unit-testable +/// without spawning real player processes. +fn replace_viewer_index(viewers: &[(String, T)], ticket: &str) -> Option { + viewers.iter().position(|(t, _)| t == ticket) +} + #[cfg(test)] mod tests { use super::{ KnownPeers, MAX_OPUS_PAYLOAD, MAX_RETAINED_PEERS, MIC_LEVEL_REPORT_SAMPLES, MicLevelMeter, PLAYBACK_HANDOFF_QUEUE_FRAMES, PeerSpeakTicket, admit_retained, apply_peer_volume, apply_volume, audio_datagram_len_ok, coalesce_insert, coalesce_pop, frame_level, - mix_frames, mix_stereo_frames, next_game_change, send_playback_frame, should_auto_fetch, - stereo_to_mono, + mix_frames, mix_stereo_frames, next_game_change, replace_viewer_index, send_playback_frame, + should_auto_fetch, stereo_to_mono, }; use crate::core::messages::{CoalesceKey, CoreCommand, coalesce_key}; use std::collections::{HashMap, HashSet}; @@ -3259,6 +3286,19 @@ mod tests { iroh::SecretKey::generate().public() } + #[test] + fn re_watch_replaces_existing_viewer_for_same_ticket() { + // The value type stands in for a viewer Child; only the ticket matters. + let viewers = vec![("ticket-A".to_string(), 0u8), ("ticket-B".to_string(), 1u8)]; + // Re-watching an already-open share finds the existing player to replace. + assert_eq!(replace_viewer_index(&viewers, "ticket-A"), Some(0)); + assert_eq!(replace_viewer_index(&viewers, "ticket-B"), Some(1)); + // A different (new) share has nothing to replace — it opens fresh. + assert_eq!(replace_viewer_index(&viewers, "ticket-C"), None); + // Empty list: first watch of anything opens fresh. + assert_eq!(replace_viewer_index::(&[], "ticket-A"), None); + } + #[test] fn admit_retained_rejects_only_new_ids_at_the_cap() { // Below the cap, a brand-new identity is retained.