diff --git a/README.md b/README.md index e7139b3..99e9d65 100644 --- a/README.md +++ b/README.md @@ -108,14 +108,6 @@ default** and covers anything the hooked path doesn't. ## Roadmap -### Current work - -From an in-depth review pass. Each item is fixed test-first (a failing test, then the fix) and lands -as its own commit; "verify" items are confirmed real before any change, and dropped if not. - -Code quality: -- **Consolidate `VtableHook`** — remove the duplicate copy in `audio_hook.cpp`. - ### Future work - **Per-game profiles** — persist each game's subsystem / capture-mode / audio choices and re-apply diff --git a/hook/src/audio_hook.cpp b/hook/src/audio_hook.cpp index 9d59686..1433934 100644 --- a/hook/src/audio_hook.cpp +++ b/hook/src/audio_hook.cpp @@ -14,6 +14,7 @@ #include "debug_log.hpp" #include "hook_registry.hpp" #include "rate_estimator.hpp" +#include "vtable_hook.hpp" namespace coop::hook { @@ -41,69 +42,11 @@ using GetServiceFn = HRESULT(STDMETHODCALLTYPE*)(IAudioClient*, REFIID, void**); using GetBufferFn = HRESULT(STDMETHODCALLTYPE*)(IAudioRenderClient*, UINT32, BYTE**); using ReleaseBufferFn = HRESULT(STDMETHODCALLTYPE*)(IAudioRenderClient*, UINT32, DWORD); -// Hooks one COM vtable slot by overwriting its function pointer; the original is -// called through the saved pointer. We use this instead of SafetyHook's inline -// hooks for the WASAPI COM methods because, on x86, MMDevApi/AudioSes prologues -// use dynamic stack alignment (`and esp,-8`) with EBP-relative argument access, -// which SafetyHook's trampoline relocation mishandles: the relocated prologue -// leaves EBP wrong, so the original reads garbage arguments and faults (it froze -// 32-bit FMOD games the instant audio init ran through the hook). Swapping the -// vtable entry leaves the original code untouched, so it runs with a pristine -// stack regardless of prologue shape. Every instance of a COM coclass shares one -// vtable, so a single swap intercepts all of them (the same property the old -// inline approach relied on). See the project's stdcall-x86 note. -class VtableHook -{ -public: - bool install(void* com_object, unsigned index, void* detour) - { - if (m_vtable != nullptr) - { - return true; // already installed (shared vtable covers every instance) - } - auto** vtable = *reinterpret_cast(com_object); - DWORD old_protect = 0; - if (!VirtualProtect(&vtable[index], sizeof(void*), PAGE_READWRITE, &old_protect)) - { - return false; - } - m_original = vtable[index]; - vtable[index] = detour; // aligned pointer store -> atomic vs. a concurrent caller - VirtualProtect(&vtable[index], sizeof(void*), old_protect, &old_protect); - m_vtable = vtable; - m_index = index; - return true; - } - - void remove() - { - if (m_vtable == nullptr) - { - return; - } - DWORD old_protect = 0; - if (VirtualProtect(&m_vtable[m_index], sizeof(void*), PAGE_READWRITE, &old_protect)) - { - m_vtable[m_index] = m_original; - VirtualProtect(&m_vtable[m_index], sizeof(void*), old_protect, &old_protect); - } - m_vtable = nullptr; - // Deliberately keep m_original valid: a detour already running on the audio thread - // (it doesn't hold our setup lock) may still call original() after we restore the - // slot. The original function lives in the loaded audio module, so the pointer stays - // valid; nulling it here would race that in-flight detour into a null call (a rapid - // hook/unhook crash the mock-game stress test caught). A re-install re-reads it. - m_index = 0; - } - - template Fn original() const { return reinterpret_cast(m_original); } - explicit operator bool() const { return m_vtable != nullptr; } - -private: - void** m_vtable = nullptr; - unsigned m_index = 0; - void* m_original = nullptr; -}; +// The WASAPI COM methods are hooked by SWAPPING their vtable slot -- coop::hook::VtableHook from the +// shared vtable_hook.hpp -- rather than inline-hooking: on x86 the MMDevApi/AudioSes prologues do +// dynamic stack alignment (`and esp,-8`) with EBP-relative args that SafetyHook's trampoline +// relocation mishandles (it froze 32-bit FMOD games the instant audio init ran through the hook). +// Swapping the slot leaves the original code untouched. See the project's stdcall-x86 note. // The scalar audio format we forward; resolved from the game's WAVEFORMATEX. struct CapturedFormat diff --git a/hook/src/vtable_hook.hpp b/hook/src/vtable_hook.hpp index b4b8bb4..aad8410 100644 --- a/hook/src/vtable_hook.hpp +++ b/hook/src/vtable_hook.hpp @@ -5,8 +5,8 @@ // mishandles -> the original runs with garbage args and faults. Swapping the vtable entry leaves // the original code untouched, so it runs with a pristine stack regardless of prologue shape. // Every instance of a COM coclass shares one vtable, so a single swap intercepts all of them. -// See the project's stdcall-x86 note. (audio_hook.cpp has its own equivalent; this is the shared -// copy for the input-side COM hooks.) +// See the project's stdcall-x86 note. Shared by the audio render-hook (audio_hook.cpp) and the +// input-side DirectInput hook (mkb_hook.cpp). #pragma once #include