1196 Commits

Include renames

Author	SHA1	Message	Date
enricobuehlerandClaude Fable 5	35c61fee64	chore(client): the windows-rs pin moves a month forward, onto the SDK-metadata bindings ... The July 2026 windows-rs brings a reconciler keyed-child-order fix (#4728), widget validation (#4727), a DPI collision fix (#4751), icon elements (#4736), multi-window support (#4730) and scroll virtualization (#4710) — the re-render fixes the Windows client has been working around at the architecture level. All three pinned deps (windows-reactor, windows, windows-reactor-setup) move together so windows-core stays unified across the swap-chain hand-off, and pf-client-core moves with them. The bulk of the diff is #4689: windows/windows-sys now generate straight from the Windows SDK, so the `Win32_*` namespace features became one feature per SDK header (winuser, dxgi, d3d11, …), the PascalCase namespace modules became header-named modules, struct-returning COM methods take explicit out-params and return HRESULT, Win32 functions return their raw BOOL/HANDLE instead of Result, and flag constants are plain integers. Both crates' Win32 code is rewritten to that shape; behaviour is unchanged on every path. Riding along, all already stale before the bump: the README and the three Windows workflows stop claiming windows-reactor's build.rs needs CARGO_WORKSPACE_DIR (that build.rs no longer exists — staging moved to windows-reactor-setup via OUT_DIR); the README layout section stops describing modules that moved into the session binary long ago and gains the manual smoke checklist; the notices generator learns the SPDX for crates that ship license files without a `license` field, which turns windows-reactor-setup's UNKNOWN into MIT OR Apache-2.0; and the crate records its real rust-version (1.96) instead of inheriting the workspace's 1.82. Verified: cargo check/clippy/fmt clean on punktfunk-client-windows, pf-client-core and punktfunk-client-session; both bins build; --discover finds the LAN hosts; the GUI shell comes up (WinAppSDK bootstrap intact under the new reactor-setup). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-29 18:31:47 +02:00
enricobuehlerandClaude Fable 5	ddd0e5cc5d	fix(vdisplay/kwin): primary is a PRIORITY, and the log now reads the answer back ... A field report (CachyOS, KWin 6.7.3, Plasma): topology primary streams the new virtual display but the desktop stays on the physical — while the host logs primary_taken=true on every apply. Both halves were real: * KWin's handler for kde_output_configuration_v2.set_primary_output is literally '// intentionally ignored' (verified in the 6.7.3 source). The output order is driven by per-output set_priority (management >= 3; we bind up to v22 and never called it). Exclusive only ever LOOKED right because disabling every other output leaves KWin nothing else to promote. Now: ours gets priority 1 and every other enabled output is renumbered uniquely behind it, exactly what kscreen-doctor does — the reporter proved that path applies and sticks on their box. set_primary_output is still sent for pre-v3 compositors. * primary_taken echoed the REQUEST. The log now waits one sync barrier for KWin's post-apply priority events and reports primary_verified from the read-back — a request the compositor ignores can never ship a green log again; the mismatch is a WARN naming the symptom. First-slot-wins refinement the working priorities made necessary: a SAME-NAMED output is this slot's own predecessor mid-supersede (mode switches create the replacement before dropping the old), not a sibling whose primary must be respected — deferring to it handed primary to whatever KWin promoted when the predecessor vanished a moment later. On-glass (nested KWin 6.7.3): fresh create, high-refresh custom-mode create, and the mode-switch supersede all read back verified=true; exclusive still disables + restores the other output in-process. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-29 17:59:24 +02:00
enricobuehlerandClaude Fable 5	751fbd6c47	fix(host/linux): park the seat pointer on the streamed surface — a capture-mode client cannot ... ROOT CAUSE of the GNOME capture-mode cursorless stream (and the same latent hole on KWin, whose screencast also gates cursor data on includesCursor): a virtual output is created fresh per session while the seat pointer stays wherever it was — usually a physical monitor. A capture-model (pointer-lock) client sends only RELATIVE deltas, so nothing ever moves the pointer INTO the streamed output: input lands on the wrong monitor, Mutter suppresses SPA_META_Cursor entirely (should_cursor_metadata_be_set: visible AND in-stream), and both the embedded and the encoder-blend composite models stream cursorless. The stream loop now parks the pointer at the streamed surface's centre through the session's own input pipeline (capability routing, region ladder, anchor — the path client events take): armed per (re)built display and by the capture-model flip, retried on a schedule because the first park of a session can land on a still-cold EIS connection (devices not resumed — the injector drops it; observed on-glass), and kept trying while a capture-model session still has no live overlay. A desktop-model client overrides the park with its first absolute move. Verified end-to-end on GNOME 50.3/Mutter (RTX 5070 Ti): the reference client in capture mode now shows the host-composited cursor moving in the decoded H.265 dump, and the PyroWave session hands the overlay to the wavelet CSC blend. One-shot per-session/per-stream breadcrumbs (first meta region, first bitmap, first overlay handed to the blend / still-cursorless) make the next field triage a grep instead of a rebuild. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-29 17:59:03 +02:00
enricobuehlerandClaude Fable 5	39aa0c57ce	fix(inject/libei): a scaled output's region is the streamed mode in logical pixels — match it ... A display scale s shrinks an output's EI region to logical pixels (Mutter advertises 853x533 for a 1280x800 output at 1.5), so the exact-size rung missed every scaled output and absolute input fell through to regions.first() — the wrong monitor whenever another region sorts first (on-glass: a lingering sibling virtual display). New rung between exact and first: one consistent scale factor (1..=4, fractional included) must map the region onto the mode on both axes, with per-axis rounding slack. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-29 17:59:03 +02:00
enricobuehlerandClaude Fable 5	944c03dd32	feat(client): the desktop clients wear the host's OS mark ... The client half of the host's new `os=` advert, shared once in pf-client-core: `sanitize_os` (mDNS is unauthenticated input — lowercase `[a-z0-9._-]` tokens, capped) and `os_icon_tokens`, the most-specific-first walk with the brand aliases (`macos`→apple, `steamos`→steam) every platform resolves through. `DiscoveredHost` carries the chain, `KnownHost` persists it (`serde(default)`, elided when empty — older stores load unchanged and older clients read back exactly what they wrote), `upsert` moves it only when carried, and `learn_os` mirrors `learn_mac` — no-op, no disk write when unchanged — so the mark survives the host going to sleep. GTK shells: the card's status row leads with a recolorable symbolic glyph. That needed real embedded assets — the shells had none — so `data/` gains the ten `pf-os-*-symbolic` SVGs compiled into a gresource (new build.rs, glib-build-tools) and registered on the icon theme at startup; the Adwaita theme then tints them like every other status glyph. WinUI shell: reactor renders raster-from-URI only, so the embedded mid-gray PNGs (legible on both themes) materialize once into %LOCALAPPDATA%\punktfunk\os-icons\ — the library's poster-art pattern — and the tile's status row leads with a 16px image. The couch UI plumbs `HostRow.os` (live advert preferred, else the store) for a Skia glyph that is a declared follow-up; `--list-hosts` / `hosts --json` emit the stored chain so the Decky plugin can read it. A host that advertises no `os` renders everywhere exactly as it did before the field existed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-29 17:58:33 +02:00
enricobuehlerandClaude Fable 5	4c2a6dd091	feat(host,api): the host knows what OS it runs, and says so ... Clients want to show which OS/distro a host runs. The host now detects it once per process (new osinfo module: compile-time on Windows/macOS, os-release on Linux) and advertises an icon-friendly specificity chain, generic → specific — `windows`, `macos`, `linux[/<family>][/<id>]`, e.g. `linux/fedora/bazzite`, `linux/arch/steamos`. A client walks the chain most-specific-first and shows the first token it has art for, so an unknown distro degrades to its family's mark and finally to plain Tux — the host emits the full chain precisely so clients need zero distro→parent knowledge. The middle token is the first recognized ID_LIKE ancestor (the spec orders them most-similar-first); the leaf is ID verbatim, sanitized to TXT-safe `[a-z0-9._-]` because it feeds a DNS record. Two carriers, both additive: a new advisory mDNS `os=` TXT key (same trust posture as `mac` — unauthenticated, a wrong value only draws a wrong icon), and `HostInfo.os` + `HostInfo.os_name` on the mgmt API (`os_name` is the os-release PRETTY_NAME, REST-only so the TXT stays small). GameStream serverinfo and the QUIC Welcome are untouched. api/openapi.json regenerated (drift test green on Linux) and the TS SDK gen refreshed from it — which also catches the committed gen up with earlier spec changes it had missed (monitors, native_paired_clients, encoder_backend). assets/os-icons/ gains the ten master SVG marks every client derives its per-platform art from (Font Awesome Free brands CC BY 4.0 + Simple Icons CC0 — provenance in its README), with attribution folded into THIRD-PARTY-NOTICES.txt via the generator (regenerating also catches the stale crate manifest up with the current lockfile). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-29 17:58:33 +02:00
enricobuehlerandClaude Opus 5	b0ea1e6b51	fix(client/linux): the override marker appears on touch, profiles get a colour, and 4:4:4 gets a switch ... Three things found by actually driving the client. **The marker didn't appear until you reopened the dialog.** It was rendered once, at build time, from the stored overlay — so changing a setting inside a profile looked like it did nothing. The design says touching a control creates the override and the marker appears immediately, and it has to: a user who changes a row and sees no acknowledgement has no reason to believe it took. Every profileable row now builds its dot and reset hidden, and the same handler that records the touch reveals them. Resetting a row touched in the same sitting also un-touches it, so the commit can't re-write what the reset just removed. **Profiles had no colour.** `accent` has been in the schema since P0 and nothing could set it, which left every chip the same grey — and telling profiles apart at a glance across a grid is the entire reason chips exist. Creating a profile now picks a colour in the same breath as its name (hunting for it afterwards is what leaves them all grey), an existing profile has a Colour row, and host-card chips are tinted with it. A palette of eight rather than a free picker: legibility across light and dark is the job, and the schema still accepts any `#RRGGBB` a hand-edit or a future picker writes. Anything that isn't `#RRGGBB` is refused rather than interpolated into CSS, and each distinct colour registers one display-wide rule (per-widget providers are gone since GTK 4.10). **4:4:4 had no switch anywhere but Apple.** `VIDEO_CAP_444` has been on the wire for a while with only `punktfunk-probe`'s env var to set it. It is now a setting — and a profileable one, which is the point: full chroma is what makes small text and thin UI lines crisp, so a "Work" profile wants it where "Game" usually doesn't. The host still gates it on its own policy, HEVC, and a GPU that can actually encode it; advertising only says "I can decode this and I want it". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 12:38:53 +02:00
enricobuehlerandClaude Opus 5	9243a770bc	fix(host/encode): an Automatic bitrate follows the pixels actually encoded ... `build_pipeline` opens the encoder at the DELIVERED frame size but was handed a bitrate resolved from the NEGOTIATED mode. Those agree for a virtual display, which is created at the mode that was asked for. They do not agree for a monitor mirror: `MirrorDisplay::create` ignores the requested mode by design (a physical head runs at the mode its owner set), so a client asking for 1080p and mirroring a 4K panel encoded four times the pixels at the 1080p rate — a quarter of the bits per pixel the codec was sized for, which arrives as a soft, stuttering picture rather than as the mismatch it actually is. This is the reported "laggy mirror". Re-resolve for what will really be encoded, and thread the applied rate back out so the session adopts it: `bitrate_kbps` is what the ABR controller climbs from, what the console samples, and what a `SetBitrate` ack is measured against, so letting it disagree with the live encoder makes each of those reason about a stream that does not exist. The mid-stream rebuild paths (mode switch, compositor switch, capture-loss) adopt it too — each can land on a different source than it left. Automatic only. An explicit client rate is the operator's statement about their own link and is never second-guessed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 12:23:32 +02:00
enricobuehlerandClaude Opus 5	664c26cb2b	fix(vdisplay): a pin with no heads to mirror must not refuse the session ... The streamed-screen pin is host-wide and persisted, but the session it applies to is not. A box that boots between a desktop (heads to mirror) and a nested or headless Game Mode (none) carried the pin into a session where `monitors::resolve` could only fail — and since `vdisplay::open` is the one place every session opens a display, that failure was a host refusing to stream at all rather than one streaming the normal way. Pinning a monitor in Desktop mode bricked Game Mode. A compositor reporting NO heads whatsoever now degrades to the virtual-display path with the reason logged. Narrow on purpose: a pin that misses among heads that DO exist stays the hard error §5.2 makes it — showing someone a different screen than they asked for is the failure worth refusing over, and "there are no screens here" is not that. An enumeration ERROR also stays on the mirror path, so the session fails with the real reason instead of quietly ignoring the operator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 12:23:32 +02:00
enricobuehlerandClaude Opus 5	028462ef6d	feat(vdisplay/gamescope): a Game Mode session drives a real head — list it, and mirror it ... The per-monitor capture feature shipped with gamescope written off as "nested, therefore no physical heads of its own", and `monitors::list` returned a hard-coded empty Vec for it. That premise holds for a gamescope nested in another compositor and for the headless ones this crate spawns — but not for the deployment people actually stream from: a Bazzite/SteamOS Game Mode session is the DRM master and drives the TV directly. On such a box the console's picker was permanently empty, on exactly the hardware the feature was asked for. `gamescope/heads.rs` answers the narrow question instead — is this gamescope on the DRM backend, and which connector is it lighting? — from `/proc/<pid>/cmdline` (the backend flag and `--prefer-output`; gamescope publishes no protocol that reports its own output) and `/sys/class/drm` (what is plugged in, plus each connector's EDID for the picker label and the panel's preferred timing). The nested/headless shapes still answer an empty list, which is what they always meant. Mirroring needs no new capture code: gamescope composites its one head into a PipeWire node it already publishes, so the mirror arm is an ATTACH to that node — the gaming session is not stopped, relaunched or re-moded, which is the whole difference between "stream the panel I am looking at" and the MANAGED takeover that deliberately blanks it. Refresh comes from the EDID and never from `--nested-refresh`: that flag is the rate gamescope composites the nested game at, and reading it as the connector's rate would let a session launched at 60 cap a 120 Hz TV for the whole stream. Verified against home-bazzite-1's live EDID: manufacturer `1e6d` -> GSM, name descriptor -> "LG TV SSCR2", preferred timing 594 MHz / 4400x2250 -> 3840x2160@60. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 12:23:32 +02:00
enricobuehlerandClaude Opus 5	a49858f194	feat(packaging/windows): give the drivers one publisher identity, and give the trust back ... The drivers had no cryptographic identity at all. Every build minted a fresh `CN=punktfunk-driver` cert (build-pf-vdisplay.ps1, build-gamepad-drivers.ps1), and install.rs `trust_cert` adds whatever .cer it finds in the unpacked bundle to machine Root AND TrustedPublisher. So the signature vouched for nothing an attacker couldn't restage — replace the bundle, ship your own cert beside your own driver, install proceeds identically. It was ceremony to make PnP install quietly. Worse, it leaked. `trust_cert` runs once per driver (install.rs:102 and :155), so every upgrade added TWO more self-signed root CAs under the same name, and nothing ever removed them: uninstalling punktfunk left trust behind that the user had no reason to keep granting. So: both build scripts now take a stable cert via DRIVER_CERT_PFX_B64 (they already read the env var — windows-host.yml just never passed it) and fail closed on a v* tag, same rule as the host and MSIX packers. `driver uninstall` purges every `CN=punktfunk-driver` cert from both stores, and `driver install` purges before adding, so an upgrade also collects the historical pile instead of adding to it. Purge-before-add lives ONLY on the pf-vdisplay install path, not the gamepad one. The installer runs vdisplay first and gamepad second; purging in both would have the gamepad leg delete the cert the vdisplay leg just added whenever the two bundles carry different certs — which is exactly what canary's per-build fallback produces. Purging by subject rather than thumbprint is deliberate too: it is what lets one install clean up certs from builds that no longer exist anywhere, and it needs no parsing of certutil's localized output (this module exists because locale-parsed PowerShell broke the driver install on a German box). This does NOT make the driver download authenticated — a self-signed leaf is its own root, so the installer must trust it for the driver to install at all. What it buys is a fingerprint we can publish out-of-band so a substituted driver is detectable, an allowlistable publisher, continuity across releases, and no root accumulation. Attestation signing remains the real fix; documented as such. Documented the key-custody trade honestly in packaging/windows/README.md: a stable key trusted as a machine root on every install is worth stealing in a way a throwaway never was, with no revocation path. CI secret only. ⚠️ The secrets must exist before the next v* tag or the release will fail — that is the guard working, and the generation runbook is in that README. The fingerprint line there is a placeholder until the key is generated. Verified: punktfunk-host compiles clean on the windows-amd64 runner with this install.rs (exit 0, no warnings); both build scripts parse; cargo fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 12:12:21 +02:00
enricobuehler	bcfb833ff7	fix(abi): the panic boundary was documented as universal and wasn't ... `abi.rs`'s header states "panics never cross the boundary: every entry point is wrapped in `catch_unwind`". Of its 78 `extern "C"` entry points, 16 were not. Ten of those are fine and were always fine — `punktfunk_abi_version` returns a constant, and the nine `punktfunk_connect*` shims forward every argument unchanged into a guarded implementation — but five ran real code bare: `session_free`, `connection_close`, `connection_disconnect_quit`, `reanchor_gate_free`, `reanchor_gate_arm`. The three `*_free`/`close` ones are the point. They run `Drop` for an entire `Session` or `Connection` — transports, threads, mutexes — and a `Drop` impl that unwraps a poisoned lock panics. Since Rust 1.81 that unwind is a hard abort rather than undefined behaviour, so this is not a soundness hole; it is worse-behaved than it looks. Aborting the CALLER'S process because one of our teardown paths hit a poisoned mutex is not an acceptable failure mode for a library, and it would present as "the app died in punktfunk_session_free" with no Rust backtrace to explain it. Adds `guard_void`, the sibling of `guard` for entry points with no status to report through: it catches, logs, and returns — right for teardown, where the object is going away regardless. The five are wrapped in it. The header now says what is actually true, including WHICH entry points are deliberately bare and why, so the next reader can check the claim instead of trusting it. That is the same failure this sweep found in `MappedView`'s `Sync` proof: a stated invariant that a reviewer would rely on and that had quietly stopped holding. Verified: Linux .21 fmt + both CI clippy steps rc=0, and the C ABI harness — which exercises the `*_free`/`close` paths it touches — still passes, 4 frames byte-exact through lossy loopback.	2026-07-29 08:48:42 +02:00
enricobuehler	aa070f2d55	feat(ffi): hand-mirrored C structs are now layout-checked at compile time ... The sharpest memory-safety risk left in this codebase is not an `unsafe` block — it is a hand-written `#[repr(C)]` mirror of an external C struct. Get a field offset wrong and nothing fails to compile and nothing reliably crashes: the library reads a pointer, a length or a pitch out of the wrong bytes. Eleven such structs across five files had NO check at all. Guarded here, each next to the struct it protects: * `AVCUDADeviceContext`, and `AVD3D11VADeviceContext`/`AVD3D11VAFramesContext`. `ffmpeg-sys-next` binds none of them, so these mirrors are the only definitions — and we WRITE through them (`cuda_ctx`, `device`, `bind_flags`). ⚠ The D3D11VA pair is duplicated VERBATIM in two crates (pf-encode's `ffmpeg_win.rs`, pf-client-core's `video_d3d11.rs`) because neither can depend on the other; they must agree with libav and with each other, and now a drift in either is a build error. * The six cuda.h structs. Three were already asserted — but only in `#[cfg(test)]`, so the check ran when someone ran the tests and never in a release build. They are `const` now. The other three, including `CUDA_MEMCPY2D` which is filled on EVERY zero-copy frame, had nothing. * `MsghdrX`, Darwin's `msghdr_x`, which `libc` does not expose. Its layout is not reviewable by eye: the 32-bit fields force padding before each following pointer, so `msg_iov` sits at 16 and not 12. `sendmsg_x`/`recvmsg_x` take the pointer and length from it. * `IPolicyConfigVtbl` — the sharpest of the set. It mirrors an UNDOCUMENTED COM interface, and `set_default_endpoint` is called by SLOT INDEX through a ten-entry `_reserved` gap that carries no names to anchor a review. A field added or resized above it does not break the build; it calls a different function pointer through a mismatched signature. Every assertion is `const _: () = assert!(..)`, so it holds on every build including release and cannot be skipped. The compiler verified the numbers — the sizes and offsets asserted here are the ones the target actually produces, on each platform that compiles the struct. Verified: Linux .21 fmt + both CI clippy steps rc=0 (CUDA + libav CUDA mirrors); Windows .47 full CI clippy set rc=0 + pf-capture tests (D3D11VA pair, COM vtable); macOS `cargo check -p punktfunk-core` (MsghdrX — the only platform that compiles it).	2026-07-29 08:48:42 +02:00
enricobuehler	8a5a5edc37	fix(small crates): the proof lint now covers every first-party crate but one ... Six crates were still unguarded, and all six are OURS — none vendored: pf-console-ui, pf-gpu, punktfunk-tray, clients/windows, tools/display-disturb, wdk-probe. Two of them ship (the tray and the Windows client), so "small" was about item count, not exposure. Five are closed here. Only 17 of their 75 unsafe items actually lacked a proof — pf-gpu, punktfunk-tray and display-disturb were already fully documented and needed nothing but the deny, which is the good case: the convention was being followed, just not enforced. The 17 that were missing are the usual Win32/COM shapes, and two were worth stating properly. `clients/windows`'s `GetCurrentPackageFullName` is called with `len = 0` and no buffer — that is the documented identity PROBE, which writes nothing, and reading it as a normal query would be a mistake. `pf-console-ui`'s two `destroy_image_view` calls are the load-bearing ones: the comment above one already argued that in-flight sampling of that slot ended two presents ago (the ring alternates and the presenter waits its fence before each record), which is exactly the kind of reasoning a `// SAFETY:` should carry and it was sitting there unlabelled. Also fixes a real Windows-only clippy error this uncovered: `pf-gpu` had a `#[cfg(target_os = "windows")]` fn AFTER its `mod tests`, tripping `items_after_test_module`. It never fired on Linux (the item does not exist there) and no CI job clippies pf-gpu on Windows, so it sat unseen. Moved above the test module. Remaining: `wdk-probe` (26 items) alone, and only because it needs the WDK to build — .47 cannot, so nothing here can verify a deny on it. Verified: Linux .21 fmt + both CI clippy steps rc=0; Windows .47 the four Windows-relevant crates at `-D warnings` rc=0.	2026-07-29 08:48:42 +02:00
enricobuehler	65cd388a52	fix(presenter,core): close the last of the proof-lint hole — the Vulkan contract, stated once ... pf-presenter's 120 sites are `ash` calls almost without exception, so 120 independent arguments would have been 120 restatements of the signature — the exact noise this program exists to remove. They get the `abi.rs` treatment instead: the Vulkan contract stated once in `lib.rs`, each site naming which of three shapes it is. The three are not equal, and separating them is the point. CREATE and RECORD carry no real precondition — the device is owned, the builders are locals, nothing executes until submit. DESTROY does: the GPU must not still be using the object, and that is established by the path (a fence wait, a `queue_wait_idle`, a retired swapchain), not by the call. Those sites say so, because getting it wrong is a use-after-free no type catches. The contract also tells the next person that a block outside the three shapes needs a real proof, and that writing "as above" is the signal it doesn't belong in them. punktfunk-core's Windows half is finished here too: `qos_windows.rs`'s `GetLastError` reads (called before anything can reset the thread's error slot) and `udp/windows.rs`'s control-message write, whose argument is that `ctrl` is sized by `WSA_CMSG_SPACE(4)` — computed two lines up — so header plus payload cannot run past it, and `write_unaligned` is used because `WSA_CMSG_DATA` offers no alignment guarantee. ⚠️ THE WINDOWS BLIND SPOT BIT A THIRD TIME. A Linux measurement put this crate pair at 113; the real number was 129 — `d3d11.rs`, `win32.rs`, `qos_windows.rs`, `udp/windows.rs` are all `cfg`-hidden. Every crate in this sweep had to be finished on .47 after being "done" on .21. For a cross-platform crate the Linux number is a lower bound, never the answer. All three crates now deny `undocumented_unsafe_blocks`, which was the goal: it applied to 8 of 11 crates carrying unsafe, and the three exempt ones held 381 items between them — including the C ABI surface and the presenter. Verified: Linux .21 fmt + both CI clippy steps rc=0; Windows .47 all four closed crates clippy `-D warnings` rc=0 plus the full Windows CI clippy set and pf-capture's tests. Only small crates remain unguarded (75 items total, largest 26).	2026-07-29 08:48:42 +02:00
enricobuehler	dda9ed1aa2	fix(core): close the proof-lint hole in punktfunk-core — the ABI contract, stated once ... 146 sites, 141 of them in `abi.rs`, the `extern "C"` surface `cbindgen` turns into `punktfunk_core.h`. Writing 141 independent arguments would have been the wrong answer and would have read like it: they are instances of ONE contract in six shapes — opaque handles reached only through `as_mut()`/`as_ref()` (so null becomes a status, never a dereference), caller-owned out-params (null-checked exactly where the header calls them optional), NUL-terminated-or-null C strings through `opt_cstr`, unchanged forwarding to a versioned entry point, fixed-length output buffers, and `addr_of!` reads. So the contract is stated once at the top of the file, the way `pf-win-display`'s CCD contract is, and each site says which instance it is. Two properties that hold everywhere are stated there and not repeated 141 times: no pointer is retained past the call that received it, and every entry point runs inside `guard`'s `catch_unwind`, so a panic becomes a status code instead of unwinding into C. The `addr_of!` sites got a real proof rather than a shape, because that one is load-bearing and non-obvious: it forms a raw pointer WITHOUT creating a reference precisely because the caller's struct may be an older, smaller version, so the field must be read by offset and not through a `&`. `transport/udp/linux.rs`'s five are genuinely individual — two `zeroed()` POD initialisations, and the `sendmmsg`/`recvmmsg`/`CMSG` trio, where the argument worth writing down is that `msg_controllen` is set to `CMSG_SPACE(size_of::<u16>())` and the `CmsgBuf` is 64 bytes, so the kernel cannot write past the control buffer. punktfunk-core now denies `undocumented_unsafe_blocks`. Verified: Linux .21 fmt + both CI clippy steps rc=0, and — since this is the C surface and comments alone should not change it — the C ABI harness still passes, 4 frames round-tripped byte-exact through lossy loopback.	2026-07-29 08:48:42 +02:00
enricobuehler	e1ddd49e37	fix(client-core,ffvk): close the proof-lint hole in two of the three unguarded crates ... `clippy::undocumented_unsafe_blocks` is what makes the SAFETY convention a rule rather than a habit, and three crates had never adopted it — pf-client-core (91 unsafe items), pf-presenter (123) and punktfunk-core (167) — while every other subsystem crate denied it. That gap is why the decoders' `unsafe impl Send`s carried a one-line aside instead of an argument: nothing required one. pf-client-core and pf-ffvk now deny it, with a proof written for all 58 + 3 sites they had. ⚠️ 44 of those 58 were WINDOWS-ONLY — `clipboard.rs` 24 and `video_d3d11.rs` 20 — and invisible to the Linux measurement that sized this work at 14. Same trap as the E0133 sweep: a Linux-only survey of a cross-platform crate undercounts by whatever the `cfg` hides, here by 3x. Landing the deny on the strength of that number alone would have re-broken Windows CI, which is exactly the mistake this session already made once with the `warn`-that-was-really-`deny`. The proofs say what is actually load-bearing rather than restating the call. In `clipboard.rs` that is the ownership split Win32 requires and nothing in the code stated: `GetClipboardData` returns a handle BORROWED from the clipboard (never freed here), while `GlobalAlloc` + `SetClipboardData` TRANSFERS ownership to it (which is why nothing frees that one either) — two opposite rules, three lines apart. In `video_d3d11.rs` the recurring one is that libav's `get_format` list is NUL-terminated by `AV_PIX_FMT_NONE`, which is what keeps the walk in bounds. Remaining: punktfunk-core (~146, of which `abi.rs` is 141) and pf-presenter (~108). Both want the "state the contract once" treatment — abi.rs's sites are a handful of repeating shapes (`opt_cstr` on caller C strings, null-guarded out-param writes, forwarding calls), not 141 distinct arguments. Note the vendored `fec-rs` (18 sites) is a separate path-dependency crate, so it is out of scope rather than something to prove. Verified: Linux .21 fmt + both CI clippy steps rc=0; Windows .47 `-p pf-client-core` clippy `-D warnings` rc=0 (the only place the 44 are visible), plus the full Windows CI clippy set and pf-capture's 18 tests.	2026-07-29 08:48:42 +02:00
enricobuehlerandClaude Opus 5	f32c3aaa71	feat(session): spec mode — the renderer stops resolving policy and stops writing settings ... C2 of design/client-architecture-split.md, and the last of the session's overreach. `--resolved-spec <path>` hands the session everything it needs already resolved — effective settings, the host's clipboard decision, the profile's name — and in that mode it performs ZERO store reads. It had been re-deriving all three, which meant policy was being evaluated inside the thing that draws pixels, and that the spawner and the child could disagree about a file either of them might have written in between. First-party spawns (the shells, the CLI) always pass one now. The compat path stays for hand-run `punktfunk-session --connect` and old Decky scripts — but it calls the SAME helper, so the two modes cannot drift; it is the identical function invoked in-process instead of by the parent. A spec that is named but unreadable fails loudly rather than quietly falling back: a spawner that asked for exact settings must not get store-derived ones instead. The match-window write-back is gone too. The callback used to load-modify-save the shared settings file from inside the renderer — one of that file's five concurrent writers, for a value only the parent needs. It now reports `{"window":{w,h}}` on stdout and the spawner persists it, on a real change only. A hand-run session still persists its own window, because nobody is listening to its stdout there and the event alone would drop the value. Verified on .21: a spec naming a profile that doesn't exist in the catalog is honoured (proving no lookup happened), a missing spec errors instead of falling back, and the CLI's spec file is written and cleaned up per launch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 01:16:17 +02:00
enricobuehler	d2b6f5b65f	docs(unsafe): audit all 49 unsafe impl — one proof was wrong, four were missing ... `unsafe impl Send`/`Sync` is the highest-risk unsafe category here and the one this program had never looked at: a wrong one is cross-thread UB that is invisible at every call site, with no `unsafe` block to catch a reviewer's eye. 49 of them (41 Send, 8 Sync). Two results. **`MappedView`'s `Sync` proof was factually wrong.** It read "only exposes accessors that are safe under concurrent use" — they are not. `read_u8`/`write_u8`/`read_u16` are plain unaligned accesses through `&self`, and `&MappedView` really is shared across threads: `ChannelState::data()` hands out `&'static MappedView`, and pf-xusb, pf-mouse and pf-gamepad all dispatch `WdfIoQueueDispatchParallel` with `NumberOfPresentedRequests = u32::MAX`. The struct's own doc had the right story — consistency is the channel protocol's job — but the `unsafe impl` stated a different, stronger claim, which is the one a reviewer checking that line would rely on. The impl is still sound, for a reason worth writing down: these bytes are mapped into ANOTHER PROCESS that writes them concurrently, so Rust-level exclusivity over them is unachievable no matter what this type does. Sync fields go through the atomic accessors; the plain ones cover only protocol-fenced bytes. The proof now says that, and states the rule it implies for accessors added later — plain path only for bytes the protocol already fences. **Four `Send` impls carried a one-line aside instead of a proof** — the pf-client-core decoders and `DrmFrameGuard`. All four are sound, and each now says why, including the two facts that make them work and were nowhere stated: libav permits a codec context to be used from a thread other than its creator provided use is serialised (`&mut self` is that serialisation), and D3D11's immediate context is thread-AGNOSTIC rather than thread-safe — it wants serialised use, not one fixed thread. Each also records that it is deliberately not `Sync`, which is the invariant a future `impl Sync` would silently break. Every one of the 49 now carries reasoning. Verified: Linux .21 fmt + both CI clippy steps rc=0; Windows .47 `-p pf-client-core` clippy `-D warnings` rc=0 (it compiles the `video_d3d11` proof the Linux run cannot see). The pf-umdf-util edit is comment-only — confirmed by diff, since that crate needs the WDK, which .47 does not have.	2026-07-29 00:37:14 +02:00
sassycrownandOpenAI Codex	22936bbc89	fix(vaapi): use FFmpeg bt2020nc matrix name ... FFmpeg rejects bt2020 as an out_color_matrix value. Use its canonical bt2020nc name for non-constant-luminance BT.2020, matching the existing AVCOL_SPC_BT2020_NCL encoder VUI. Co-Authored-By: OpenAI Codex <noreply@openai.com>	2026-07-29 00:26:57 +02:00
enricobuehler	60a85a1344	refactor(encode/windows): fourth fence off — ffmpeg_win.rs, and D3d11Hw::new joins VaapiHw ... 47 sites, 25 of them raw pointer dereferences: the same libav shape as `vaapi.rs`, so the same verdict — a proof here carries an argument rather than restating a call. `D3d11Hw::new` loses its marker for exactly the reason `VaapiHw::new` did, and the trio is now the clearest statement of the rule in the tree: `CudaHw::new` KEEPS it (handed a `CUcontext`), `VaapiHw::new` and `D3d11Hw::new` do not (a borrowed COM wrapper and scalars, opening what they need themselves). Three near-identical libav context builders, sorted by whether a caller can hand them something broken. Also unmarked, all for the same reason — no parameter a caller can get wrong: `immediate_context` and `ensure_staging` (`&ID3D11Device` IS the live-device guarantee), `send` and `ensure_sws` (only scalars, operating on the `AVFrame`/`SwsContext` the struct owns from its constructor to `Drop`), and `test_hw_device`, whose `# Safety` section described its own body — "calls the DXGI enumeration FFI and `make_device`" — while taking a single `u32`. `open_win_encoder` keeps its marker (raw `*mut AVBufferRef` pair) and gains the proof it lacked, including the null case the system path relies on: both refs may be null, the `is_null` guards keep `av_buffer_ref` off that path, and the refs it does take are new ones the codec context adopts. `D3d11Hw::new`'s proof records the ordering the multithread-protection fix depends on — the device store must precede `av_hwdevice_ctx_init`, which reads it. 14 fenced files -> 10. Verified on the Intel box .47: `-p pf-encode --features nvenc,amf-qsv,qsv` clippy `-D warnings` rc=0, the full Windows CI clippy set rc=0, pf-capture's 18 tests pass, and — because this is the live D3D11VA construct path, not dead code — `d3d11hw_alloc_drop_cycles` passes on real Intel silicon: 8 construct/drop cycles, no abort. Linux .21: fmt + both CI clippy steps rc=0.	2026-07-29 00:20:35 +02:00
enricobuehler	8e7ba00d2d	refactor(encode/linux): third fence off — vaapi.rs, and VaapiHw::new needed no marker at all ... Same criterion as the last two: `vaapi.rs`'s sites are pointer dereferences and libav ctx calls, not ash, so a proof here carries an argument. Three regions, three arguments. `VaapiHw::new` also loses its `unsafe fn` outright, and the contrast with its CUDA twin is the whole point: `CudaHw::new` keeps the marker because it is HANDED a `CUcontext` the caller must vouch for, while this one takes four scalars and opens the VAAPI device itself. Two functions of near-identical shape, opposite answers, decided by whether a caller can supply something broken. The other two regions keep their markers (`open_vaapi_encoder`/`_mode` are handed raw `*mut AVBufferRef`s) and gain the proofs they lacked. The encoder-config block now records the fact that makes it sound rather than obvious: `av_buffer_ref` returns a NEW reference the codec context adopts, so the callee shares the caller's device/frames buffers instead of consuming them — which is also why the low-power entrypoint ladder can retry with the same two pointers after a failed attempt. The frames-pool block reuses the `CudaHw::new` argument: alloc returns null-or-initialized and `AvBuffer::from_raw` rejects null, so the `?` leaves before any field store can run. One call site dropped its `unsafe {}` and its comment with it — the comment argued that libav was initialized, which is a real precondition of the call but not one a caller can violate, so it is now a note rather than a contract. 14 fenced files -> 11. Verified on .21 (fmt + both CI clippy steps rc=0) and, because this is the live VAAPI construct path rather than dead code, ON THE AMD 780M (.116, pf-build distrobox): `vaapi_cpu_encode_smoke`, `dmabuf_inner_alloc_drop_cycles` and `vaapi_probe_smoke` all pass — 3 passed / 0 failed, H265 + AV1 probes still true in both 8- and 10-bit.	2026-07-29 00:20:35 +02:00
enricobuehler	ee29b3c3a9	refactor(client-core): three decoder lift methods that took nothing to get wrong ... `D3d11Decoder::lift`, `VaapiDecoder::map_dmabuf` and `VulkanDecoder::extract` each take `&mut self` and NOTHING else. They dereference `self.frame`, the `*mut AVFrame` the decoder allocates in its own constructor and owns until `Drop` — a pointer no caller supplies, can replace, or can invalidate. None carried a `# Safety` section, and each body was already one `unsafe {}` with its proof, so the marker was pure ceremony and its removal moves nothing. All three live in files the workspace `deny` already covers (`video_vulkan.rs` since the fence came off it), so they stay fully enforced. Worth recording why this is where the marker sweep STOPS for these two crates: every remaining candidate in pf-encode and pf-client-core sits inside a fenced file, and there removing a marker is not free. The fence excuses `unsafe_op_in_unsafe_fn`, which only applies to `unsafe fn` BODIES — a safe fn always needs explicit blocks — so unmarking an ash-dense helper forces exactly the whole-body `unsafe {}` this program rejected when it rejected `cargo fix`. The way to reach those is to reclaim the file first, not to unmark inside it. Verified on .21: fmt + `clippy --workspace --all-targets -- -D warnings` + the feature-gated `-p pf-encode` step, all rc=0.	2026-07-29 00:20:35 +02:00
enricobuehler	d72822ced7	refactor(encode/linux): second fence off — CudaHw::new's pointer walk gets its proof ... `linux/mod.rs`'s fifteen sites are the same kind as `video_vulkan.rs`'s, not the ash kind: nine raw pointer dereferences and six libav calls, all inside `CudaHw::new`, which had no `unsafe` block and therefore no proof of the one thing worth proving here — that the pointer chain it walks is live. The marker STAYS (`cu_ctx: *mut c_void` is a `CUcontext` the caller must supply valid). The body is now two blocks, one per phase, because there are two distinct arguments to make. Both turn on the same non-obvious fact: `av_hwdevice_ctx_alloc`/`av_hwframe_ctx_alloc` return null or a ref whose `data` libav has ALREADY initialized, and `AvBuffer::from_raw` rejects null — so the `?` leaves before any of the field stores below it can run. That is what makes the `(*dev_ctx)`/`(*fc)` writes in-bounds stores on live allocations rather than a hope, and it is exactly the reasoning that was missing. The device block also records the ordering constraint that was implicit: `cuda_ctx` must be stored BEFORE `av_hwdevice_ctx_init`, which reads it. Two files now need no exemption: 14 fenced -> 12. Both were removable for the same reason — their sites are pointer dereferences, where a proof carries an argument, unlike the ash backends where it could only restate the call. That is the criterion for which fence to attack next, not file size. Verified on .21: fmt + `clippy --workspace --all-targets -- -D warnings` + the feature-gated `-p pf-encode --features nvenc,vulkan-encode,pyrowave` step, all rc=0 with no allow in either file.	2026-07-29 00:20:35 +02:00
enricobuehler	ed4bbc6b0b	refactor(client-core): the first fence comes off — video_vulkan.rs is at zero ... `video_vulkan.rs` was fenced with the other thirteen GPU/FFI backends, but it never belonged with them: its four sites are not ash calls, they are RAW POINTER DEREFERENCES inside the two FFmpeg `lock_queue`/`unlock_queue` callback trampolines — exactly the case where the lint pays, and where a proof carries an argument instead of restating a signature. Both bodies had none at all. They keep `unsafe extern "C"`: FFmpeg invokes them with a `*mut AVHWDeviceContext`, which is a real contract, now written down. What was missing is why the dereference chain is sound, and it is worth stating because it is not obvious — the trampoline casts between pf_ffvk's `AVHWDeviceContext` and ffmpeg-sys's (the same C struct declared twice), reads `user_opaque`, and dereferences it as a `QueueLock`. That pointer borrows `VkCtxStorage::_queue_lock`, an `Arc<QueueLock>` whose field doc already says it exists to outlive every call the context can make. The proof now connects those two facts, so a future edit to the storage's lifetime has something to contradict. With that, the file needs no exemption and the workspace `deny` covers it: 14 fenced files -> 13. This is what removing a fence is supposed to look like — the sites that are worth narrowing get narrowed, and the exemption disappears rather than being renewed. Verified on .21: fmt + `clippy --workspace --all-targets -- -D warnings` + the feature-gated `-p pf-encode` step, all rc=0 with no allow in the file.	2026-07-29 00:20:35 +02:00
enricobuehler	5b2be889f9	refactor(capture/windows): two # Safety sections that stated no contract ... The previous commit took the markers with no `# Safety` at all. These two HAVE one, and that is what makes them worth naming: a section can exist and still describe nothing a caller can violate. `resolve_render_adapter` takes **no arguments**. Its section read "calls DXGI factory/adapter enumeration; returns owned COM objects or an error" — a summary of the body. With no parameters and no globals touched, there is no way to call it wrongly. `create_nv12` asked that "`device` must be a live D3D11 device". It takes `&ID3D11Device`, a borrowed reference-counted COM wrapper, so the borrow itself is that guarantee — safe Rust cannot produce a dangling one. The remainder ("the returned texture is owned by the caller") is an ownership note, the same distinction `service::open_log_handle` already draws in its doc. Every other parameter is a plain scalar. Both bodies already had their explicit block, so again nothing moved. `create_nv12`'s proof said "on the live `device` borrow (per the contract above)" and now says the borrow is what keeps it live — a proof that pointed at a contract being deleted had to stop pointing at it. The other four were read and KEPT, and they are the shape of a real one: `channel::send` and `duplicate_and_deliver` take raw `HANDLE`s; `prepare_blend_scratch` requires the caller to hold the slot's keyed mutex before it copies; `pyro_fence_signal` must run on the thread owning the immediate context, which is load-bearing precisely because `IddPushCapturer` is `unsafe impl Send` and so CAN be moved. `verify_is_wudfhost` (raw `HANDLE`, and the driver-channel security check) keeps its marker too. pf-capture: 21 `unsafe fn` -> 8, every survivor carrying an obligation a caller can actually break. Verified on .47: pf-capture clippy `-D warnings` rc=0 + 18 Windows tests pass, host/pf-encode/ pf-vdisplay green; Linux .21 fmt + both CI clippy steps rc=0.	2026-07-29 00:20:35 +02:00
enricobuehler	9e8473b46a	refactor(capture/windows): eleven D3D11 helpers had no caller contract ... The DXGI converters (`HdrP010Converter`, `BgraToYuvPlanes`, `VideoConverter`), the cursor blend pass, and `create_ring_slots` declared their methods `unsafe fn` because their bodies are D3D11 FFI. That is a property of the body, not an obligation on the caller, and their own proofs said so: "`?`-checked D3D11 methods on the live `device` borrow". Every parameter is either a borrowed windows-rs COM wrapper (`&ID3D11Device`, `&ID3D11DeviceContext`, `&ID3D11Texture2D`, the views) or a plain `u32`/`bool`/`DXGI_FORMAT`; each body builds its own descriptors from those and every interface it creates owns its reference. There is nothing a caller can pass that makes them unsound. Not one of the eleven carried a `# Safety` section — the marker had no stated contract to inherit, which is the tell. Each body was already one `unsafe {}` with its proof, so this is purely subtractive: no block moved and no proof was written. The four call sites lost `unsafe {}` wrappers whose own comments gave the game away — "`X::new` is `unsafe` (it compiles D3D11 shaders…)", "`create_ring_slots` is an `unsafe fn` (it makes D3D11/DXGI COM calls)" — comments explaining that the marker described the callee's body rather than anything the caller had to guarantee. `compile_shader` KEEPS its `unsafe fn`: it takes `PCSTR`, a raw pointer the caller must guarantee points at a NUL-terminated literal. That is the line — a contract a caller can actually break. pf-zerocopy was surveyed the same way and deliberately left alone: its CUDA/Vulkan helpers look identical by signature but state real obligations ("the shared context is current", "single-threaded use of handles this bridge owns"), so signature screening finds candidates and only reading decides. pf-capture already denies both `unsafe_op_in_unsafe_fn` and `undocumented_unsafe_blocks`, so the crate stays fully enforced; the reduction is in obligations, not in warnings. It now has exactly one `unsafe fn` left. Verified on the Intel box .47: pf-capture clippy `-D warnings` rc=0 and its 18 Windows tests pass, with host/pf-encode/pf-vdisplay clippy still green; Linux .21 fmt + both CI clippy steps rc=0.	2026-07-29 00:20:35 +02:00
enricobuehler	50531c8e9e	fix(host/display): a screen picker that cannot stream a screen now says so instead of saving ... The Windows console's "Streamed screen" picker SAVED and then did nothing. `capture_monitor` is platform-neutral, so the PUT persisted; every consumer of it is Linux-gated, so a virtual display was still created on connect. The operator got a control that acknowledged the click and changed nothing — the worst of the three possible behaviours. The root cause is not a missing gate, it is a missing BACKEND. Per-monitor capture is Linux/portal only: `vdisplay::open`'s mirror arm is `#[cfg(target_os = "linux")]` because `pf-capture` has no Windows entry point that can capture an arbitrary head. Its sole Windows entry point is `open_idd_push`, a frame channel pushed by our OWN IddCx virtual display; DXGI Desktop Duplication was deliberately REMOVED (`windows/dxgi.rs` keeps the GPU-preference hook only to stop DXGI reparenting the virtual display off the pinned adapter, and says so). So there is nothing for a pin to aim at, and exposing the picker got ahead of that. Decision of record (user, this session): mark it unsupported now rather than build the Windows backend here. A Windows mirror backend is a real feature gap and a project of its own — it needs a duplication capturer plus everything the IDD-push path carries today (cursor sidechannel, 444, 10-bit, PyroWave, HDR) re-plumbed through it — not a follow-on to an enumeration commit. Three places, so the answer is consistent wherever it is asked: * `MonitorsResponse.pin_supported` — a CAPABILITY, reported by the build that would have to honor the pin rather than sniffed from the OS client-side. When a Windows mirror backend lands this flips and the console needs no change. `pinned` stays `None` off-Linux deliberately, and now says why: it is what the console highlights as "sessions stream this", and highlighting a head nothing will capture is the same lie in a different place. * `enforced` drops `capture_monitor` off Linux. That list is exactly the "which controls are live vs. coming soon" contract, and claiming this one unconditionally is what let the picker ship enabled. * The PUT drops a non-Linux `capture_monitor` instead of storing it, and logs that it did. COERCED, not rejected with a 400: this PUT is WHOLE-OBJECT, so a host that already stored a pin would have every later settings save rejected over a field the operator cannot see — taking the other axes down with it. This way such a policy self-heals on the next write. Console: the picker renders read-only with an explanation, reusing the `envLocked` shape rather than inventing a second one (`locked = envLocked || !pinSupported`). Managed heads were ALREADY excluded from selection here (`mon.enabled && !mon.managed`), which is the "grey out, never filter blindly" rule working as intended — the heads stay listed and explicable. `pin_supported` defaults to TRUE when absent so an older host, which only ever shipped this picker where it worked, is not retroactively locked out. ⚠️ `api/openapi.json` is hand-patched — punktfunk-host does not build on macOS, so the spec could not be regenerated from the binary. The shape was verified by running the real generator over it: orval emits `pin_supported: boolean` (required, no `?`) on `MonitorsResponse`. Verified: `bun run codegen` (orval + paraglide + i18n parity: 434 messages, en + de), `bun run build`, `bun run lint` (tsc --noEmit) all clean. Rust verified separately on .173 — see the next commit's note if that lands, since neither xcheck target covers punktfunk-host.	2026-07-29 00:18:27 +02:00
enricobuehler	ed021a13ee	test(vdisplay): give the 3.2 case eyes — a subscriber, and the probe that splits the two candidates ... §5 3.2 reproduces on glass and the shipped fix is not sufficient: the panel stays dark after teardown. Two candidates were left undistinguished by the first on-glass run — the adopted snapshot was already poisoned, or the dark-desk backstop never fired — because the case had no way to tell them apart. This adds that way. **The subscriber.** The adoption arm and `restore_displays_ccd`'s backstop announce themselves ONLY through `tracing`, and a bare `cargo test` harness installs no subscriber, so both events went nowhere. `tracing-subscriber` becomes a DEV-dependency of pf-vdisplay (the shipped host's closure through this crate is unchanged) and `init_test_tracing` wires it: `try_init` so a second live case in the same binary is a no-op rather than a panic, `with_test_writer` so it interleaves with the harness rather than racing `println!`, `RUST_LOG` still winning over the `debug` default. `live_inplace_resize` gets it too — its comment claimed tracing-subscriber "is not a dep of this crate, run the host binary for traced runs", which was the same blindness written down as a limitation. It no longer holds. **The probe is the actual discriminator, and it needs no logs at all.** Member 1's isolate is INJECTED to fail, so nothing of ours deactivates anything there. Sampling `active_physicals()` immediately after member 1's create therefore answers the question directly: * empty -> the arriving IddCx monitor took the desktop on its own. Every snapshot from that instant on records "panel off", so member 2's adopted snapshot is POISONED AT BIRTH and restoring it faithfully restores darkness. Adoption works; the snapshot SOURCE is the defect. * non-empty -> poisoning is excluded and the break is downstream: no adoption line means teardown's restore was never gated on, a backstop line with a non-zero force-EXTEND rc means the remedy itself failed. The failing assertion now reads that verdict out instead of asserting one cause, so the next on-glass run reports which link broke rather than only that one did. ⚠️ Still UNRUN on glass — this is the instrument, not the measurement. Needs .173's CONSOLE session (ssh is session 0 and CCD sees nothing there), and it can leave the desk dark: recover with `SDC_USE_DATABASE_CURRENT|SDC_APPLY`, not EXTEND, which returns rc=31 against a single display. Checked while writing this, and recorded so it is not re-derived: the "backstop is structurally inert on a one-display box" theory does NOT survive. Both real call sites restore BEFORE the virtual is REMOVEd, so two displays are connected and the preset applies — `live_force_extend_*` measured exactly that (1 -> 1 -> 2). The documented residual is narrower: a restore that fails once the virtual is already gone. Verified: `xcheck.sh windows clippy` clean (it passes `--all-targets`, so this test code really is compiled), `cargo fmt --all --check` clean. ⚠️ `Cargo.lock` moves with this commit and must not be split from it. CI runs `cargo clippy --workspace --all-targets --locked`, so a new dependency edge whose lockfile entry is missing does not degrade to a re-resolve — it FAILS the build outright. The dev-dependency adds one line (pf-vdisplay gains a `tracing-subscriber` edge; the package itself was already in the graph via punktfunk-host and pf-encode, so nothing new is vendored).	2026-07-29 00:18:27 +02:00
enricobuehler	8381fd73ef	test(vdisplay): bound the 3.2 case's creates, and what it found on glass ... A hang must fail the case, not wedge the box. The first attempt at this case hung inside `create`; killing the harness skipped every `Drop` and leaked an IddCx monitor, and a few of those exhaust the driver's slot pool — after which every later run wedges too and only a reboot clears it. Both creates now run on a worker thread with a 45 s budget, so the harness exits NORMALLY, which is what lets the driver reap the session. Re-run with matched host+drivers: no hang, no leak, and the box came back with the panel lit. ⭐ With that guard in, §5 3.2 REPRODUCED ON GLASS for the first time (.173, LG TV attached, host 0.21.0 + drivers 9.9.728.2241, first isolate injected to fail): physicals before : [(4352, "LG TV SSCR2 [HDMI]")] after member 1 (isolate INJECTED to fail): [(260, "punktfunk [punktfunk-virtual]")] after member 2 (isolate REAL) : [(260, …), (264, …)] physicals during : [] physicals after teardown : [] <-- the operator's panel, still dark So the shipped 3.2 fix — adopt the first SUCCESSFUL isolate as the group's restore snapshot — is necessary but NOT sufficient. Two candidate reasons, both worth checking before changing code: * The adopted snapshot is already POISONED. The panel went dark at member 1's create even though our isolate never ran (the injected failure), because the arriving IddCx monitor took the desktop on its own. Member 2's isolate then snapshotted a topology in which the physical was ALREADY inactive, so restoring it faithfully restores "TV off". This is the same poisoned-snapshot chain §5 already records from the field logs. * `restore_displays_ccd`'s dark-desk backstop did not rescue it either. Worth its own look, since 21eda37a is what first made that condition evaluable at all (before it, our own display counted as a lit physical and the guard could never fire). ⚠️ Not yet distinguished, because this case installs no tracing subscriber: whether the adoption arm fired at all, and whether the backstop ran and failed or never ran. That is the next diagnostic — `live_inplace_resize` shows the pattern for wiring a subscriber into a live case. Also confirms 21eda37a in the field: the virtual displays report `tech=punktfunk-virtual` and are correctly absent from the physicals list, which is what makes "physicals after teardown: []" mean what it says rather than counting our own output as the operator's panel.	2026-07-29 00:18:27 +02:00
enricobuehler	22f2686680	feat(vdisplay/windows): the console can finally show the operator's real screens ... `GET /display/monitors` returned an empty list on every Windows host, plus a LINUX error string: `monitors::list` is a per-compositor dispatch whose non-Linux arm bails, and it never even got that far because `detect()` was not cfg-gated — on Windows it fell through to an `XDG_CURRENT_DESKTOP` sniff and failed with advice about setting `PUNKTFUNK_COMPOSITOR`, which the handler puts VERBATIM into the response. So the console showed no physical screen and its only explanation was Linux troubleshooting (sweep §13.17). The data was there all along. `target_inventory()` already walks the CCD database; it now also reports the geometry that walk had in hand — the driving source's position and mode, the GDI device name, the refresh rational, and `primary`. No second CCD walker. * `monitors::list_windows()` maps that onto `PhysicalMonitor`. Two fields cannot mean on Windows what they mean on Linux and are reported honestly rather than invented: `scale` is always 1.0 (Windows scaling is per-monitor DPI applied per application, not a compositor-global logical scale — so the geometry is PIXELS), and an INACTIVE head gets zeroed geometry because the CCD mode indices are only valid for active paths. Inactive heads are still listed, per `list`'s own contract, so "why can't I pick it?" keeps an answer. * `managed` is finally truthful. The doc says only KWin can tell a virtual output from a real head; Windows can too, via our own EDID manufacturer id in the device path (21eda37a). * `detect()` gets an honest non-Linux arm, the operator pin gated with it — naming a Wayland compositor on Windows could never be honoured either. * The handler reports `compositor: "windows"` instead of null-plus-a-Linux-error. ON GLASS (.173, host 0.21.0, TV attached): connector=\\.\DISPLAY1 enabled=true managed=false primary=true 3840x2160 @165Hz pos=(0,0) "LG TV SSCR2" Pinned by an `#[ignore]`d hardware case that asserts at least one NON-managed head is visible — the exact thing that was missing. Read-only, so it is safe against a live host. This also gives `capture_monitor` a picker on Windows, where the handler currently hardcodes `pinned: None`; wiring that is left for the per-monitor-capture work rather than smuggled in here. Verified on .173: `clippy -p punktfunk-host -p pf-vdisplay -p pf-win-display --all-targets -- -D warnings` clean; both xcheck targets clean.	2026-07-29 00:18:27 +02:00
enricobuehlerandClaude Opus 5	2a088e13a4	feat(client/windows): bind a host to a profile, connect with one, pin one, copy its link ... The WinUI half of §5.2: `Target` carries a one-off profile into the session's `--profile`, the host editor gains the binding picker, and the tile names its bound profile on a chip so what a plain click will do is visible without opening anything. Shape notes, both forced by this toolkit rather than chosen: - The flyout has no submenus, so "Connect with", "Pin as card" and "Unpin card" are flat items, one per profile, matched by prefix. The prefixes end in ": " so a profile named "Copy link" can't collide with a fixed entry. - The binding picker commits on change rather than at Save. The rest of that sheet is text boxes with draft refs, which need a Save; a ComboBox doesn't, and making it wait would be the odd one out. Semantics stay identical to the Linux client, because they are the design's, not the platform's: "Connect with" never rebinds, "Default settings" is `Some("")` so it really does override a bound host for one session, and a binding whose profile was deleted reads as Default settings and resolves to it. `pf_client_core::clipboard::set_text` is the small addition the crate needed for "Copy link" — the OS clipboard plumbing was already there, just private to the streaming bridge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 00:17:15 +02:00
enricobuehlerandClaude Opus 5	8299151a09	fix(client-core): drop nine GTK-client files a stray copy left in the crate ... An `rsync` with the wrong destination dropped the Linux shell's sources into `crates/pf-client-core/src/` and they rode along with the previous commit. Nothing referenced them — `lib.rs` never declared the modules, so they were dead weight rather than a build break — but a crate that says it is UI-agnostic must not have a GTK shell sitting in it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 00:04:57 +02:00
enricobuehlerandClaude Opus 5	ad99be2fb2	feat(client): the overlay learns absorb + clear, so per-control shells can edit profiles too ... Groundwork for the Windows half of P1, and a dedup of the Linux one. The two shells commit differently: GTK writes once when the dialog closes (so it can hand over a list of touched rows), WinUI writes on every control change (so it can't). `absorb` serves the second shape — give it the effective settings before and after one control fired, and it records the field that moved. That is deliberately NOT the diff-on-save the design rejects. The comparison is against the EFFECTIVE settings, i.e. what the control was actually showing, not against the globals — so setting a value back to whatever the global happens to be still records an override, which is exactly the pin the design wants. It only ever adds; removing an override stays an explicit, separate operation. `clear` is that operation, keyed by the overlay's own field names, with `resolution` as the one alias for the width/height/match-window tri-state a single control drives on every client. The GTK reset path now calls it instead of carrying its own `match` — that second list was already a place where the two could drift, and the next client would have made it a third. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 00:04:26 +02:00
enricobuehler	6de325a6b6	fix(ci): the unsafe lint said warn while CI enforced it as deny, and main went red ... `unsafe_op_in_unsafe_fn = "warn"` was adopted workspace-wide in 39513528 on the assumption that `warn` is a soft setting you can clear at leisure. It is not: ci.yml runs `cargo clippy … -D warnings`, which promotes it to a hard error, so main has failed on EVERY commit since — Linux `rust` and `rust-arm64` both dying on `pf-client-core` with 70 E0133 errors, and windows-host.yml alongside them. A lint level that understates its own severity is worse than a strict one, so this states what CI already does — `deny` — and writes the exemptions down instead. Fourteen GPU/FFI backend files take `#![allow(unsafe_op_in_unsafe_fn)]`, each with its reason and the workspace Cargo.toml carrying the argument in full. They are not "not done yet": measured across them, 64% of the sites are a single third-party FFI call (ash, pyrowave-sys, libav, the NVENC/AMF entry tables), and of the 44 `unsafe fn`s only 4 have a body containing no unsafe operation at all. Since pf-encode also denies `undocumented_unsafe_blocks`, narrowing them means a hand-written SAFETY comment per line that could only restate the signature — the exact noise that made `unsafe` stop meaning anything here before. Everything else stays at zero and enforced, and each allow is removable on its own terms. Two smaller things this had to clear, both invisible to the job that would have caught them: - `service.rs`: `undocumented_unsafe_blocks` wants the proof on EACH block, and a comment covering a group of consecutive `unsafe` statements only credits the first — so the two `OwnedHandle` wraps became their own statements. Windows-gated, so only the .47 gate sees it. 37a42078 fixed the same class of failure in pf-frame/dxgi.rs concurrently; that fix is taken as-is here. - `clients/linux/ui_settings.rs`: `show`'s deletion left its doc comment stranded on `show_scoped`, which silently merged two unrelated doc blocks onto one fn. Removed. Verified by running the real CI commands, not proxies: on .21 fmt + `clippy --workspace --all-targets -- -D warnings` + the feature-gated `-p pf-encode --features nvenc,vulkan-encode,pyrowave` + build + test, all rc=0; on the Intel box .47 all four windows-host.yml clippy invocations, all rc=0. The arm64 job lints pf-client-core and punktfunk-client-linux, both covered here.	2026-07-28 22:32:30 +02:00
enricobuehler	f40c90aa23	refactor(host/inject/win-display): the Win32 unsafe surface is the FFI call, not the function ... `spawn_in_active_session` existed only to launder `spawn_inner`'s `unsafe fn` marker — and its own comment said why that marker was empty: the callee "has no caller-side preconditions, it validates the session/token itself and owns every handle it opens". A marker that documents the absence of a contract is not a contract. The two collapse into one safe fn whose `unsafe` now sits on the eight Win32 calls, each with the proof it actually needs. Kept `unsafe fn` exactly where a caller CAN break something: `merged_env_block` (a `*const u16` block it must trust), `spawn_host` (a borrowed job `HANDLE`), `desktop_name` (an `HDESK`), `inject_following_desktop` (a live synthetic-pointer device). Those bodies gain explicit blocks instead — `merged_env_block`'s pointer walk is the one place here where the proof is load-bearing rather than restating a signature, and it now says why the scan cannot run off the block's end. With those four files at zero, all three crates take `deny(unsafe_op_in_unsafe_fn)` at the root, next to the `undocumented_unsafe_blocks` deny they already had. The pair matters: alone, the older deny is satisfied by an `unsafe fn` body, which needs no blocks at all and so can hide an unproven FFI call. The workspace lint stays `warn` — it is a ratchet over the encoder backends still to be cleared, and a crate that reaches zero should not be able to drift back silently. Windows E0133 214 -> 182 (the remainder is the ash/AMF-dense encode backends). Verified on the Intel box .47 and on Linux .21: no errors, no `unused_unsafe`, no dead-code fallout.	2026-07-28 22:31:11 +02:00
enricobuehler	37a42078a0	fix(xcheck): mirror [workspace.lints] too — and the two things that hid behind it ... `scripts/xcheck.sh` stopped working the moment the workspace adopted its unsafe discipline (`52191071`). That commit added `[lints] workspace = true` to every crate manifest, and a member inheriting a lint table the workspace ROOT does not define does not merely lose the lint — cargo refuses to parse the manifest at all: error inheriting `lints` from workspace root manifest's `workspace.lints` `workspace.lints` was not defined The generated root mirrored `[workspace.package]` but nothing else. It now mirrors every `[workspace.lints.*]` table generically, so a future table (clippy, rustdoc, …) is picked up without touching the script again. That mattered more than a broken helper, because xcheck is the ONLY thing that lints these crates for Windows — the Windows CI job clippies `-p punktfunk-host -p punktfunk-tray`, which does not lint a dependency. So while it was down, two real failures sat on main unseen: * **pf-frame `dxgi.rs`, 8 × `clippy::undocumented_unsafe_blocks`.** `307ca88b`/`0a710fdf` split the D3DKMT/DXGI calls into one `unsafe` block per FFI call under a single shared SAFETY comment, but the lint wants a proof per block — a comment two lines up with an intervening statement does not count. Each site now carries its own. Not "as above", which this sweep already found to be a proof that rots when the block it points at moves. * **pf-win-display `input_desktop.rs`, one E0133.** `GetUserObjectInformationW` is an unsafe operation in an `unsafe fn` body, which `unsafe_op_in_unsafe_fn` (now warn workspace-wide, and `-D warnings` here) requires to sit in its own block. The SAFETY proof was already written; only the block was missing. The `&&` chain in `elevate_process_gpu_priority` keeps two separate blocks rather than one around the chain — wrapping it would destroy the short-circuit and always issue the relative-priority call. Same trap this sweep hit in `wait_mode_settled`. Verified: both xcheck targets clean again.	2026-07-28 22:17:43 +02:00
enricobuehler	21eda37aa3	fix(win-display): punktfunk's own display is not one of the operator's panels ... The IddCx driver declares `DISPLAYCONFIG_OUTPUT_TECHNOLOGY_HDMI` for its monitors (`packaging/windows/drivers/pf-vdisplay/src/monitor.rs`, `IDDCX_MONITOR_INFO::MonitorType`), and `output_tech_class`'s allowlist reads HDMI as a real external panel. So every consumer of `target_inventory` counted OUR OWN virtual display among the operator's physical monitors. Measured on .173: both targets came back `external_physical`, `[(4352, "LG TV SSCR2", HDMI), (257, "punktfunk", HDMI)]`. That is not cosmetic. `restore_displays_ccd` ends with the last-resort guarantee that the desk is never left all-dark: let (connected, lit) = target_inventory().filter(external_physical) … if connected > 0 && lit == 0 { force_extend_topology() } The restore runs BEFORE the virtual is REMOVEd, so our own still-active display kept `lit >= 1` and the backstop could NEVER fire — in exactly the situation it was written for. It also made our display a candidate "physical suspect" in the disturbance-attribution inventory, which `TargetInventory`'s own doc rules out ("only indirect/virtual targets (our own IDD included)"). Targets carrying our EDID manufacturer id ("PNK", stamped by the driver's edid.rs) are now classified `external_physical = false`, tech `punktfunk-virtual`. Matching on the monitor device path rather than the friendly name — the path carries the manufacturer id the OS itself derived — and on both spellings, since the CCD device-interface path uses `#` (`\\?\DISPLAY#PNK0000#…`) while the PnP instance id for the same monitor uses `\` (`DISPLAY\PNK0000\1&15ecd195&5&UID264`, observed on .173). Allowlist-shaped like `output_tech_class`, so a third-party virtual display is never silently adopted. ON GLASS, read-only, on .173 — and it caught the backstop's trigger live: target 264 active=true external_physical=false tech=punktfunk-virtual "punktfunk" target 4352 active=false external_physical=true tech=HDMI "LG TV SSCR2" The physical is dark while our virtual holds the desktop. Post-fix that is `connected=1, lit=0` — the backstop fires. Pre-fix it was `connected=2, lit=1` — suppressed. The new hardware case is deliberately READ-ONLY (creates and destroys nothing) so it is safe against a live host; repeated IddCx create/destroy churn is what wedges the driver's slot pool. Also adds `manager::FAIL_NEXT_ISOLATES`, a `#[cfg(test)]` seam that fails the next N `isolate_displays_ccd` calls, with every call site in manager.rs routed through it. The Phase-3 recovery legs fire only on a failed isolate, which real hardware does not produce, so they cannot otherwise be exercised. `#[cfg(test)]` rather than an env knob: this crate's live tests compile under `cfg(test)`, so no production switch exists that could disable display isolation on an operator's box. The §5 3.2 case built on it is included but is NOT yet a passing on-glass result — see the plan for where it got to. Verified on .173: clippy -p punktfunk-host -p pf-vdisplay --all-targets and -p pf-win-display --all-targets clean; pf-vdisplay 56 pass, pf-win-display 2 pass + hardware cases; both xcheck targets clean.	2026-07-28 22:17:26 +02:00
enricobuehler	2b2c6f045d	test(vdisplay): pin the force-EXTEND backstop on glass, with a real panel attached ... `force_extend_topology` does two jobs — stop a fresh IddCx monitor being CLONED onto the existing panel, and serve as `restore_displays_ccd`'s last-resort "the desk is not left dark" backstop. Probed directly on .173 with only the LG TV connected, its preset returns rc=31 ERROR_GEN_FAILURE while SDC_USE_DATABASE_CURRENT returns 0, which reads like a backstop that cannot back anything up. On glass it is not, and this case is the measurement that settled it. Active paths went 1 -> (virtual up) 1 -> (after force-EXTEND) 2: with one connected display there is nothing to extend across, hence rc=31; with the virtual present there are two and the preset applies. Both real call sites run in exactly that state — the restore fires BEFORE the REMOVE, so the virtual is still there. No defect. Recording it so nobody re-derives the alarming half from the rc alone. ⭐ It also caught the clone hazard live: the arriving virtual monitor did NOT get its own active path (1 -> 1); only the forced EXTEND gave it one (-> 2). That is exactly the "no distinct source -> no frames" case the function's own doc describes, observed rather than assumed. ⚠️ Residual, noted not asserted: a restore that fails once the virtual is already gone is back to one connected display, where EXTEND returns 31 and cannot re-light anything. Context for the sweep's other on-glass claims: the earlier `live_create_drop` and `live_inplace_resize` passes on this box were taken with the TV POWERED OFF (every monitor devnode Code 45), so they proved the driver round trip but never exercised deactivating a real panel. Re-run now with the TV attached, `live_inplace_resize` still reports in_place=true (target 257 -> 257, 2356x1332), so Phase 6.2 holds with a physical present. Verified on .173: clippy -p punktfunk-host -p pf-vdisplay --all-targets and -p pf-win-display --all-targets clean, 56 pass + 3 ignored, no orphans.	2026-07-28 22:17:26 +02:00
enricobuehler	ea9a25d3b1	fix(win-display): a host with nothing lit is an ANSWER, not a failed CCD query ... `query_active_config` asked `QueryDisplayConfig` for the active topology even when the sizing call had just answered `numPaths = 0`. Windows rejects a zero-count query rather than handing back an empty set, so "nothing is active" came back as "the query failed". Zero active paths is an ordinary state, not a broken one: every panel off or in standby, a KVM switched away, a headless box between its adapter arriving and its first monitor. The `None` propagated into the two places that matter — `count_other_active` reported failure where the honest answer is `Some(0)`, and `isolate_displays_ccd` bailed at its first line, which is exactly the teardown-gate condition whose recovery legs exist to stop the operator's physical panels being left dark (sweep §5 3.1). MEASURED on .173 (RTX 4090, Win11 26200) with the TV powered off — every monitor devnode Code 45, `numPaths = 0` for `QDC_ALL_PATHS` too — in a live, logged-on CONSOLE session: `GetDisplayConfigBufferSizes` returns 0 paths and `QueryDisplayConfig` then returns 0x57 ERROR_INVALID_PARAMETER (0x5 ERROR_ACCESS_DENIED from session 0). This is the mechanism behind the plan's ⭐ note that `count_other_active(&[])` yields `None` on that box: not the API refusing us, just a query nobody should have made. Three changes, one defect: * `query_active_config` short-circuits `numPaths == 0` to an empty-but-valid config. * `isolate_displays_ccd` returns that empty snapshot straight away instead of entering its retry loop. Nothing is active, so nothing needs deactivating and nothing needs restoring; the loop's re-commit exists to drive the IddCx adapter's COMMIT_MODES, and with no path at all there is no config to commit — four attempts would each log a rejected apply and sleep 250 ms to reach the topology we already have. `restore_displays_ccd` already treated an empty config as the no-op it is, so the round trip stays consistent. * `set_virtual_primary_ccd` goes through the shared query. It carried a verbatim copy of it — same flags, same shape — and was therefore the one CCD entry point that would not have inherited the fix. Same seam asymmetry this crate keeps producing, and two hand-written unsafe blocks go with it. Also this file's first tests. The case is `#[ignore]`d hardware, so it reports `ignored` rather than a vacuous `ok` when nobody runs it. On-glass A/B in the console session on .173: it passes with the short-circuit and FAILS without it, naming the conflation. A box that does have a display lit passes either way. ⚠️ The test is read-only and must stay that way — the obvious companion assertion, `isolate_displays_ccd(&[])`, is deliberately not written: an empty keep set means "keep nothing", so on a box with displays it would deactivate every one of them. Verified on .173: `clippy -p punktfunk-host -p pf-vdisplay --all-targets` and `clippy -p pf-win-display --all-targets` both clean, pf-vdisplay 56 pass, pf-win-display 1 pass + 1 ignored. Both xcheck targets clean.	2026-07-28 22:17:26 +02:00
enricobuehler	5a14d2b3f4	fix(vdisplay): the helper budget must end the tree, not just the process we spawned ... `Child::kill` is one `TerminateProcess`: it ends exactly the process we launched. On Unix that is the whole story here — `kscreen-doctor`, `systemctl`, `pw-dump` are single processes we exec directly. On Windows there is no direct exec, so every helper is reached through a shell (`cmd /c …`, `powershell -Command "… | pnputil …"`) and the process that actually hangs is a GRANDCHILD. Killing the shell left it running, holding the stdio handles and the working directory it inherited from us, which means the budget bounded nothing. Not theoretical: it is why a fully green `cargo test -p pf-vdisplay` still failed its CI job on `33a31427`. The suite's own hung-helper case orphaned a 60-second `ping.exe`; it kept the build step's stdout pipe open past the runner's 10 s WaitDelay ("exec: WaitDelay expired before I/O complete") and pinned `crates\pf-vdisplay` so the workspace could not be cleaned up. The tests reported `55 passed; 0 failed` in the same log. Spawned children are now enrolled in a Job object. Job membership is inherited across `CreateProcess`, so one `TerminateJobObject` ends every descendant, and `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` makes that hold on paths that never reach the explicit call — an early `?`, a panic — because closing the last handle is then itself the kill. Best-effort: a box that cannot make a job object degrades to the single-process kill it did before rather than failing the query, the same stance as the already-ignored `Child::kill`. `output_within` ends the tree BEFORE `wait_with_output`. That read runs to EOF, and a grandchild outliving the helper holds the write end — the one way this "bounded" helper could still hang forever. The new test is the permanent lever, and it is not vacuous: it asserts the grandchild reached its first marker before asserting it never reached its second. Verified on .173 (Win11 26200): 56 pass, `clippy -p punktfunk-host -p pf-vdisplay --all-targets` clean, no `ping.exe` survivors. A/B with the job disabled: the test fails with "a grandchild outlived the budget" and one orphan survives. On the CI runner .133 the job APIs were also checked directly, as Administrator (already inside a job — nesting works) and as SYSTEM: both kill the tree. Trap for anyone editing that test: a .cmd file is read in the OEM code page, so an absolute path baked into it is mangled the moment the temp dir holds a non-ASCII character (`C:\Users\Enrico Bühler\…` arrives as `B?hler`) and every redirect fails with "path not found". It uses `%~dp0` instead, so the file stays pure ASCII.	2026-07-28 22:17:26 +02:00
enricobuehler	c262bf43df	refactor(host): three more Windows unsafe fns that had no caller contract ... Continues the crate-by-crate `unsafe_op_in_unsafe_fn` work. Same finding as pf-frame: the useful move is often deleting the `unsafe fn` marker, not decorating the body. `install_steam_audio_pair` (no arguments) and `try_install_steam_audio(&str)` are now safe fns. The call sites had already worked this out — both carried a SAFETY comment opening "`install_steam_audio_pair` is `unsafe` only because it `LoadLibraryExW`s `newdev.dll`", i.e. the obligation was never the caller's. Making them safe deletes 14 lines of that prose from two call sites and moves the real reasoning next to the `LoadLibraryExW`/`transmute`/call chain it actually describes. wasapi_mic.rs: 8 -> 0. `make_job` (no arguments, and it wraps its handle in an `OwnedHandle` before the first fallible step) and `open_log_handle(&Path)` are safe fns too. The latter returns a raw `HANDLE`, but that is an ownership obligation, not a safety one — nothing a caller does with it can cause UB, and it is inherited by the child and closed there. service.rs: 19 -> 14. `spawn_host` keeps `unsafe fn`: it takes a raw `HANDLE`, and an invalid one is UB. That is a contract worth stating, so it keeps the marker. Windows-only note: `LoadLibraryExW` here already passes `LOAD_LIBRARY_SEARCH_SYSTEM32`, so the newdev.dll load cannot pick up a planted DLL from the working directory — now recorded in the SAFETY note rather than left implicit. Verified on Windows .47: punktfunk-host clean at EXITCODE=0 — zero errors, zero unused-unsafe. Crate's own files now 29 E0133 (interactive.rs 15, service.rs 14), down from 42.	2026-07-28 21:31:42 +02:00
enricobuehler	307ca88ba7	refactor(frame): dxgi.rs to zero E0133, and two of its unsafe fns didn't need to be ... First crate done under `unsafe_op_in_unsafe_fn`. 23 sites -> 0, and the file's `unsafe fn` count drops 5 -> 2, because narrowing was the wrong answer for two of them: `enable_inc_base_priority` takes no arguments and touches only the current process's own token. `hags_enabled` takes a `LUID`, a plain value with no pointer a caller could get wrong, and builds every gdi32 argument struct locally. Neither leaves a precondition for a caller to uphold, so neither should have been `unsafe fn` at all — they are now safe fns with the unsafe confined inside, which deletes the obligation from their callers rather than restating it. That is a strictly better outcome than wrapping their bodies, and it is invisible if you treat the lint as a mechanical find-and-wrap. `d3dkmt_set_scheduling_priority_class` stays `unsafe fn`: it takes a `HANDLE`, and an invalid one is UB. That contract is real, so it keeps the marker and gains narrow blocks inside instead. The rest is narrowing. The clearest win is `elevate_process_gpu_priority`, whose `ONCE.call_once(|| unsafe { .. })` had 25 lines inside one block — a `match` on a config enum, three `tracing!` arms and an early return — for exactly one call that needed it. That block is gone; only the `d3dkmt_*` call carries `unsafe` now. Every remaining block is scoped to the FFI call it covers and carries a SAFETY note naming what makes it hold: which handle is live, which local the callee writes, which size matches which variable. Verified: Windows .47 pf-frame clean at EXITCODE=0 — zero errors, zero E0133, zero unused-unsafe. Linux .21 pf-frame zero errors (dxgi.rs is Windows-gated, checked for regressions in the shared parts).	2026-07-28 21:31:42 +02:00
enricobuehler	0a710fdfe5	refactor(frame): narrow make_device's unsafe to the FFI calls themselves ... First slice of the `unsafe_op_in_unsafe_fn` work on the Windows-gated code, and a deliberate demonstration of the shape the rest should take. `cargo fix` is NOT the tool here. Its suggestion for this lint wraps the entire function body: pub unsafe fn make_device(..) -> Result<..> { unsafe { ..40 lines.. }} One block spanning everything, `{ unsafe {` / `}}` bracing, and — the actual problem — zero change in how much code runs unchecked. It makes the status quo explicit rather than smaller, which is the opposite of the goal. Narrowed by hand instead: four blocks, each around the FFI call that needs it (`D3D11CreateDevice`, two `SetGPUThreadPriority`, `SetMaximumFrameLatency`), each with its own SAFETY note. The `Option` handling, the `?`, the `if let Ok(..)` casts and the `tracing!` calls are back outside, where the compiler checks them. Roughly 40 unchecked lines become 6. Fixed two of my own mistakes in the same pass: `elevate_process_gpu_priority` and `auto_priority_gate` are ALREADY safe fns, and I had wrapped their calls in `unsafe` out of momentum. Unnecessary `unsafe` is not free — it is the same "this is scary" noise the lint exists to remove, and the compiler only warns about it. That mistake is a good argument against doing the remaining ~590 sites mechanically. pf-frame drops 23 -> 19 E0133. The remaining three functions in this file need a different answer, not just narrowing: `enable_inc_base_priority` takes no arguments and touches only the current process's own token, so it has NO caller contract and should become a SAFE fn with unsafe blocks inside — which removes an `unsafe fn` rather than decorating it. Same likely for `hags_enabled(LUID)`, whose only argument is a plain value. Verified on Windows .47: pf-frame checks clean at EXITCODE=0, no errors, no unused-unsafe warnings.	2026-07-28 21:31:42 +02:00
enricobuehler	a3843b6996	test(encode): the VAAPI dmabuf path verified on AMD silicon, and its owner-only fields say so ... Closes the last unverified leg of the AvBuffer work. `DmabufInner` was the heaviest ownership change in the crate — four owned objects (DRM device, derived VAAPI device, DRM-PRIME frames ctx, filter graph) whose eight failure branches each repeated the same four-line unwind, once inside a macro, plus a ninth copy in `Drop` — and it had no coverage at all. `vaapi_cpu_encode_smoke` does not reach it: that drives the swscale/CPU-upload path, which uses `VaapiHw` and never builds a graph. `dmabuf_inner_alloc_drop_cycles` loops construct/drop, which is the whole contract now that every handle releases itself. **It passes on a Radeon 780M**, as do the two pre-existing VAAPI tests, so `VaapiHw` and `DmabufInner` are both hardware-verified rather than compile-only. Also fixes three more owner-only fields the AMD box surfaced: `graph`, `vaapi_device` and `drm_device` are `never read` since the hand-written `Drop` that used to read them is gone. Same call as the decoders and the QSV pair — annotate rather than delete (removing `graph` would free it while `src`/`sink` still point into it) or underscore-rename (which hides what they hold). `drm_frames` is NOT annotated: `submit` genuinely reads it per frame. I had missed these twice: my Linux log greps searched "never constructed|never used", which does not match "never read". The check now includes all three spellings. Two environment notes worth keeping, both diagnosed by testing OUTSIDE our code first (ffmpeg's own CLI reproduced each): * Inside a distrobox on an immutable host, VAAPI needs `LIBVA_DRIVERS_PATH=/run/host/usr/lib64/dri`. The container's mesa (25.3.6) is older than the host's (26.0.4) and every encoder open fails with a bare ENOSYS — "Function not implemented", naming nothing. The test doc says so. * `cargo check --workspace` in that container fails on `glib-sys` (no GTK dev headers). Unrelated to this branch; .21 checks the full workspace clean. Verified: AMD .116 (Radeon 780M, Mesa 26.0.4) full pf-encode suite 33 passed / 0 failed plus all 3 ignored VAAPI hardware tests green. Linux .21 workspace check exit 0 / zero errors, pf-encode 33/0, pf-client-core 34/0, and zero dead-code warnings across all three spellings.	2026-07-28 21:31:42 +02:00
enricobuehler	4241bc0383	fix(encode/windows): enable D3D11 multithread protection before libav sees the device ... libav turns on `ID3D11Multithread::SetMultithreadProtected` in `d3d11va_device_create` — the path where it creates the device itself. We take the other path, `d3d11va_device_init`, because we hand it the capturer's existing `ID3D11Device`, and that path does not. Nothing enabled it on our side either, so it was simply off. Two consequences, one of them shipping since long before this branch: QSV zero-copy could not open at all. `av_hwdevice_ctx_create_derived(QSV <- D3D11VA)` ends in `MFXVideoCORE_SetHandle`, and MFX rejects a device without multithread protection as `MFX_ERR_UNDEFINED_BEHAVIOR (-16)`. libav reports that as "Error setting child device handle", which names neither the cause nor the cure. AMF — the DEFAULT Windows zero-copy path — has been running with a lock that does nothing. We deliberately leave `lock`/`unlock` null so libav installs its `d3d11va_default_lock`, and that lock is `ID3D11Multithread::Enter`/`Leave`, which are documented no-ops while protection is off. The lock libav installs to serialise our capture thread against its encode thread has therefore never actually serialised anything. That is the more serious half of this fix, and it is not QSV-specific. Measured on the Intel VM (UHD 750, FFmpeg 7.1.5 + libvpl), sweeping device flags x adapter x protection: BGRA -> QSV derive FAILED (-16) BGRA +MT -> QSV derive OK BGRA|VIDEO +MT -> QSV derive OK `D3D11_CREATE_DEVICE_VIDEO_SUPPORT` makes no difference in either direction, and the same result holds on both Intel adapters the box enumerates. Protection is the only variable that matters, and it read back `was=false` every time, confirming nothing else had enabled it. The hypothesis came from ffmpeg's own CLI succeeding at the identical derive (`-init_hw_device d3d11va=d3d -init_hw_device qsv=q@d3d`) — the only difference being who created the device. With this, `zerocopy_qsv_alloc_drop_cycles` passes on real Intel silicon, which also makes the QSV arm of the AvBuffer refactor hardware-verified rather than compile-only. Its doc comment now points back here, since a future regression will present as that same opaque "child device handle" line. NOT flipped: `zerocopy_active` still defaults QSV off. The derive works and the handles construct and release cleanly, but that is not the same as a validated streaming session, and the default should move on glass evidence, not on this. Verified on .47 (Intel UHD 750): full pf-encode --features amf-qsv suite 39 passed / 0 failed, d3d11hw_alloc_drop_cycles green, zerocopy_qsv_alloc_drop_cycles green, and the native VPL path (qsv_encode_live_smoke) still green. Linux .21: workspace check exit 0 / zero errors, pf-encode 33/0, pf-client-core 34/0.	2026-07-28 21:31:42 +02:00
enricobuehler	2d78d2a514	test(encode): D3d11Hw verified on real silicon; the QSV derive is broken upstream of us ... Ran the QSV probe on the Intel VM (UHD 750, FFmpeg 8). It fails — and not in the code it was written to cover: [AVHWDeviceContext] Error setting child device handle: -16 ZeroCopyInner::open(QSV) failed on iteration 0: derive QSV device from D3D11VA That is `MFX_ERR_UNDEFINED_BEHAVIOR` out of `MFXVideoCORE_SetHandle`, raised inside `av_hwdevice_ctx_create_derived` — libav's own code, called with the same arguments as before this branch. Confirmed pre-existing by A/B: the same probe, written against unmodified `origin/main` with the raw-pointer struct, fails byte-identically on the same box at the same first iteration. The ownership change is not implicated. It is also not the hardware. `qsv::tests::qsv_encode_live_smoke` — the native VPL backend, untouched by this branch — encodes real H.264 on that box via VPL 2.15 with its own D3D11 zero-copy. So QSV works; libav's QSV-from-D3D11VA derive is what does not. That matches `zerocopy_active` already defaulting QSV **off** pending Intel validation, and is now a measured fact rather than a suspicion. The test stays, `#[ignore]`d, with the finding in its doc comment so the next Intel driver / FFmpeg bump re-checks it instead of the question being quietly dropped. What that leaves is real coverage of the half that IS reachable anywhere: `d3d11hw_alloc_drop_cycles` loops construct/drop on `D3d11Hw`, the hwdevice + frames-pool pair both Windows zero-copy vendors share, so it covers the AMF path's ownership without needing AMD hardware. **It passes on the Intel box: 8 cycles, no abort.** Adapter selection is factored into `test_hw_device`, which prefers a vendor and skips the Microsoft Basic Render Driver — a punktfunk host enumerates our own virtual-display adapter too, so `EnumAdapters1(0)` is not a safe assumption. Verified: QSV VM .47 full `pf-encode --features amf-qsv` suite 39 passed / 0 failed (EXITCODE=0), with `d3d11hw_alloc_drop_cycles` green on real silicon. Linux .21 workspace check exit 0 / zero errors, pf-encode 33/0, pf-client-core 34/0.	2026-07-28 21:31:41 +02:00
enricobuehler	564797a12a	test(encode): a QSV construct/drop smoke test, and AvFilterGraph goes Linux-only ... Two loose ends ahead of running this on real Intel silicon. `zerocopy_qsv_alloc_drop_cycles` covers the one ownership question in the crate that was genuinely ambiguous. `ZeroCopyInner::open` builds a `D3d11Hw` and then DERIVES a QSV device + frames ctx from it, and the tuple it used to return handed those two derived pointers out twice — once as the encoder's args, once as the pair moved into `Self`. Free for raw pointers, two owners for `AvBuffer`. Looping construct/drop is what separates the outcomes: a double-unref aborts in the CRT, a missed one leaks an Intel device per session. Nothing else reaches this code — `zerocopy_enabled` defaults QSV OFF, and the native VPL backend supersedes this whole file unless `PUNKTFUNK_QSV_FFMPEG=1` — so the test calls `open` directly and sidesteps both gates, the same shape as `cuda_hw_alloc_drop_cycles`. `AvFilterGraph` is now `#[cfg(target_os = "linux")]`. The Windows gate had been reporting `struct AvFilterGraph is never constructed` since a960dff8 and I had been filtering it out of my own log greps (searching for "never read", which does not match "never constructed"). It is true: the VAAPI dmabuf path is the only filter-graph user, and the AMF/QSV backends build no graph. Cfg'd out rather than `allow`ed, so it cannot outlive its last caller unnoticed. Verified. Windows .133: `cargo test -p pf-encode --features amf-qsv --no-run` at EXITCODE=0 with the AvFilterGraph warnings gone (107 -> 105), and the test binary lists `ffmpeg_win::tests::zerocopy_qsv_alloc_drop_cycles`. Linux .21: workspace check exit 0 / zero errors, pf-encode 33 passed / 0 failed, pf-client-core 34 passed / 0 failed, and zero dead-code warnings there (AvFilterGraph is live on Linux). NOTE for running it: the test binary needs `C:\Users\Public\ffmpeg\bin` on PATH at RUNTIME. `FFMPEG_DIR` is build-time only, and without the DLLs the harness exits silently with no output at all — which reads exactly like a test that does not exist.	2026-07-28 21:31:41 +02:00
enricobuehler	e60fee7da6	refactor(encode): the QSV derived pair stops being two owners of the same pointer ... `ZeroCopyInner` held the QSV device + frames ctx as nullable raw pointers, where null meant "AMF, which feeds D3D11 frames directly" — a convention documented in a comment and enforced by three `is_null()` checks. `Option<AvBuffer>` says it in the type, so the checks and the hand-written `Drop` both go away. The reason this one was left out of the previous commit is the aliasing. `open` built a five-element tuple whose QSV arm handed the SAME two pointers out twice: once as the encoder's `dev_ref`/`frames_ref`, once as the pair moved into `Self`. For raw pointers that is free; for an owning type it is two owners and a double-unref. Ownership and borrowing are now separated — the pair is owned in `qsv_frames`/`qsv_device`, and the encoder's arguments are `as_ptr()` views taken from whichever owner applies. `open_win_encoder` takes its own refs of what it is handed, so lending transfers nothing. Net: all 13 `av_buffer_unref` calls in this file are gone (the last two hand-written Drops with them), including the encoder-open failure arm, which collapses to `?` now that every handle releases itself. Field order pinned and commented, as with the others: QSV frames, QSV device, then `enc`, then `hw` — reproducing the old `Drop`, which ran ahead of all fields and so released the derived pair before the encoder's AddRef'd copies and the D3D11 refs. `qsv_device` picked up an `#[allow(dead_code)]`: `qsv_frames` is still read by the send path, but the device is now purely an owner (the frames ctx and the encoder each hold their own ref). Same call as the decoders — deleting it would free the device early, an underscore name would hide it. Verified on BOTH: Windows .133 `cargo check -p pf-encode --features amf-qsv --all-targets` EXITCODE=0, no dead-code warnings, and confirmed non-vacuous (log shows `Removed 23 files, 36.2MiB` then a real `Checking pf-encode`). Linux .21 `cargo check --workspace --all-targets` exit 0 / zero errors, pf-encode 33 passed / 0 failed, pf-client-core 34 passed / 0 failed, and `cuda_hw_alloc_drop_cycles` still passing on real CUDA. The QSV path itself still wants Intel silicon to exercise.	2026-07-28 21:31:41 +02:00
enricobuehler	5b142a7e85	refactor(encode): the Windows D3D11VA hwdevice owns its refs ... `D3d11Hw::new` is the third instance of the same shape as `CudaHw` and `VaapiHw` — alloc a hwdevice, deref to fill it, init, alloc a frames ctx, deref, init — with the same hand-written unwind: three `av_buffer_unref` calls spread over the failure branches plus a `Drop` repeating the pair. It gets the same treatment, so all three libav hwdevice wrappers in the crate now share one release path. Field order carries the semantics here as in the other two: frames declared before device, so declaration-order dropping reproduces what the hand-written `Drop` did. `ZeroCopyInner`'s `qsv_device`/`qsv_frames` are deliberately NOT converted in this commit. They are nullable (null for AMF, which feeds D3D11 frames directly), and the QSV branch hands the same two pointers out twice — once as the encoder's `dev_ref`/`frames_ref` and once as the owned pair moved into the struct. Modelling that needs `Option<AvBuffer>` plus a borrow/own split so the aliasing does not become two owners, which is a different change from the mechanical one this commit makes. Verified on the Windows runner .133: `cargo check -p pf-encode --features amf-qsv --all-targets` at EXITCODE=0. Confirmed non-vacuous — the log shows `cargo clean -p pf-encode` removing 23 files / 36.2 MiB followed by a real `Checking pf-encode`, because a fast green on that box can otherwise just be cargo reusing artifacts whose timestamps outrank the freshly-extracted sources.	2026-07-28 21:31:41 +02:00