diff --git a/crates/openplay-sender/src/app.rs b/crates/openplay-sender/src/app.rs index f918a53..7f1642c 100644 --- a/crates/openplay-sender/src/app.rs +++ b/crates/openplay-sender/src/app.rs @@ -48,6 +48,7 @@ pub struct SenderApp { show_add_miracast: bool, miracast_name: String, miracast_addr: String, + miracast_port: String, miracast_is_p2p: bool, } @@ -161,10 +162,11 @@ impl SenderApp { show_add_airplay: false, airplay_name: String::new(), airplay_ip: String::new(), - airplay_port: "7000".to_string(), + airplay_port: DEFAULT_AIRPLAY_PORT.to_string(), show_add_miracast: false, miracast_name: String::new(), miracast_addr: String::new(), + miracast_port: DEFAULT_MIRACAST_PORT.to_string(), miracast_is_p2p: false, } } @@ -351,13 +353,7 @@ impl SenderApp { .build() .unwrap(); rt.block_on(start_miracast_cast( - addr.ip(), - bitrate, - fps, - force_sw, - handle, - stop, - status_cb, + addr, bitrate, fps, force_sw, handle, stop, status_cb, )); }); } @@ -573,33 +569,45 @@ impl eframe::App for SenderApp { ui.add_space(8.0); ui.horizontal(|ui| { if ui.button("Add").clicked() { - if let Ok(ip) = IpAddr::from_str(&self.airplay_ip) { - let port: u16 = self.airplay_port.parse().unwrap_or(7000); - let name = if self.airplay_name.is_empty() { - self.airplay_ip.clone() - } else { - self.airplay_name.clone() - }; - let receiver = DiscoveredReceiver::AirPlay( - openplay_discovery::AirPlayReceiverInfo { - name: format!("manual-airplay-{ip}"), - display_name: name, - addresses: vec![ip], - port, - device_id: String::new(), - features: String::new(), - model: "Manual".to_string(), - }, - ); - self.add_receiver(receiver); - info!(ip = %ip, port, "Added manual AirPlay receiver"); - } else { - self.status = "Invalid IP address".to_string(); + // Only dismiss the dialog once something was + // actually added: closing on a rejected entry threw + // away what the user typed and left the complaint + // stranded in the status bar with nothing to fix. + match ( + IpAddr::from_str(&self.airplay_ip), + parse_port(&self.airplay_port), + ) { + (Ok(ip), Some(port)) => { + let name = if self.airplay_name.is_empty() { + self.airplay_ip.clone() + } else { + self.airplay_name.clone() + }; + let receiver = DiscoveredReceiver::AirPlay( + openplay_discovery::AirPlayReceiverInfo { + name: format!("manual-airplay-{ip}"), + display_name: name, + addresses: vec![ip], + port, + device_id: String::new(), + features: String::new(), + model: "Manual".to_string(), + }, + ); + self.add_receiver(receiver); + info!(ip = %ip, port, "Added manual AirPlay receiver"); + self.show_add_airplay = false; + self.airplay_name.clear(); + self.airplay_ip.clear(); + self.airplay_port = DEFAULT_AIRPLAY_PORT.to_string(); + } + (Err(_), _) => { + self.status = "Invalid IP address".to_string(); + } + (_, None) => { + self.status = INVALID_PORT_MSG.to_string(); + } } - self.show_add_airplay = false; - self.airplay_name.clear(); - self.airplay_ip.clear(); - self.airplay_port = "7000".to_string(); } if ui.button("Cancel").clicked() { self.show_add_airplay = false; @@ -623,6 +631,12 @@ impl eframe::App for SenderApp { "IP Address:" }); ui.text_edit_singleline(&mut self.miracast_addr); + // Wi-Fi Direct has no port to enter: the sink's address does + // not exist until the P2P group forms. + if !self.miracast_is_p2p { + ui.label("Port:"); + ui.text_edit_singleline(&mut self.miracast_port); + } ui.add_space(8.0); ui.horizontal(|ui| { if ui.button("Add").clicked() { @@ -631,7 +645,7 @@ impl eframe::App for SenderApp { } else { self.miracast_name.clone() }; - if self.miracast_is_p2p { + let added = if self.miracast_is_p2p { if self.miracast_addr.len() >= 11 && self.miracast_addr.contains(':') { @@ -642,26 +656,45 @@ impl eframe::App for SenderApp { }, }); self.add_receiver(r); + true } else { self.status = "Invalid MAC address".to_string(); + false } - } else if let Ok(ip) = IpAddr::from_str(&self.miracast_addr) { - let r = DiscoveredReceiver::Miracast(MiracastReceiver { - display_name: name, - mode: MiracastMode::Infrastructure { - addr: ip, - port: 7236, - }, - }); - self.add_receiver(r); - info!(ip = %ip, "Added manual Miracast receiver"); } else { - self.status = "Invalid IP address".to_string(); + match ( + IpAddr::from_str(&self.miracast_addr), + parse_port(&self.miracast_port), + ) { + (Ok(ip), Some(port)) => { + let r = DiscoveredReceiver::Miracast(MiracastReceiver { + display_name: name, + mode: MiracastMode::Infrastructure { addr: ip, port }, + }); + self.add_receiver(r); + info!(ip = %ip, port, "Added manual Miracast receiver"); + true + } + (Err(_), _) => { + self.status = "Invalid IP address".to_string(); + false + } + (_, None) => { + self.status = INVALID_PORT_MSG.to_string(); + false + } + } + }; + + // Same rule as the AirPlay dialog: keep the entry on + // screen unless it was accepted. + if added { + self.show_add_miracast = false; + self.miracast_name.clear(); + self.miracast_addr.clear(); + self.miracast_port = DEFAULT_MIRACAST_PORT.to_string(); + self.miracast_is_p2p = false; } - self.show_add_miracast = false; - self.miracast_name.clear(); - self.miracast_addr.clear(); - self.miracast_is_p2p = false; } if ui.button("Cancel").clicked() { self.show_add_miracast = false; @@ -674,6 +707,30 @@ impl eframe::App for SenderApp { // ─── Helpers ────────────────────────────────────────────────────────────────── +/// Default port an AirPlay receiver listens on. +const DEFAULT_AIRPLAY_PORT: u16 = 7000; + +/// Default RTSP port for Wi-Fi Display. OpenPlay is the RTSP *server* for +/// Miracast, but a manually added sink may still listen elsewhere. +const DEFAULT_MIRACAST_PORT: u16 = 7236; + +/// Message shown when a manually entered port will not parse. +const INVALID_PORT_MSG: &str = "Invalid port — expected 1-65535"; + +/// Parses a user-entered port, rejecting anything that is not a usable port. +/// +/// The previous `parse().unwrap_or(7000)` swallowed typos silently: `70000` +/// overflows a `u16` and `abc` is not a number, and both became 7000, so the +/// dialog reported success and the cast then failed against a port the user +/// never chose. Port 0 is rejected too — it parses fine but asks the OS for an +/// ephemeral port, which is meaningless for a destination address. +fn parse_port(raw: &str) -> Option { + match raw.trim().parse::() { + Ok(0) | Err(_) => None, + Ok(port) => Some(port), + } +} + fn protocol_badge(protocol: Protocol) -> (&'static str, Color32) { let color = match protocol { Protocol::OpenPlay => Color32::from_rgb(80, 160, 255), @@ -711,3 +768,68 @@ fn receiver_subtitle(receiver: &DiscoveredReceiver) -> String { }, } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_ordinary_ports() { + assert_eq!(parse_port("7000"), Some(7000)); + assert_eq!(parse_port("7236"), Some(7236)); + assert_eq!(parse_port("1"), Some(1)); + assert_eq!(parse_port("65535"), Some(65535)); + } + + #[test] + fn tolerates_surrounding_whitespace() { + assert_eq!(parse_port(" 7000 "), Some(7000)); + assert_eq!(parse_port("\t7236\n"), Some(7236)); + } + + /// The regression this helper exists for: `parse().unwrap_or(7000)` turned + /// every one of these into 7000 and reported success. + #[test] + fn rejects_what_unwrap_or_used_to_swallow() { + assert_eq!(parse_port("70000"), None, "overflows u16"); + assert_eq!(parse_port("abc"), None, "not a number"); + assert_eq!(parse_port(""), None, "empty"); + assert_eq!(parse_port(" "), None, "whitespace only"); + assert_eq!(parse_port("-1"), None, "negative"); + assert_eq!(parse_port("70.00"), None, "not an integer"); + } + + #[test] + fn rejects_port_zero() { + // Parses as a u16 but asks the OS for an ephemeral port, which is + // meaningless as a destination. + assert_eq!(parse_port("0"), None); + } + + /// The port a user types has to survive all the way to the socket. It used + /// to reach `MiracastMode::Infrastructure` correctly and then get dropped + /// at the call site, which passed `addr.ip()` into a function that + /// re-attached a hardcoded 7236. + #[test] + fn a_custom_miracast_port_survives_into_the_socket_address() { + let r = DiscoveredReceiver::Miracast(MiracastReceiver { + display_name: "linux box".to_string(), + mode: MiracastMode::Infrastructure { + addr: "192.168.0.105".parse().unwrap(), + port: 7290, + }, + }); + + let addr = r.addr().expect("infrastructure mode always has an address"); + assert_eq!(addr.port(), 7290, "the entered port must not be replaced"); + assert_eq!(addr.to_string(), "192.168.0.105:7290"); + } + + #[test] + fn defaults_are_the_documented_protocol_ports() { + assert_eq!(DEFAULT_AIRPLAY_PORT, 7000); + assert_eq!(DEFAULT_MIRACAST_PORT, 7236); + assert_eq!(parse_port(&DEFAULT_AIRPLAY_PORT.to_string()), Some(7000)); + assert_eq!(parse_port(&DEFAULT_MIRACAST_PORT.to_string()), Some(7236)); + } +} diff --git a/crates/openplay-sender/src/casting.rs b/crates/openplay-sender/src/casting.rs index 913603c..96db0ab 100644 --- a/crates/openplay-sender/src/casting.rs +++ b/crates/openplay-sender/src/casting.rs @@ -14,8 +14,6 @@ use openplay_pipeline::{ probe_best_encoder, AirPlaySenderPipeline, CaptureConfig, EncoderType, MiracastSenderPipeline, }; -const MIRACAST_RTSP_PORT: u16 = 7236; - /// Handle that allows signalling an active cast to stop. #[derive(Clone)] pub struct CastStopHandle { @@ -115,8 +113,11 @@ pub async fn start_airplay_cast( // ─── Miracast (Infrastructure / MICE) ───────────────────────────────────────── +/// `sink_addr` carries the port the receiver was discovered or entered with. +/// This used to take a bare `IpAddr` and re-attach a hardcoded 7236, which +/// silently discarded the port from a manually added sink. pub async fn start_miracast_cast( - receiver_ip: std::net::IpAddr, + sink_addr: SocketAddr, bitrate_kbps: u32, framerate: u32, force_sw_encode: bool, @@ -136,7 +137,6 @@ pub async fn start_miracast_cast( }; let capture_config = make_capture_config(&capture, framerate); - let sink_addr = SocketAddr::new(receiver_ip, MIRACAST_RTSP_PORT); let stop_flag = stop_handle.flag(); status_callback("Connecting via Miracast...");