diff --git a/crates/punktfunk-host/src/punktfunk1.rs b/crates/punktfunk-host/src/punktfunk1.rs index 584c3a2..9c0597f 100644 --- a/crates/punktfunk-host/src/punktfunk1.rs +++ b/crates/punktfunk-host/src/punktfunk1.rs @@ -957,21 +957,21 @@ async fn serve_session( Punktfunk1Source::Virtual => { let compositor = compositor .expect("the Virtual source resolves a compositor during the handshake"); - virtual_stream( + virtual_stream(SessionContext { session, mode, seconds, - stop_stream, - &reconfig_rx, - &keyframe_rx, + stop: stop_stream, + reconfig: reconfig_rx, + keyframe: keyframe_rx, compositor, bitrate_kbps, bit_depth, probe_rx, probe_result_tx, - fec_target_dp, - conn_stream, - ) + fec_target: fec_target_dp, + conn: conn_stream, + }) } } }) @@ -2165,22 +2165,40 @@ static IDD_SESSION_STOP: std::sync::Mutex>> = std::sync:: #[cfg(target_os = "windows")] static IDD_SETUP_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); -#[allow(clippy::too_many_arguments)] -fn virtual_stream( +/// All per-session inputs for [`virtual_stream`] / [`virtual_stream_relay`], bundled so the session entry +/// is one moved value instead of a 13-positional-argument `#[allow(too_many_arguments)]` signature +/// (Goal-1 stage 4, plan §2.4). Everything is **owned** — the receivers move in (`virtual_stream` is their +/// only consumer) — so the whole context moves into the stream thread and the borrow plumbing disappears. +struct SessionContext { + /// The hardened data-plane `Session` (Leopard FEC + AES-GCM over UDP); moved into the send thread. session: Session, + /// The client's requested mode — the virtual output is created at exactly this WxH@Hz (no scaling). mode: punktfunk_core::Mode, + /// Stream duration cap (the persistent listener bounds back-to-back sessions). seconds: u32, + /// Session stop flag (set on disconnect / reconnect-preempt). stop: Arc, - reconfig: &std::sync::mpsc::Receiver, - keyframe: &std::sync::mpsc::Receiver<()>, + /// Accepted mid-stream mode switches — the pipeline is rebuilt at the new mode. + reconfig: std::sync::mpsc::Receiver, + /// Client decode-recovery keyframe requests. + keyframe: std::sync::mpsc::Receiver<()>, + /// The resolved compositor backend (moot on Windows — `vdisplay::open` ignores it there). compositor: crate::vdisplay::Compositor, + /// Negotiated encoder bitrate (kbps). bitrate_kbps: u32, + /// Negotiated encode bit depth (8, or 10 = HEVC Main10). bit_depth: u8, + /// Speed-test burst requests (see [`service_probes`]). probe_rx: std::sync::mpsc::Receiver, + /// Speed-test results back to the control task. probe_result_tx: tokio::sync::mpsc::UnboundedSender, + /// Adaptive-FEC target the control task updates from the client's loss reports. fec_target: Arc, + /// The QUIC control connection (carries host→client 0xCE source-HDR metadata mid-stream). conn: quinn::Connection, -) -> Result<()> { +} + +fn virtual_stream(ctx: SessionContext) -> Result<()> { // This thread runs the capture+encode loop (single-process: Linux / synthetic / NO_WGC DDA) — or // tail-calls the relay below. Elevate it so a CPU-heavy game can't deschedule our GPU submission. boost_thread_priority(true); @@ -2188,7 +2206,7 @@ fn virtual_stream( // path now reads this typed `SessionPlan` instead of re-deriving from config at each dispatch site // (the latent "capture and encode disagree on the backend" hazard, plan §2.4). `bit_depth` is the // only per-session input — capture/topology/encoder are otherwise pure functions of `HostConfig`. - let plan = crate::session_plan::SessionPlan::resolve(bit_depth); + let plan = crate::session_plan::SessionPlan::resolve(ctx.bit_depth); tracing::info!(?plan, "resolved session plan"); // Windows two-process secure-desktop path: when the host runs as SYSTEM (required for the secure // desktop + SendInput), WGC can't activate in-process, so we capture the normal desktop via a @@ -2196,22 +2214,25 @@ fn virtual_stream( // user, and stays the path on Linux.) See docs/windows-secure-desktop.md. #[cfg(target_os = "windows")] if plan.topology == crate::session_plan::SessionTopology::TwoProcessRelay { - return virtual_stream_relay( - session, - mode, - seconds, - stop, - reconfig, - keyframe, - compositor, - bitrate_kbps, - bit_depth, - probe_rx, - probe_result_tx, - fec_target, - conn, - ); + return virtual_stream_relay(ctx); } + // Single-process path: unpack the context into the locals the loop below uses (names unchanged, so the + // body is byte-for-byte the same; the receivers are now owned but `try_recv()` is identical). + let SessionContext { + session, + mode, + seconds, + stop, + reconfig, + keyframe, + compositor, + bitrate_kbps, + bit_depth, + probe_rx, + probe_result_tx, + fec_target, + conn, + } = ctx; tracing::info!( compositor = compositor.id(), ?mode, @@ -2587,27 +2608,29 @@ fn virtual_stream( /// helper at the new mode (and drops the stale-target DDA); keyframe requests forward to the active /// source. #[cfg(target_os = "windows")] -#[allow(clippy::too_many_arguments)] -fn virtual_stream_relay( - session: Session, - mode: punktfunk_core::Mode, - seconds: u32, - stop: Arc, - reconfig: &std::sync::mpsc::Receiver, - keyframe: &std::sync::mpsc::Receiver<()>, - compositor: crate::vdisplay::Compositor, - bitrate_kbps: u32, - bit_depth: u8, - probe_rx: std::sync::mpsc::Receiver, - probe_result_tx: tokio::sync::mpsc::UnboundedSender, - fec_target: Arc, - // The SYSTEM-host relay path doesn't yet send the source mastering metadata as 0xCE — the - // helper's in-band SEI carries it (Windows follow-up). Held for that future wiring. - _conn: quinn::Connection, -) -> Result<()> { +fn virtual_stream_relay(ctx: SessionContext) -> Result<()> { use crate::capture::dxgi::WinCaptureTarget; use crate::capture::wgc_relay::HelperRelay; use crate::capture::Capturer; // trait methods (set_active/next_frame) on the concrete DuplCapturer + + // Unpack the context (names unchanged so the body is identical). The relay doesn't yet send the + // source's 0xCE HDR metadata — the helper's in-band SEI carries it (a Windows follow-up) — so `conn` + // is held unused. + let SessionContext { + session, + mode, + seconds, + stop, + reconfig, + keyframe, + compositor, + bitrate_kbps, + bit_depth, + probe_rx, + probe_result_tx, + fec_target, + conn: _conn, + } = ctx; tracing::info!( ?mode, bitrate_kbps, diff --git a/docs/windows-host-goal1-plan.md b/docs/windows-host-goal1-plan.md index ce0accc..3866890 100644 --- a/docs/windows-host-goal1-plan.md +++ b/docs/windows-host-goal1-plan.md @@ -72,11 +72,24 @@ so it is behavior-preserving. Risk: medium-high (rewires the deployed decision). binary reproduced the identical `frames=0`**, proving the no-frame is environmental, **not** a Stage-3 regression. Stage 3 is behavior-equivalent to the shipping host. Box restored to its deployed state. -**Stage 4 — `SessionContext` + `SessionFactory`/`Session`.** -Bundle the 12–13-arg `#[allow(too_many_arguments)]` signatures into `SessionContext`; `SessionFactory.build()` -owns the RAII chain `vdm.lease(mode) → open_capturer(vout, fmt) → open_encoder(plan) → spawn pipeline`, with -`Session::drop` the ONLY teardown path. Risk: high (teardown ordering — the §1 RAII asserts are mandatory). -Verify: box build + on-glass (connect/disconnect/reconnect, no leaked monitors/threads). +**Stage 4 — `SessionContext` (the arg-bundling). ✅ DONE (box-build validated). `SessionFactory`/`Session::drop` deferred to §2.5 — see below.** +Bundled the 13-positional-argument `#[allow(too_many_arguments)]` session entry (`virtual_stream` **and** +`virtual_stream_relay`) into one owned `SessionContext` struct, moved into the stream thread. The receivers +move in (`virtual_stream` is their only consumer), retiring the `&Receiver` borrow plumbing. **Behavior- +identical by construction**: each function destructures the context into the same local names at the top, so +the ~400-line loop bodies are byte-for-byte unchanged. Removed both `#[allow(too_many_arguments)]` attrs. + +**Scoped deliberately.** The plan's `SessionFactory.build()` owning a `vdm.lease(mode) → open_capturer → +open_encoder → spawn` RAII chain with `Session::drop` as the *only* teardown is **coupled to §2.5's +ownership-model rewrite** — it needs a host-side `VirtualDisplayManager`/`MonitorLease` that does not exist +yet (the lifecycle still lives in the `CURRENT_MON_GEN`/`IDD_SETUP_LOCK` globals + the per-compositor +`vdisplay` backends). The current teardown is **already drop-based** (the capturer owns the keepalive whose +`Drop` releases the monitor — "restore displays before REMOVE" lives there; only `send_thread.join()` is +explicit), and it is the validated shipping path. Wrapping the deployed reconfig/switch/rebuild loop in a +`Session::drop` for a behavior-preserving change would add real regression risk for marginal gain. So the +`SessionFactory`/`Session::drop`/`vdm.lease` work is folded into §2.5 (its natural home); this stage delivers +the concrete, safe arg-bundling. Risk: low (behavior-identical). Verify: Linux + box build (the relay +destructure is the only Windows-only piece); the teardown on-glass gate moves to the §2.5 work. **Stage 5 — seam-trait tightenings (plan §2.3).** `Capturer::open_capturer(vout, want: OutputFormat)` takes the format IN (kills the