From d6b9862f1eba13437c8869d9752e07158bddd9f5 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 12 Aug 2026 13:10:14 +0200 Subject: [PATCH 1/3] fix(win-display): prune unplugged targets from the CCD restore snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A monitor unplugged mid-session left the teardown replaying a snapshot whose paths reference an absent target — SetDisplayConfig rejects the WHOLE array with 0x57 ERROR_INVALID_PARAMETER, nothing restores, the desk stays dark, and the next session snapshots the wreckage (the poisoned-snapshot chain; field 2026-08-12: rc=0x57 across a mid-session unplug, then sessions flipping between black and working at random, incl. one that genuinely encoded black). The restore now prunes the snapshot against a live QDC_ALL_PATHS availability sweep before replaying: stale paths drop, the mode table is rebuilt with only the entries the survivors reference (an orphaned mode entry is itself an 0x57), and an empty survivor set skips the apply so the dark-desk backstop decides. Verbatim behavior is unchanged when everything is still attached or when the availability query itself fails. --- crates/pf-win-display/src/win_display.rs | 303 +++++++++++++++++++++-- 1 file changed, 284 insertions(+), 19 deletions(-) diff --git a/crates/pf-win-display/src/win_display.rs b/crates/pf-win-display/src/win_display.rs index 80cbd8d7..e46caefe 100644 --- a/crates/pf-win-display/src/win_display.rs +++ b/crates/pf-win-display/src/win_display.rs @@ -1962,29 +1962,173 @@ pub fn restore_displays_ccd(saved: &SavedConfig) { isolate_journal::clear(); } +/// Every display target that still EXISTS right now — `(adapter LUID low, high, target id)` keys +/// from a full `QDC_ALL_PATHS` sweep, counting a target present when the OS says a monitor is +/// attached (`targetAvailable`) OR an active path drives it (the flag reads FALSE transiently +/// right after a removal — same rule as [`target_inventory`]). `None` when the CCD query itself +/// fails, so the caller can fall back to trusting its snapshot verbatim. +fn available_target_keys() -> Option> { + let mut np = 0u32; + let mut nm = 0u32; + // SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live locals the + // OS fills with the counts it wants for these flags. + if unsafe { GetDisplayConfigBufferSizes(QDC_ALL_PATHS, &mut np, &mut nm) }.is_err() { + return None; + } + let mut paths = vec![DISPLAYCONFIG_PATH_INFO::default(); np as usize]; + let mut modes = vec![DISPLAYCONFIG_MODE_INFO::default(); nm as usize]; + // SAFETY: the CCD contract — `paths`/`modes` were just allocated with exactly `np`/`nm` + // elements from the sizing call above, and are handed over with those same counts. + if unsafe { + QueryDisplayConfig( + QDC_ALL_PATHS, + &mut np, + paths.as_mut_ptr(), + &mut nm, + modes.as_mut_ptr(), + None, + ) + } + .is_err() + { + return None; + } + paths.truncate(np as usize); + let mut keys: Vec<(u32, i32, u32)> = Vec::new(); + for p in &paths { + let t = &p.targetInfo; + let key = (t.adapterId.LowPart, t.adapterId.HighPart, t.id); + let present = t.targetAvailable.as_bool() || p.flags & DISPLAYCONFIG_PATH_ACTIVE != 0; + if present && !keys.contains(&key) { + keys.push(key); + } + } + Some(keys) +} + +/// Drop every snapshot path whose TARGET no longer exists (`avail` — the live +/// [`available_target_keys`] sweep) and rebuild the mode table with only the entries the +/// survivors reference, remapping their `modeInfoIdx` slots. Both halves matter: +/// `SetDisplayConfig(SDC_USE_SUPPLIED_DISPLAY_CONFIG)` validates the WHOLE submission, so one +/// stale path — or one orphaned mode entry left behind by a dropped path — fails the entire +/// restore with 0x57 ERROR_INVALID_PARAMETER. Returns `(paths, modes, dropped_path_count)`; +/// pure over its inputs so the remap arithmetic is unit-testable without a live CCD. +fn prune_saved_config_for_targets( + paths: &[DISPLAYCONFIG_PATH_INFO], + modes: &[DISPLAYCONFIG_MODE_INFO], + avail: &[(u32, i32, u32)], +) -> ( + Vec, + Vec, + usize, +) { + let mut kept: Vec = Vec::with_capacity(paths.len()); + let mut new_modes: Vec = Vec::with_capacity(modes.len()); + // old mode index → new mode index, memoized: clone configs legitimately share a source mode + // entry between paths, and it must land in the rebuilt table exactly once. + let mut remap: Vec> = vec![None; modes.len()]; + let take = + |idx: u32, new_modes: &mut Vec, remap: &mut Vec>| { + if idx == DISPLAYCONFIG_PATH_MODE_IDX_INVALID { + return DISPLAYCONFIG_PATH_MODE_IDX_INVALID; + } + match modes.get(idx as usize) { + // An out-of-range index could never have applied — un-pin the mode rather than + // shipping a table the whole submission fails on. + None => DISPLAYCONFIG_PATH_MODE_IDX_INVALID, + Some(m) => match remap[idx as usize] { + Some(n) => n, + None => { + let n = new_modes.len() as u32; + new_modes.push(*m); + remap[idx as usize] = Some(n); + n + } + }, + } + }; + let mut dropped = 0usize; + for p in paths { + let t = &p.targetInfo; + if !avail.contains(&(t.adapterId.LowPart, t.adapterId.HighPart, t.id)) { + dropped += 1; + continue; + } + let mut p = *p; + // SAFETY: POD union reads (CCD header contract) — `modeInfoIdx` overlays a same-sized + // bitfield struct, both valid for every bit pattern; used only as bounds-checked indices. + let (src_idx, tgt_idx) = unsafe { + ( + p.sourceInfo.Anonymous.modeInfoIdx, + p.targetInfo.Anonymous.modeInfoIdx, + ) + }; + p.sourceInfo.Anonymous.modeInfoIdx = take(src_idx, &mut new_modes, &mut remap); + p.targetInfo.Anonymous.modeInfoIdx = take(tgt_idx, &mut new_modes, &mut remap); + kept.push(p); + } + (kept, new_modes, dropped) +} + fn restore_displays_ccd_inner(saved: &SavedConfig) { - let (paths, modes) = saved; - if paths.is_empty() { + let (saved_paths, saved_modes) = saved; + if saved_paths.is_empty() { return; } - // SAFETY: the CCD contract at the top of this file — the path/mode arrays go over as - // slices, so pointer and length cannot disagree, and both outlive this synchronous - // call. `retry_set_display_config` binds it to the input desktop, which is the one - // precondition a caller of this global-state write could otherwise get wrong. - let rc = crate::input_desktop::retry_set_display_config(|| unsafe { - SetDisplayConfig( - Some(paths.as_slice()), - Some(modes.as_slice()), - SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_ALLOW_CHANGES, - ) - }); - if rc == 0 { - tracing::info!("display isolate (CCD): restored original topology"); - } else { + // Prune the snapshot against what is STILL ATTACHED before replaying it. A monitor unplugged + // mid-session leaves the snapshot referencing an absent target, and SetDisplayConfig rejects + // the WHOLE array with 0x57 ERROR_INVALID_PARAMETER — nothing restores, the desk stays dark, + // and the next session snapshots that wreckage (the poisoned-snapshot chain's first link; + // field 2026-08-12: rc=0x57 across a mid-session unplug, then sessions flipping between + // black/working at random). Dropping the stale paths lets the surviving displays restore + // normally; when NOTHING survives there is nothing to replay and the dark-desk backstop + // below is the whole answer. + let (kept, pruned_modes, dropped); + let (paths, modes): (&Vec<_>, &Vec<_>) = match available_target_keys() { + Some(avail) => { + (kept, pruned_modes, dropped) = + prune_saved_config_for_targets(saved_paths, saved_modes, &avail); + if dropped > 0 { + tracing::warn!( + dropped, + kept = kept.len(), + "display isolate (CCD): snapshot references target(s) that are no longer \ + attached (unplugged mid-session?) — pruned them so the survivors can restore \ + (a verbatim replay fails whole with rc=0x57)" + ); + } + (&kept, &pruned_modes) + } + // The availability query itself failed — replay verbatim, exactly the old behavior. + None => (saved_paths, saved_modes), + }; + let mut apply_rc = 0i32; // 0 also when the replay was skipped (nothing left to apply) + if paths.is_empty() { tracing::warn!( - "display isolate (CCD): topology restore failed rc={rc:#x}{} — physical displays may be left deactivated", - sdc_access_denied_hint(rc) + "display isolate (CCD): nothing from the topology snapshot is still attached — \ + skipping the replay (the dark-desk backstop decides what lights up)" ); + } else { + // SAFETY: the CCD contract at the top of this file — the path/mode arrays go over as + // slices, so pointer and length cannot disagree, and both outlive this synchronous + // call. `retry_set_display_config` binds it to the input desktop, which is the one + // precondition a caller of this global-state write could otherwise get wrong. + let rc = crate::input_desktop::retry_set_display_config(|| unsafe { + SetDisplayConfig( + Some(paths.as_slice()), + Some(modes.as_slice()), + SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_ALLOW_CHANGES, + ) + }); + apply_rc = rc; + if rc == 0 { + tracing::info!("display isolate (CCD): restored original topology"); + } else { + tracing::warn!( + "display isolate (CCD): topology restore failed rc={rc:#x}{} — physical displays may be left deactivated", + sdc_access_denied_hint(rc) + ); + } } // GUARANTEE the desk is never left all-dark. The saved config can be unappliable (field // rc=0x64a ERROR_BAD_CONFIGURATION: it pinned a virtual target incarnation that was since @@ -2020,7 +2164,7 @@ fn restore_displays_ccd_inner(saved: &SavedConfig) { return; } tracing::warn!( - "display isolate (CCD): no external physical display active after the restore (rc={rc:#x}, connected={connected}) — forcing the EXTEND preset so the desk is not left dark" + "display isolate (CCD): no external physical display active after the restore (rc={apply_rc:#x}, connected={connected}) — forcing the EXTEND preset so the desk is not left dark" ); force_extend_topology(); // Measure what the force achieved: a sink still dark AFTER the EXTEND preset can never @@ -2128,3 +2272,124 @@ mod live_tests { tracing::info!("live CCD query: {n} active display path(s)"); } } + +#[cfg(test)] +mod prune_saved_config_tests { + //! The snapshot-prune remap arithmetic (`prune_saved_config_for_targets`) — pure over its + //! inputs, so the 0x57-poisoned-restore fix is testable without a live CCD: a stale target's + //! path must vanish, its modes must not orphan (an orphaned entry fails the whole + //! SetDisplayConfig exactly like the stale path did), and clone-shared modes must land once. + use super::*; + + fn path( + luid_low: u32, + target_id: u32, + src_mode: u32, + tgt_mode: u32, + ) -> DISPLAYCONFIG_PATH_INFO { + let mut p = DISPLAYCONFIG_PATH_INFO::default(); + p.targetInfo.adapterId.LowPart = luid_low; + p.targetInfo.id = target_id; + p.sourceInfo.adapterId.LowPart = luid_low; + p.sourceInfo.Anonymous.modeInfoIdx = src_mode; + p.targetInfo.Anonymous.modeInfoIdx = tgt_mode; + p + } + + fn mode(marker: u32) -> DISPLAYCONFIG_MODE_INFO { + DISPLAYCONFIG_MODE_INFO { + id: marker, + ..Default::default() + } + } + + fn indices(p: &DISPLAYCONFIG_PATH_INFO) -> (u32, u32) { + // SAFETY: POD union reads — `modeInfoIdx` overlays a same-sized bitfield struct, both + // valid for every bit pattern (the same contract the production reads rely on). + unsafe { + ( + p.sourceInfo.Anonymous.modeInfoIdx, + p.targetInfo.Anonymous.modeInfoIdx, + ) + } + } + + #[test] + fn everything_attached_survives_with_dense_indices() { + let paths = vec![path(1, 100, 0, 1), path(1, 200, 2, 3)]; + let modes = vec![mode(10), mode(11), mode(12), mode(13)]; + let avail = vec![(1, 0, 100), (1, 0, 200)]; + let (kept, new_modes, dropped) = prune_saved_config_for_targets(&paths, &modes, &avail); + assert_eq!(dropped, 0); + assert_eq!(kept.len(), 2); + assert_eq!(new_modes.len(), 4); + assert_eq!(indices(&kept[0]), (0, 1)); + assert_eq!(indices(&kept[1]), (2, 3)); + assert_eq!(new_modes[3].id, 13, "mode entries follow their paths"); + } + + #[test] + fn a_gone_target_drops_its_path_and_modes() { + // Target 200 was unplugged mid-session (the field rc=0x57 case): its path AND its two + // mode entries must vanish, and the survivor's indices must be remapped dense. + let paths = vec![path(1, 100, 0, 1), path(1, 200, 2, 3)]; + let modes = vec![mode(10), mode(11), mode(12), mode(13)]; + let avail = vec![(1, 0, 100)]; + let (kept, new_modes, dropped) = prune_saved_config_for_targets(&paths, &modes, &avail); + assert_eq!(dropped, 1); + assert_eq!(kept.len(), 1); + assert_eq!(kept[0].targetInfo.id, 100); + assert_eq!( + new_modes.len(), + 2, + "the dropped path's modes must not orphan" + ); + assert_eq!((new_modes[0].id, new_modes[1].id), (10, 11)); + assert_eq!(indices(&kept[0]), (0, 1)); + } + + #[test] + fn a_clone_shared_source_mode_lands_exactly_once() { + // Clone configs share one source mode entry between paths — the rebuilt table must + // contain it once, referenced by both survivors. + let paths = vec![path(1, 100, 0, 1), path(1, 200, 0, 2)]; + let modes = vec![mode(10), mode(11), mode(12)]; + let avail = vec![(1, 0, 100), (1, 0, 200)]; + let (kept, new_modes, dropped) = prune_saved_config_for_targets(&paths, &modes, &avail); + assert_eq!(dropped, 0); + assert_eq!(new_modes.len(), 3); + let (a_src, _) = indices(&kept[0]); + let (b_src, _) = indices(&kept[1]); + assert_eq!(a_src, b_src, "shared source mode keeps one table entry"); + } + + #[test] + fn unpinned_and_corrupt_indices_stay_unpinned() { + // The INVALID sentinel must pass through, and an out-of-range index (a corrupt snapshot) + // must degrade to unpinned rather than shipping a table the whole apply fails on. + let paths = vec![path(1, 100, DISPLAYCONFIG_PATH_MODE_IDX_INVALID, 99)]; + let modes = vec![mode(10)]; + let avail = vec![(1, 0, 100)]; + let (kept, new_modes, dropped) = prune_saved_config_for_targets(&paths, &modes, &avail); + assert_eq!(dropped, 0); + assert!(new_modes.is_empty()); + assert_eq!( + indices(&kept[0]), + ( + DISPLAYCONFIG_PATH_MODE_IDX_INVALID, + DISPLAYCONFIG_PATH_MODE_IDX_INVALID + ) + ); + } + + #[test] + fn different_adapters_do_not_alias_the_same_target_id() { + // Target ids are only unique per adapter LUID — a survivor on adapter 2 must not keep a + // stale path alive on adapter 1 just because the ids match. + let paths = vec![path(1, 100, 0, 1)]; + let modes = vec![mode(10), mode(11)]; + let avail = vec![(2, 0, 100)]; + let (kept, _, dropped) = prune_saved_config_for_targets(&paths, &modes, &avail); + assert_eq!((kept.len(), dropped), (0, 1)); + } +} -- 2.54.0 From a2aa0a5f970ed3d0c0027a68e8856423f2207c4f Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 12 Aug 2026 13:16:58 +0200 Subject: [PATCH 2/3] fix(apple): never sit black AND silent while the video format is unseeded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both pumps (Stage2Pipeline + the stage-1 StreamPump) dropped every AU in a silent guard while 'format' was nil — the opening IDR's parameter sets never arrived or never parsed, and under the host's infinite GOP nothing re-delivers them unless the client asks. Field 2026-08-12 (iOS, H265): sessions decoding nothing, requesting nothing, host streaming perfectly — a black stream with zero recovery requests, randomly self-healing only when some later event produced a fresh IDR. While format is nil after the first received AU, the pumps now set awaitingIDR (routing through the existing 100 ms-throttled recovery.request()) and log the state once, so a lost/unparsed opening IDR converts to a normal recovery round-trip instead of a permanent silent black. --- .../PunktfunkKit/Video/Stage2Pipeline.swift | 17 +++++++++++++++++ .../Sources/PunktfunkKit/Video/StreamPump.swift | 15 +++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/clients/apple/Sources/PunktfunkKit/Video/Stage2Pipeline.swift b/clients/apple/Sources/PunktfunkKit/Video/Stage2Pipeline.swift index bd7e9d52..17e54014 100644 --- a/clients/apple/Sources/PunktfunkKit/Video/Stage2Pipeline.swift +++ b/clients/apple/Sources/PunktfunkKit/Video/Stage2Pipeline.swift @@ -57,6 +57,8 @@ let presentDebug = ProcessInfo.processInfo.environment["PUNKTFUNK_PRESENT_DEBUG" /// to Console.app wirelessly with no env var / Xcode attach. Always on for deadline pacing (the /// stats are a few arrays + one log line per second); other pacings keep the env-gated print. private let presentLog = Logger(subsystem: "io.unom.punktfunk", category: "present") +/// Pump-side events (loss recovery, format seeding) — the stage-2 sibling of StreamPump's log. +private let pumpLog = Logger(subsystem: "io.unom.punktfunk", category: "pump") /// Decoded-frame hand-off between the decode half and the render thread. The POLICY is the /// user's presentation intent (design/apple-presentation-rebuild.md — the 2026-07 rebuild that @@ -932,6 +934,21 @@ public final class Stage2Pipeline { } awaitingIDR = false // a fresh IDR re-anchored decode — recovery complete } + if format == nil { + // No decodable format yet: the opening IDR's parameter sets never + // arrived (or never parsed), and under the host's infinite GOP nothing + // re-delivers them unless we ASK. Without this the guard below drops + // every AU silently, forever — the field "black stream, zero recovery + // requests" state (2026-08-12): the host streams perfectly, the client + // shows nothing and says nothing. awaitingIDR routes through the same + // 100 ms-throttled recovery.request() at the top of the loop. + if !awaitingIDR { + pumpLog.warning( + "video: received AUs but no decodable format (missing/unparsed parameter sets) — requesting an IDR until one seeds it" + ) + } + awaitingIDR = true + } guard let f = format, !token.isStopped else { return true } if decoder.decode(au: au, format: f) { decodeFailRun = 0 diff --git a/clients/apple/Sources/PunktfunkKit/Video/StreamPump.swift b/clients/apple/Sources/PunktfunkKit/Video/StreamPump.swift index e647f44f..215a5aea 100644 --- a/clients/apple/Sources/PunktfunkKit/Video/StreamPump.swift +++ b/clients/apple/Sources/PunktfunkKit/Video/StreamPump.swift @@ -116,6 +116,21 @@ final class StreamPump { } awaitingIDR = false // a fresh IDR re-anchored decode — recovery complete } + if format == nil { + // No decodable format yet: the opening IDR's parameter sets never + // arrived (or never parsed), and under the host's infinite GOP nothing + // re-delivers them unless we ASK. Without this the format guard below + // drops every AU silently, forever — the field "black stream, zero + // recovery requests" state (2026-08-12). awaitingIDR routes through the + // same 100 ms-throttled recovery.request() at the top of the loop. + if !awaitingIDR { + awaitingSince = Date() + pumpLog.warning( + "video: received AUs but no decodable format (missing/unparsed parameter sets) — requesting an IDR until one seeds it" + ) + } + awaitingIDR = true + } let failed = layer.status == .failed if failed { // Decode wedged hard (the cold-first-connect case — a lost/corrupt opening -- 2.54.0 From 712ee935d6834bb2d70fb998c731c17ab5068e79 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 12 Aug 2026 13:29:26 +0200 Subject: [PATCH 3/3] fix(win-display): a failed devnode re-enable must keep its crash-journal entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit enable_instances cleared every requested id from the pnp-disabled-monitors journal unconditionally — a mid-life re-enable failure erased its own crash-recovery entry, so neither the running host nor the next start would ever retry, leaving the operator's monitor disabled (invisible to Windows and every display listing) until a manual Device Manager fix. Field 2026-08-12 (Android-client host running the pnp_disable_monitors axis): displays gone from the client after 2-3 sessions, only a full host restart bringing them back — the restart path only works while the journal still has the entry. Only successfully re-enabled ids clear now; failures stay journaled (and are logged with the consequence) so startup_recover retries them. --- crates/pf-win-display/src/monitor_devnode.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/pf-win-display/src/monitor_devnode.rs b/crates/pf-win-display/src/monitor_devnode.rs index 0d1df1cd..151ae157 100644 --- a/crates/pf-win-display/src/monitor_devnode.rs +++ b/crates/pf-win-display/src/monitor_devnode.rs @@ -217,18 +217,32 @@ fn journal_and_disable(targets: Vec<(String, String)>) -> Vec { disabled } -/// Re-enable `ids` (teardown / recovery) and clear them from the journal. +/// Re-enable `ids` (teardown / recovery) and clear the ones that actually re-enabled from the +/// journal. A FAILED re-enable must keep its journal entry: it is the only record that the +/// devnode is still disabled, and the next host start's [`startup_recover`] is the only thing +/// left that will retry it. (The old behavior cleared every requested id unconditionally — a +/// mid-life re-enable failure erased its own crash-recovery entry, leaving the operator's +/// monitor invisible to Windows AND to every display listing until they re-enabled it by hand +/// in Device Manager: the "my displays are gone until I restart everything" field class.) pub fn enable_instances(ids: &[String]) -> u32 { let mut ok = 0u32; + let mut reenabled: Vec<&String> = Vec::with_capacity(ids.len()); for id in ids { if set_devnode(id, false) { tracing::info!(id, "PnP-disable: monitor devnode re-enabled"); + reenabled.push(id); ok += 1; + } else { + tracing::warn!( + id, + "PnP-disable: monitor devnode re-enable FAILED — keeping its crash-journal \ + entry so the next host start retries (until then this monitor stays disabled)" + ); } } let journal: Vec = read_journal() .into_iter() - .filter(|j| !ids.contains(j)) + .filter(|j| !reenabled.contains(&j)) .collect(); write_journal(&journal); ok -- 2.54.0