Fix stale comments

- hook_guard.hpp top block: described removal as `hook = {}` (destroy/reset); the
  model is now persistent disable_for_removal (never destroyed mid-session, the
  trampoline stays alive). Updated to match.
- input_source.hpp: SteamInputSource is no longer "future" -- it exists and is
  opt-in; reworded.
- audio_ring.hpp: format_generation actually bumps on every set_format (not
  "reserved, v1 sets once"); verify_capture is a 4-byte atomic guarded by the
  version gate (not "repurposed from a reserved byte old builds saw"); and the
  SharedBlock is no longer "20-byte pads".
- audio_format_verifier.cpp: dropped a dead `(void)recover_layout;` with a stale
  "step (a) only" comment -- the parameter is actually used.

Comment-only except the dead (void) cast.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-24 02:25:42 +02:00
parent 47be3fa53f
commit 5b2334f6e6
5 changed files with 21 additions and 23 deletions

View File

@@ -114,9 +114,6 @@ From an in-depth review pass. Each item is fixed test-first (a failing test, the
as its own commit; "verify" items are confirmed real before any change, and dropped if not.
Code quality:
- **Stale comments** — `hook_guard.hpp` top block (still the old destroy-on-remove model),
`input_source.hpp` ("future" Steam source), `audio_ring.hpp` (v1/reserved), `shared_memory.hpp` /
`audio_ring.hpp` ("20-byte pads").
- **Consolidate `VtableHook`** — remove the duplicate copy in `audio_hook.cpp`.
### Future work

View File

@@ -2,9 +2,9 @@
//
// The injected hook (coop_hook.dll) captures the game's WASAPI render frames and
// is the sole *producer*; the host (coop_host.exe) is the sole *consumer* and
// re-renders the frames for Steam Remote Play Together. This is a separate,
// larger mapping from the input/status SharedBlock (which is only 20-byte pads
// and can't hold PCM): a header followed by a byte ring of `capacity` bytes.
// re-renders the frames for Steam Remote Play Together. This is a separate, larger mapping from the
// input/status SharedBlock (which holds fixed-size POD state, not bulk PCM): a header followed by a
// byte ring of `capacity` bytes.
//
// Lock-free SPSC with free-running 64-bit positions (release on publish, acquire
// on read) — the same cross-process atomic model as the input seqlock. POD and
@@ -58,9 +58,9 @@ struct AudioRingHeader
// (stream counting in HookStatus still runs regardless of this flag).
std::atomic<std::uint32_t> capture_enabled;
// Producer publishes the captured stream's format once, then sets
// format_valid=1 (release). format_generation is reserved so a future
// mid-session device re-init can be made forward-compatible; v1 sets once.
// Producer publishes the captured stream's format, then sets format_valid=1 (release).
// format_generation bumps on every (re)publish (audio_ring_set_format), so the host can notice a
// mid-session format change (a device re-init, or a measured-rate / override update).
std::atomic<std::uint32_t> format_valid;
std::atomic<std::uint32_t> format_generation;
@@ -92,8 +92,8 @@ struct AudioRingHeader
// capture both the hook (pre-mix) and a parallel process-loopback (post-mix) of the same audio
// and cross-correlate them to recover the true sample rate (and, in step b, channels/bit-depth)
// from ground truth instead of guessing. Inert (0) by default -- normal capture is unaffected,
// so it never changes the shipping no-echo path. Repurposed from `reserved`, so the layout and
// size are unchanged (old builds saw it as a zero reserved byte).
// so it never changes the shipping no-echo path. It's a 4-byte atomic carved out of the header's
// reserved space; the version gate (kAudioRingVersion) rejects any layout that doesn't match.
std::atomic<std::uint32_t> verify_capture;
std::uint8_t reserved[36];

View File

@@ -7,13 +7,14 @@
// use-after-free -> the game crashes (the "spamming Mirror video crashed Brotato" bug).
//
// The fix mirrors the audio hooks' epoch+drain pattern, generalised for inline hooks:
// 1. Restore/disable the hook FIRST so no NEW detour can start. For a SafetyHook inline hook
// that's `hook = {}` (reset): it restores the original bytes under thread suspension, and
// its mutex-guarded call wrappers make any in-flight trampoline call safe. For a hook the
// game reaches by a cached pointer (Vulkan present, the WNDPROC subclass) it's clearing an
// atomic gate / restoring the window proc.
// 2. drain() -- wait (bounded) for detour BODIES already running to finish, since the reset
// above does NOT wait for the part of the detour that runs before it calls the trampoline.
// 1. Disable the hook FIRST so no NEW detour can start. For a SafetyHook inline hook that's
// `disable_for_removal(hook)` (disable, NOT `= {}` destroy): it restores the original bytes but
// keeps the trampoline alive -- hooks are PERSISTENT, never destroyed mid-session, so an in-flight
// detour about to call the trampoline never finds it freed (see hook_install.hpp). For a hook the
// game reaches by a cached pointer (Vulkan present, the WNDPROC subclass) it's clearing an atomic
// gate / restoring the window proc instead.
// 2. drain() -- wait (bounded) for detour BODIES already running to finish, since the disable above
// does NOT wait for the part of the detour that runs before it calls the trampoline.
// 3. Only THEN free the shared state the detour was reading.
//
// Each detour wraps its whole body in a DetourGate::Guard (an RAII active-count). drain() spins

View File

@@ -132,7 +132,6 @@ ChunkedCapture parse_chunks(const std::vector<BYTE>& raw, unsigned stride)
FormatVerification verify_stream_format(DWORD pid, AudioRingHeader* ring, unsigned window_ms, bool recover_layout)
{
(void)recover_layout; // step (b) extends this; step (a) recovers the rate only
FormatVerification result;
if (ring == nullptr)
{

View File

@@ -1,9 +1,10 @@
// Abstraction over "where controller input comes from" on the host side.
//
// Phase 0/1 use XInputSource: Remote Play Together delivers each guest's gamepad
// to the focused window (our host) as a virtual XInput controller, so reading
// XInput is enough to see guests. A future SteamInputSource can implement this
// same interface for cleaner per-guest handles once the Steamworks SDK is wired.
// XInputSource is the primary/default: Remote Play Together delivers each guest's gamepad to the
// focused window (our host) as a virtual XInput controller, so reading XInput is enough to see guests.
// SteamInputSource (steam_input_source.cpp) implements this same interface over Steam Input when the
// Steamworks SDK is present; it's opt-in (initializing Steam Input suppresses XInput -- see the
// Controllers panel and Lessons learned).
#pragma once
#include <array>