unom/punktfunk
5
0
Fork 0
Code Issues Pull Requests 2 Actions 4 Packages Projects Releases 36 Wiki Activity

docs(pf-capture): truth pass over comments, docs and log strings (sweep Phase 1)

Browse Source 
Every item here is prose that actively misdirects a maintainer or an operator. Landing it before
the defect fixes means those diffs get reviewed against accurate comments.

1.1 (X5) — split two merged doc blocks that documented the item ABOVE them: `hdr_meta`'s whole
contract was glued onto `fn cursor()` in lib.rs (leaving `hdr_meta` undocumented), and
`hdr_p010_selftest_at`'s was glued onto `hdr_p010_convert_bars_on_luid` in dxgi.rs. rustdoc for
the trait's central HDR contract was simply wrong.

1.2 (L9 doc half) — the `Capturer` trait doc advertised a "bounded drop-oldest channel". The
Linux portal's `sync_channel(8)` + `try_send` is drop-NEWEST: under a consumer stall the arriving
frame is discarded and the encoder gets the stalest queued one. Say so. (Phase 2.6 changes the
behaviour; this commit only stops the doc from lying about today's code.)

1.3 — delete the DONT_FIXATE claim from the dmabuf offer comment. `build_dmabuf_format` emits a
plain MANDATORY `ChoiceEnum::Enum` and `param_changed` re-emits nothing, so there is no two-step
DMA-BUF handshake here; libspa 0.9's `ChoiceFlags` cannot even express DONT_FIXATE. Recorded as a
real follow-up instead of an implemented thing.

1.4 — `mainloop.run()` no longer "blocks until process exit": the quit channel attached above it
stops the loop from `PortalCapturer::drop`.

1.5 (L11) — the 6-arm negotiation log ladder told a fully zero-copy PyroWave session "VAAPI
encode with the CPU capture path — zero-copy was disabled", one line after logging that it had
advertised the PyroWave device's dmabuf modifiers: the passthrough arm was gated on
`pyrowave_modifiers.is_empty()`, so the pyrowave case fell through to the CPU warning. Ungate it;
the CPU-path arm is now reachable only when no dmabuf is advertised at all, which is what it
claims. Its parenthetical also gains the third real cause (the session asked for CPU frames).
Control-flow-neutral for every other combination.

1.6 (W13) — `kick_dwm_compose` claimed a "sub-millisecond" round trip while its
cursor-on-a-sibling-display branch sleeps 35 ms on the CALLER's thread. State the cost on the
function and at both call sites, including which one is on the live frame path.

1.7 (W16) — DDA-era residue, DDA having been removed:
  - retarget the win32u hook's rationale + all four log strings: its job is no longer "keep DDA on
    one adapter" but "keep the virtual display on the adapter SET_RENDER_ADAPTER pinned" (a DXGI
    reparent surfaces as the driver's TEX_FAIL render-adapter mismatch). The hook stays installed.
  - the per-monitor-v2 DPI awareness rationale likewise: not DuplicateOutput1's E_ACCESSDENIED any
    more, but keeping cursor/window coordinates in the PHYSICAL pixels the host's CCD geometry
    (`source_desktop_rect`, `desktop_bounds`) is already in.
  - `HYBRID_HOOK_HITS` was write-only. Surface it as `hybrid_hook_hits()` on the IDD-push open
    line, which is the first point where DXGI has actually been exercised — the patch-readback
    check proves the bytes landed, only this proves DXGI reaches the export.
  - delete `hdr_p010_selftest()`: an unreachable 64×64 wrapper (main.rs calls
    `hdr_p010_selftest_at`); its description moves onto the function that survives.
  - `VideoConverter`'s docs promised a P010/BT.2020 output and a live scRGB input path. It pins
    `YCBCR_STUDIO_G22_LEFT_P709` unconditionally and its only caller always passes
    `scrgb_input: false`.

1.8 (X7) — the native handshake's 4:4:4 gate comment stated the OPPOSITE of what
`pf_capture::capturer_supports_444` returns on Windows ("delivers subsampled NV12/P010 today, so
it returns false there" — it forwards `resolved_backend_ingests_rgb_444()` and the IDD ring passes
BGRA through for an SDR 4:4:4 session).

No `.rs` behaviour delta: comments, doc comments and log strings, plus 1.5's control-flow-neutral
ladder and 1.7's counter accessor. Linux tests 6/6; clippy --all-targets clean on both targets.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
enricobuehler
2026-07-26 10:27:06 +02:00
co-authored by Claude Opus 5
parent 6a2a153c0b
commit 54a37aeb46
6 changed files with 153 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/pf-capture/src/linux/mod.rs
+28 -14
View File
@@ -2265,29 +2265,38 @@ mod pipewire {
);
} else if zerocopy && !want_dmabuf {
tracing::warn!("zero-copy: no importable dmabuf modifiers — using CPU path");
} else if vaapi_passthrough && policy.pyrowave_modifiers.is_empty() {
} else if vaapi_passthrough {
// The raw-passthrough advertisement. Covers the PyroWave case too: its extra
// Vulkan-importable modifiers were appended (and logged) just above, so this arm must
// NOT be gated on `pyrowave_modifiers.is_empty()` — that gate is what dropped a fully
// zero-copy PyroWave session through to the CPU-path warning below (L11).
tracing::info!(
native_nv12_preferred = prefer_native_nv12,
"zero-copy: advertising LINEAR DMA-BUF for encoder import (native NV12 first \
when enabled, packed RGB fallback)"
modifier_count = modifiers.len(),
pyrowave_extended = !policy.pyrowave_modifiers.is_empty(),
"zero-copy: advertising DMA-BUF modifiers for direct encoder import (LINEAR \
always; native NV12 first when enabled, packed RGB fallback)"
);
} else if want_dmabuf && !vaapi_passthrough {
} else if want_dmabuf {
tracing::info!(
count = modifiers.len(),
sample = ?&modifiers[..modifiers.len().min(6)],
"zero-copy: advertising EGL-importable dmabuf modifiers"
);
} else if backend_is_vaapi && policy.backend_is_gpu {
// A VAAPI session on the CPU path pays three full-frame CPU touches (mmap de-pad +
// swscale RGB→NV12 + surface upload) — make the silent fallback visible.
// Reached only when no dmabuf is advertised at all (every arm above rules out a
// zero-copy path), so this genuinely IS the CPU capture path: a VAAPI session then pays
// three full-frame CPU touches (mmap de-pad + swscale RGB→NV12 + surface upload) —
// make the silent fallback visible.
tracing::warn!(
"VAAPI encode with the CPU capture path (per-frame de-pad + swscale CSC + \
upload) — zero-copy was disabled ({}); clear PUNKTFUNK_ZEROCOPY to restore \
the dmabuf default",
upload) — zero-copy is off for this capture ({}); clear PUNKTFUNK_ZEROCOPY to \
restore the dmabuf default",
if std::env::var_os("PUNKTFUNK_ZEROCOPY").is_some() {
"PUNKTFUNK_ZEROCOPY is set falsy"
} else {
"downgraded after a failed dmabuf negotiation"
"a latched downgrade after a failed dmabuf negotiation, or this session's \
output format asked for CPU frames"
}
);
}
@@ -2679,10 +2688,13 @@ mod pipewire {
// version and are composited host-side instead (see `xfixes_cursor.rs`).
//
// When zero-copy is on, offer ONLY a BGRx dmabuf format with our EGL-importable modifiers
// (offering shm too makes the compositor pick shm). The modifier list is advertised with
// DONT_FIXATE so the compositor's allocator chooses one; we re-emit the fixated format in
// `param_changed` (the two-step DMA-BUF handshake). Otherwise offer the multi-format shm
// pod and let MAP_BUFFERS map it. An HDR session replaces ALL of this with the two 10-bit
// (offering shm too makes the compositor pick shm). The modifier list goes out as a plain
// MANDATORY `ChoiceEnum::Enum` and the producer fixates one of the alternatives directly —
// this is NOT the two-step DONT_FIXATE handshake (libspa 0.9's `ChoiceFlags` cannot express
// `SPA_POD_PROP_FLAG_DONT_FIXATE`, and `param_changed` only READS the fixated format, it
// re-emits nothing). Worth revisiting if a multi-modifier offer is ever seen to fail
// negotiation on a compositor that needs the allocator round-trip. Otherwise offer the
// multi-format shm pod and let MAP_BUFFERS map it. An HDR session replaces ALL of this with the two 10-bit
// PQ pods (LINEAR dmabuf, MANDATORY colorimetry — see `build_hdr_dmabuf_format`): offering
// SDR alongside would make the producer pick its earlier-listed SDR format, and the
// negotiation-timeout path latches the process-wide SDR downgrade if nothing matches.
@@ -2751,7 +2763,9 @@ mod pipewire {
)
.context("pw stream connect")?;
// Blocks this thread, pumping frame callbacks until process exit.
// Blocks this thread, pumping frame callbacks until the capturer's `Drop` fires the quit
// channel attached above (`_quit_attach` → `quit_loop.quit()`), at which point `run()`
// returns and the thread unwinds — releasing the importer / CUDA context deterministically.
mainloop.run();
Ok(())
}