Add audio-fidelity validator + fix mirror render under-run
Build coop_audio_validate, a tool that turns "the mirror audio sounds off" into numbers. It plays a known sine (coop_tone, 44.1 kHz on a 48 kHz endpoint -- the Godot/Brotato case), injects the hook as the host does, and runs a fidelity analyzer (coop/tone_analysis.hpp: pitch error in cents, SNR/THD, click + dropout counts), dumping a .wav to listen to. Modes: --render drives the real AudioMirror and measures its rendered output; --baseline/--selfcheck give the measurement floor; --listen <pid> records a live coop_host's output; --wav analyzes a recording. Analyzer + WAV I/O are unit-tested (tone_analysis_test) against synthesized defects. Using it, the capture ring measures pristine (~68 dB, 0 gaps) while the render path dropped to ~18 dB with gaps -- localizing a real defect in AudioMirror::run_hooked: it re-primed (withheld the feed until ~30 ms had rebuffered) on any partial fill (to_write < avail). A partial fill is normal producer jitter, and withholding the feed drains the device, so a one-frame ring dip became a full ~30 ms drop-out; on a jittery game it fired constantly. Fix: feed whatever is available each tick and re-prime only on a genuine starvation (device empty AND ring empty). The policy is factored into a pure RenderPacer reused by run_hooked + run_loopback and proven by render_pacer_test (the old policy withholds available data ~168x and drains to one period from silence on a jittery schedule; the new one never withholds). ctest 17/17. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -36,6 +36,22 @@ add_executable(rate_estimator_test rate_estimator_test.cpp)
|
||||
target_include_directories(rate_estimator_test PRIVATE ${CMAKE_SOURCE_DIR}/hook/src)
|
||||
add_test(NAME rate_estimator_test COMMAND rate_estimator_test)
|
||||
|
||||
# Unit test for the audio fidelity analyzer (pitch error in cents, SNR/THD, click +
|
||||
# dropout detection) and the WAV reader/writer. Synthesizes adversarial signals (clean /
|
||||
# wrong-rate pitch shift / injected clicks / silence gaps) and asserts the metrics. Pure
|
||||
# header logic, no device. Underpins the coop_audio_validate diagnostic tool.
|
||||
add_executable(tone_analysis_test tone_analysis_test.cpp)
|
||||
target_link_libraries(tone_analysis_test PRIVATE coop_common)
|
||||
add_test(NAME tone_analysis_test COMMAND tone_analysis_test)
|
||||
|
||||
# Unit test for the audio-mirror render pacing policy (host/src/audio/render_pacer.hpp).
|
||||
# Simulates a producer/consumer device timeline and asserts the shipping RenderPacer rides
|
||||
# through producer jitter that makes the old re-prime-on-partial-fill policy glitch
|
||||
# repeatedly (the under-run / "metallic" bug coop_audio_validate found). Header-only, no device.
|
||||
add_executable(render_pacer_test render_pacer_test.cpp)
|
||||
target_include_directories(render_pacer_test PRIVATE ${CMAKE_SOURCE_DIR}/host/src)
|
||||
add_test(NAME render_pacer_test COMMAND render_pacer_test)
|
||||
|
||||
# Unit test for the per-game audio override store (persist/reload, case-insensitive
|
||||
# lookup, differing-overwrite detection). Reuses the shipping source. No device.
|
||||
add_executable(audio_overrides_test
|
||||
@@ -207,6 +223,8 @@ coop_output_subdir(tests
|
||||
mkb_ring_test
|
||||
mkb_map_test
|
||||
audio_mix_test
|
||||
tone_analysis_test
|
||||
render_pacer_test
|
||||
rate_estimator_test
|
||||
audio_overrides_test
|
||||
audio_loopback_test
|
||||
|
||||
222
tests/render_pacer_test.cpp
Normal file
222
tests/render_pacer_test.cpp
Normal file
@@ -0,0 +1,222 @@
|
||||
// Unit test for the audio-mirror render pacing policy (host/src/audio/render_pacer.hpp).
|
||||
//
|
||||
// Reproduces, deterministically and with no audio device, the under-run bug coop_audio_validate
|
||||
// found live: the mirror re-rendered the captured ring to the output device, and the OLD policy
|
||||
// re-primed whenever it couldn't completely fill the free buffer that tick (`to_write < avail`).
|
||||
// Re-priming withholds the feed until ~30 ms has rebuffered, which drains the device and
|
||||
// manufactures a silence gap -- so a one-frame ring dip became a full drop-out. On a jittery
|
||||
// producer that fired constantly, giving the choppy / "metallic" mirror audio.
|
||||
//
|
||||
// The test simulates a producer/consumer device timeline (the ring fills in bursts; the device
|
||||
// drains a fixed amount each tick) and counts under-runs (ticks the device buffer empties =
|
||||
// audible silence). It asserts the shipping RenderPacer rides through jitter that makes the OLD
|
||||
// policy glitch repeatedly, and that neither policy glitches on a steady producer (no regression).
|
||||
#include <algorithm>
|
||||
#include <cstdint>
|
||||
#include <cstdio>
|
||||
#include <vector>
|
||||
|
||||
#include "audio/render_pacer.hpp"
|
||||
|
||||
using namespace coop;
|
||||
|
||||
namespace
|
||||
{
|
||||
int g_failures = 0;
|
||||
void check(bool ok, const char* what)
|
||||
{
|
||||
if (ok)
|
||||
{
|
||||
std::printf(" ok: %s\n", what);
|
||||
}
|
||||
else
|
||||
{
|
||||
std::printf("FAIL: %s\n", what);
|
||||
++g_failures;
|
||||
}
|
||||
}
|
||||
|
||||
// The original policy: re-prime on any partial fill (`to_write < avail`). Kept here only to
|
||||
// contrast against the shipping RenderPacer.
|
||||
struct LegacyPacer
|
||||
{
|
||||
std::uint32_t prime_frames = 0;
|
||||
bool primed = false;
|
||||
std::uint32_t pump(std::uint32_t avail, std::uint32_t have, std::uint32_t /*padding*/)
|
||||
{
|
||||
if (!primed && have >= prime_frames)
|
||||
{
|
||||
primed = true;
|
||||
}
|
||||
if (!primed)
|
||||
{
|
||||
return 0;
|
||||
}
|
||||
const std::uint32_t to_write = std::min(avail, have);
|
||||
if (to_write < avail)
|
||||
{
|
||||
primed = false; // the bug: a partial fill forces a full re-prime
|
||||
}
|
||||
return to_write;
|
||||
}
|
||||
};
|
||||
|
||||
struct SimResult
|
||||
{
|
||||
int underruns = 0; // device couldn't supply a full period (audible silence)
|
||||
std::uint32_t min_headroom = 0; // smallest device-buffer level seen after warm-up (cushion left)
|
||||
int withheld = 0; // ticks the policy refused to feed though the ring had >=1 period
|
||||
};
|
||||
|
||||
// Run one policy over a producer schedule, modelling an event-driven WASAPI render client.
|
||||
// render_frames = device buffer size (frames); period = frames the device plays per device tick.
|
||||
// Each tick: the game pushes producer[t] into the ring; the device plays a period (silence if it
|
||||
// can't); the policy refills in response to the buffer event. Reports under-runs plus two
|
||||
// non-marginal signals: the minimum cushion the policy maintained, and how often it withheld
|
||||
// data it actually had (the old re-prime policy's defining pathology).
|
||||
template <class Pacer>
|
||||
SimResult simulate(Pacer pacer, const std::vector<std::uint32_t>& producer, std::uint32_t render_frames,
|
||||
std::uint32_t period)
|
||||
{
|
||||
std::uint32_t device = 0; // frames queued in the device buffer
|
||||
std::uint64_t ring = 0; // frames available in the ring
|
||||
SimResult res;
|
||||
res.min_headroom = render_frames;
|
||||
bool warming = true; // ignore the initial fill-up before playback starts
|
||||
for (std::size_t t = 0; t < producer.size(); ++t)
|
||||
{
|
||||
ring += producer[t]; // the game pushed this tick's frames into the ring
|
||||
// The device plays a period; its event then fires asking for more. If the buffer
|
||||
// can't supply a full period, the renderer plays silence -- an audible under-run.
|
||||
if (!warming)
|
||||
{
|
||||
if (device >= period)
|
||||
{
|
||||
device -= period;
|
||||
}
|
||||
else
|
||||
{
|
||||
++res.underruns;
|
||||
device = 0;
|
||||
}
|
||||
}
|
||||
// Refill in response to the event.
|
||||
const std::uint32_t padding = device;
|
||||
const std::uint32_t avail = render_frames - device;
|
||||
const std::uint32_t have = static_cast<std::uint32_t>(std::min<std::uint64_t>(ring, render_frames));
|
||||
std::uint32_t w = pacer.pump(avail, have, padding);
|
||||
w = std::min(w, avail);
|
||||
w = static_cast<std::uint32_t>(std::min<std::uint64_t>(w, ring));
|
||||
device += w;
|
||||
ring -= w;
|
||||
if (w > 0)
|
||||
{
|
||||
warming = false; // playback has begun
|
||||
}
|
||||
if (!warming)
|
||||
{
|
||||
res.min_headroom = std::min(res.min_headroom, device);
|
||||
if (w == 0 && have >= period && avail >= period)
|
||||
{
|
||||
++res.withheld; // had at least a period to give and room to put it -- but didn't
|
||||
}
|
||||
}
|
||||
}
|
||||
return res;
|
||||
}
|
||||
|
||||
// A clock-locked jittery producer: it owes `period` frames every tick but its audio thread
|
||||
// briefly stalls (delivers 0 for 2 of every 7 ticks), then catches up the backlog -- never
|
||||
// running ahead (it is rate-locked to the device, like a real game). Long-run mean = period,
|
||||
// so the ring level doesn't drift; the stalls are pure timing jitter. A 2-tick stall is within
|
||||
// the cushion, so a policy that simply tops up rides it. The old re-prime-on-partial-fill policy
|
||||
// instead freezes the feed for the whole rebuffer window each stall, draining the device -> gaps.
|
||||
std::vector<std::uint32_t> jittery_schedule(std::uint32_t period, int ticks)
|
||||
{
|
||||
std::vector<std::uint32_t> p;
|
||||
p.reserve(ticks);
|
||||
std::uint64_t owed = 0;
|
||||
const std::uint32_t catch_up = period * 7 / 5; // 1.4x: clears the 2/7 stall backlog, but no faster
|
||||
for (int t = 0; t < ticks; ++t)
|
||||
{
|
||||
owed += period;
|
||||
const std::uint32_t cap = (t % 7 < 2) ? 0u : catch_up; // stall 2/7 ticks, else catch up gently
|
||||
const std::uint32_t deliver = static_cast<std::uint32_t>(std::min<std::uint64_t>(owed, cap));
|
||||
owed -= deliver;
|
||||
p.push_back(deliver);
|
||||
}
|
||||
return p;
|
||||
}
|
||||
} // namespace
|
||||
|
||||
int main()
|
||||
{
|
||||
constexpr std::uint32_t kPeriod = 480; // 10 ms @ 48 kHz device tick
|
||||
constexpr std::uint32_t kRender = 2400; // 50 ms device buffer (5 periods)
|
||||
constexpr std::uint32_t kPrime = 2400; // prime the full buffer before playing
|
||||
|
||||
// --- Steady producer: exactly one period per tick. Neither policy should glitch. -----
|
||||
{
|
||||
std::vector<std::uint32_t> steady(400, kPeriod);
|
||||
RenderPacer np;
|
||||
np.prime_frames = kPrime;
|
||||
LegacyPacer lp;
|
||||
lp.prime_frames = kPrime;
|
||||
const SimResult n = simulate(np, steady, kRender, kPeriod);
|
||||
const SimResult l = simulate(lp, steady, kRender, kPeriod);
|
||||
check(n.underruns == 0, "steady: new policy has no under-runs");
|
||||
check(l.underruns == 0, "steady: old policy also clean (no regression from the fix)");
|
||||
std::printf(" (steady: new under-runs=%d old=%d)\n", n.underruns, l.underruns);
|
||||
}
|
||||
|
||||
// --- Jittery producer: the bug case. New keeps the buffer fed; old withholds + starves. -
|
||||
{
|
||||
const auto sched = jittery_schedule(kPeriod, 400);
|
||||
RenderPacer np;
|
||||
np.prime_frames = kPrime;
|
||||
LegacyPacer lp;
|
||||
lp.prime_frames = kPrime;
|
||||
const SimResult n = simulate(np, sched, kRender, kPeriod);
|
||||
const SimResult l = simulate(lp, sched, kRender, kPeriod);
|
||||
std::printf(" (jittery: NEW under-runs=%d min_headroom=%u withheld=%d)\n", n.underruns,
|
||||
n.min_headroom, n.withheld);
|
||||
std::printf(" (jittery: OLD under-runs=%d min_headroom=%u withheld=%d)\n", l.underruns,
|
||||
l.min_headroom, l.withheld);
|
||||
// The defining pathology, measured directly (not timing-marginal): the old policy refuses
|
||||
// to feed data it has, repeatedly; the new policy never withholds once playing.
|
||||
check(n.withheld == 0, "jittery: new policy never withholds available data");
|
||||
check(l.withheld >= 10, "jittery: old policy withholds available data repeatedly (the bug)");
|
||||
// And that withholding drives the device buffer to the brink: the old policy drains the
|
||||
// cushion to zero (one stutter away from silence) while the new keeps real headroom.
|
||||
check(l.min_headroom < n.min_headroom, "jittery: old policy keeps far less buffer headroom");
|
||||
check(n.min_headroom >= kPeriod, "jittery: new policy always keeps >=1 period of cushion");
|
||||
}
|
||||
|
||||
// --- pump() never writes past the free space or the ring contents --------------------
|
||||
{
|
||||
RenderPacer p;
|
||||
p.prime_frames = 100;
|
||||
p.primed = true;
|
||||
check(p.pump(50, 1000, 200) == 50, "pump clamps to avail");
|
||||
check(p.pump(1000, 30, 200) == 30, "pump clamps to have");
|
||||
}
|
||||
|
||||
// --- Genuine starvation re-primes; a partial fill does not ---------------------------
|
||||
{
|
||||
RenderPacer p;
|
||||
p.prime_frames = 100;
|
||||
p.primed = true;
|
||||
(void)p.pump(480, 50, 240); // partial fill (have<avail) but device still has padding
|
||||
check(p.primed, "partial fill keeps primed (no manufactured gap)");
|
||||
(void)p.pump(1440, 0, 0); // device drained AND ring empty -> genuine starvation
|
||||
check(!p.primed, "true starvation (padding==0 && have==0) re-primes");
|
||||
}
|
||||
|
||||
if (g_failures == 0)
|
||||
{
|
||||
std::printf("PASS render_pacer_test\n");
|
||||
return 0;
|
||||
}
|
||||
std::printf("FAILED render_pacer_test (%d)\n", g_failures);
|
||||
return 1;
|
||||
}
|
||||
159
tests/tone_analysis_test.cpp
Normal file
159
tests/tone_analysis_test.cpp
Normal file
@@ -0,0 +1,159 @@
|
||||
// Unit test for the audio fidelity analyzer (common/include/coop/tone_analysis.hpp).
|
||||
//
|
||||
// Synthesizes controlled signals -- a clean sine, a sine analyzed at the wrong rate
|
||||
// (the pitch-shift bug), a sine with injected clicks, and a sine with a silence gap --
|
||||
// and asserts the analyzer's numbers match what was injected. This makes the metrics
|
||||
// trustworthy before they're used to diagnose the real mirror path. Also round-trips a
|
||||
// buffer through the WAV writer/reader + the PCM channel decoder. No audio device.
|
||||
#include <cmath>
|
||||
#include <cstdint>
|
||||
#include <cstdio>
|
||||
#include <vector>
|
||||
|
||||
#include "coop/tone_analysis.hpp"
|
||||
#include "coop/wav.hpp"
|
||||
|
||||
using namespace coop;
|
||||
|
||||
namespace
|
||||
{
|
||||
int g_failures = 0;
|
||||
void check(bool ok, const char* what)
|
||||
{
|
||||
if (ok)
|
||||
{
|
||||
std::printf(" ok: %s\n", what);
|
||||
}
|
||||
else
|
||||
{
|
||||
std::printf("FAIL: %s\n", what);
|
||||
++g_failures;
|
||||
}
|
||||
}
|
||||
|
||||
constexpr double kTwoPi = 6.283185307179586;
|
||||
|
||||
// A clean sine of `freq` Hz at `rate`, `seconds` long, amplitude 0.25 (matches coop_tone).
|
||||
std::vector<float> make_sine(double freq, unsigned rate, double seconds, double amp = 0.25)
|
||||
{
|
||||
const std::size_t n = static_cast<std::size_t>(rate * seconds);
|
||||
std::vector<float> v(n);
|
||||
const double step = kTwoPi * freq / rate;
|
||||
for (std::size_t i = 0; i < n; ++i)
|
||||
{
|
||||
v[i] = static_cast<float>(std::sin(step * i) * amp);
|
||||
}
|
||||
return v;
|
||||
}
|
||||
} // namespace
|
||||
|
||||
int main()
|
||||
{
|
||||
// --- Clean 1 kHz tone at 48 kHz: ~0 cents, high SNR, no clicks/dropouts ----------
|
||||
{
|
||||
auto sine = make_sine(1000.0, 48000, 2.0);
|
||||
const ToneReport r = analyze_tone(sine.data(), sine.size(), 48000, 1000.0);
|
||||
check(r.valid, "clean: valid");
|
||||
check(std::fabs(r.pitch_error_cents) < 5.0, "clean: pitch error < 5 cents");
|
||||
check(std::fabs(r.dominant_hz - 1000.0) < 2.0, "clean: dominant ~1000 Hz");
|
||||
check(r.snr_db > 50.0, "clean: SNR > 50 dB");
|
||||
check(r.thd_percent < 1.0, "clean: THD < 1%");
|
||||
check(r.glitch_count == 0, "clean: no clicks");
|
||||
check(r.dropout_count == 0, "clean: no dropouts");
|
||||
check(std::fabs(r.peak - 0.25) < 0.01, "clean: peak ~0.25");
|
||||
std::printf(" (clean: %.3f Hz, %.2f cents, SNR %.1f dB, THD %.3f%%)\n", r.dominant_hz,
|
||||
r.pitch_error_cents, r.snr_db, r.thd_percent);
|
||||
}
|
||||
|
||||
// --- Pitch-shift bug: real 44100 samples played as if 48000 -----------------------
|
||||
// The hook captures true 44.1 kHz samples but mis-declares 48 kHz; the host renders
|
||||
// them at 48 kHz, shifting a 1000 Hz tone up to 1000*48000/44100 ~= 1088.4 Hz. Expected
|
||||
// cents = 1200*log2(48000/44100) ~= +146.7. The analyzer must recover that.
|
||||
{
|
||||
auto sine = make_sine(1000.0, 44100, 2.0); // generated at the *true* rate
|
||||
const ToneReport r = analyze_tone(sine.data(), sine.size(), 48000, 1000.0); // analyzed at the wrong rate
|
||||
const double expect_cents = 1200.0 * std::log2(48000.0 / 44100.0);
|
||||
check(std::fabs(r.pitch_error_cents - expect_cents) < 5.0, "pitch-shift: ~+147 cents detected");
|
||||
check(r.pitch_error_ratio > 1.05, "pitch-shift: ratio > 1.05 (audibly sharp)");
|
||||
check(std::fabs(r.dominant_hz - 1088.4) < 3.0, "pitch-shift: dominant ~1088 Hz");
|
||||
std::printf(" (pitch-shift: %.2f cents vs expected %.2f, dominant %.2f Hz)\n",
|
||||
r.pitch_error_cents, expect_cents, r.dominant_hz);
|
||||
}
|
||||
|
||||
// --- Click injection: discontinuities the analyzer must count ---------------------
|
||||
{
|
||||
auto sine = make_sine(1000.0, 48000, 2.0);
|
||||
const unsigned injected = 9;
|
||||
for (unsigned k = 0; k < injected; ++k)
|
||||
{
|
||||
const std::size_t at = sine.size() * (k + 1) / (injected + 2);
|
||||
sine[at] += 0.7f; // a sharp isolated jump (a click)
|
||||
}
|
||||
const ToneReport r = analyze_tone(sine.data(), sine.size(), 48000, 1000.0);
|
||||
check(r.glitch_count >= injected - 1 && r.glitch_count <= injected + 1,
|
||||
"clicks: counted ~9 discontinuities");
|
||||
check(r.dropout_count == 0, "clicks: no false dropouts");
|
||||
std::printf(" (clicks: injected %u, detected %u, rate %.2f/s)\n", injected, r.glitch_count,
|
||||
r.glitch_rate_per_sec);
|
||||
}
|
||||
|
||||
// --- Dropout injection: a mid-signal silence gap (the re-prime artifact) ----------
|
||||
{
|
||||
auto sine = make_sine(1000.0, 48000, 2.0);
|
||||
// Two ~20 ms gaps of silence.
|
||||
for (int g = 0; g < 2; ++g)
|
||||
{
|
||||
const std::size_t at = sine.size() * (g + 1) / 3;
|
||||
for (std::size_t i = 0; i < 48000u * 20 / 1000; ++i)
|
||||
{
|
||||
sine[at + i] = 0.0f;
|
||||
}
|
||||
}
|
||||
const ToneReport r = analyze_tone(sine.data(), sine.size(), 48000, 1000.0);
|
||||
check(r.dropout_count >= 2, "dropouts: counted >= 2 gaps");
|
||||
check(r.dropout_ms > 30.0, "dropouts: total > 30 ms");
|
||||
std::printf(" (dropouts: %u gaps, %.1f ms total)\n", r.dropout_count, r.dropout_ms);
|
||||
}
|
||||
|
||||
// --- Non-tone path: expected_hz = 0 skips pitch but still levels/clicks ------------
|
||||
{
|
||||
auto sine = make_sine(440.0, 48000, 0.5);
|
||||
const ToneReport r = analyze_tone(sine.data(), sine.size(), 48000, 0.0);
|
||||
check(r.valid && r.dominant_hz == 0.0, "no-expected: pitch skipped");
|
||||
check(r.rms > 0.1, "no-expected: RMS still measured");
|
||||
}
|
||||
|
||||
// --- WAV round-trip + int16 channel decode ----------------------------------------
|
||||
{
|
||||
// Build a 2-channel int16 buffer: channel 0 a 1 kHz sine, channel 1 silent.
|
||||
const unsigned rate = 48000, ch = 2;
|
||||
auto mono = make_sine(1000.0, rate, 0.5, 0.5);
|
||||
std::vector<std::int16_t> inter(mono.size() * ch, 0);
|
||||
for (std::size_t i = 0; i < mono.size(); ++i)
|
||||
{
|
||||
inter[i * ch + 0] = static_cast<std::int16_t>(mono[i] * 32767.0f);
|
||||
}
|
||||
const std::wstring path = L"tone_analysis_test_roundtrip.wav";
|
||||
const bool wrote = wav_write(path, inter.data(), inter.size() * sizeof(std::int16_t), rate, ch, 16,
|
||||
kToneFormatPcm);
|
||||
check(wrote, "wav: write ok");
|
||||
WavData wd;
|
||||
const bool readback = wav_read(path, wd);
|
||||
check(readback, "wav: read ok");
|
||||
check(wd.sample_rate == rate && wd.channels == ch && wd.bits == 16 && wd.format_tag == kToneFormatPcm,
|
||||
"wav: format round-trips");
|
||||
auto dec = decode_channel(wd.pcm.data(), wd.pcm.size(), wd.format_tag, wd.bits, wd.channels, 0);
|
||||
check(dec.size() == mono.size(), "decode: frame count matches");
|
||||
const ToneReport r = analyze_tone(dec.data(), dec.size(), rate, 1000.0);
|
||||
check(std::fabs(r.pitch_error_cents) < 5.0, "decode: channel 0 recovers 1 kHz");
|
||||
std::remove("tone_analysis_test_roundtrip.wav");
|
||||
}
|
||||
|
||||
if (g_failures == 0)
|
||||
{
|
||||
std::printf("PASS tone_analysis_test\n");
|
||||
return 0;
|
||||
}
|
||||
std::printf("FAILED tone_analysis_test (%d)\n", g_failures);
|
||||
return 1;
|
||||
}
|
||||
Reference in New Issue
Block a user