From 9f6b84cf33016f47141c0497618d46fec21bccfa Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Mon, 4 May 2026 05:08:11 +0200 Subject: [PATCH] core: audio determinism fix and pipeline cleanup Audit pass 6 - audio path. The libretro core only ever outputs 16-bit stereo, so the question was whether anything in the audio pipeline is either non-deterministic or pointlessly busy given that constraint. DETERMINISM BUG FIX src/filter.c's `sexyfilter_acc1` and `sexyfilter_acc2` are file-scope int64_t IIR accumulators. They're saved/restored with the rest of the sound state (sound.c "FAC1"/"FAC2" SFORMAT entries), but they were NOT reset by FCEUSND_Power on cart load. So a second cart loaded in the same process inherited the first cart's IIR state - audibly minor (a few samples of transient before the IIR re-converges) but non-deterministic across load orders. `SexyFilter2`'s lowpass accumulator was even worse: a function-local `static int64_t acc = 0` that no other code could touch. Its first- load value was 0; from then on it just accumulated forever, with no way to reset it. Same load-order-dependence problem, plus it wasn't even savestate'd. Fix: - Lift SexyFilter2's local static to file scope as `sexyfilter2_acc`. - Add `void SexyFilter_Reset(void)` that zeros all three accumulators. - Call SexyFilter_Reset() from FCEUSND_Power. - Add `sexyfilter2_acc` to the SFORMAT savestate list as "FAC3" so runahead/replay/netplay restore it alongside FAC1/FAC2. `mrindex` (filter.c file-scope, NeoFilterSound's input cursor) is already reset in MakeFilters, which FCEUI_Sound calls on every cart load, so it doesn't need explicit handling. `mrratio` is set in the same path. Documented in the new SexyFilter_Reset comment. PIPELINE CLEANUPS (1) WaveHi memset bound. The HQ-mode flush was clearing the entire WaveHi[40000] (160 KB) past `left` every frame, but channels only write into [left, SOUNDTS) - everything past SOUNDTS is already zero from the previous frame's clear (or FCEUSND_Power on the first frame). Tightened the memset to (SOUNDTS - left) * 4 bytes instead of sizeof(WaveHi) - left * 4. SOUNDTS is bounded by NES cycles per frame (~30000 NTSC), so this saves ~40 KB of pointless memset every HQ frame. The (SOUNDTS > left) guard handles the degenerate case of a very short frame where SOUNDTS might not have advanced past the coefficient history. (2) RDoSQLQ silent-channel branch. When both square channels are inactive, `amp[x]` is forced to 0 (line 634), which propagates through `ttable[x]` to make `totalout = wlookup1[0] = 0`. The previous code looped (end - start) iterations adding 0 to Wave[V>>4] - genuinely no-op for ~30000 NES cycles per frame when nothing is playing. Removed the loop body. (3) LQ dispatch deduplication. `RDoSQLQ` mixes both squares in a single call; `RDoTriangleNoisePCMLQ` mixes triangle/noise/PCM in a single call. The previous code pointed DoSQ1 and DoSQ2 both at RDoSQLQ, and DoTriangle/DoNoise/DoPCM all at RDoTriangleNoisePCMLQ. The first call did the work; the subsequent calls entered the function only to be rejected by the `if (end <= start) return;` guard at the top. Now point DoSQ1/DoTriangle at the real workers and DoSQ2/DoNoise/DoPCM at Dummyfunc - same behaviour, no redundant call+early-return on every frame. (4) stereo_filter_apply_delay copy loop -> memcpy. The element-by- element copy `samples[i+pos] = sound_buffer[i]` is trivially equivalent to memcpy for non-overlapping int32_t blocks; memcpy lets the compiler/libc dispatch SIMD where available. (5) Dead code in filter.c. Removed the `#ifdef moo` and `#if 0` blocks (legacy commented-out lowpass formula attempts that have been dead since the original blargg import) and the `` include, which is no longer needed once those blocks are gone. ARCHITECTURAL NOTES (not changed - documented for future passes) - Per-frame sample count varies by +/-1 (e.g. 798/799 at 48000Hz 60Hz NTSC). This is required by libretro's variable-batch audio model and is fundamental to the deterministic phase accumulator (`soundtsoffs` carries the fractional remainder across frames). Same input -> same count sequence. Replay/netplay-stable. - The `stereo_filter_apply_null` post-pass exists to convert in-place from int32 mono samples to int32 stereo-packed ((s<<16)|(s&0xFFFF)) format that, when the buffer is cast to int16_t*, reads correctly as L,R,L,R pairs. Could be folded into SexyFilter's clamp step to save one full-buffer pass (~190 KB/s of redundant memory traffic) but requires sound.c to know about the libretro frontend's stereo filter selection. Too much frontend/core coupling to chase for a modest cache win; left as-is. - HQ path runs 5 in-place passes through WaveFinal[] per frame (NeoFilterSound, SexyFilter, optional SexyFilter2, stereo_filter_apply, audio_batch_cb read). LQ path runs 4. Each pass is ~3.2 KB at 48000Hz - ~1 MB/s of buffer traffic total. Modest; not worth restructuring. - WaveHi[40000] is BSS-allocated (160 KB) even when in LQ mode where it's never written. Cost is zero - BSS pages don't materialise until first write - so making it conditional has no runtime benefit, only adds complexity. - stereo_filter_delay uses a linear buffer with memmove on each frame to slide consumed samples back to position 0. A circular buffer would eliminate the memmove but only matters when the user opts into the delay filter (off by default). Build clean under -std=gnu11 with -Wno-write-strings -Wsign-compare -Wundef -Wmissing-prototypes; zero errors, zero warnings. audit_determinism.py: no rand/time/long double/threads issues. --- src/drivers/libretro/libretro.c | 9 ++++-- src/filter.c | 50 +++++++++++++++--------------- src/filter.h | 9 ++++++ src/sound.c | 54 ++++++++++++++++++++++++++++----- 4 files changed, 85 insertions(+), 37 deletions(-) diff --git a/src/drivers/libretro/libretro.c b/src/drivers/libretro/libretro.c index dc8d5b8..04a7d89 100644 --- a/src/drivers/libretro/libretro.c +++ b/src/drivers/libretro/libretro.c @@ -1067,9 +1067,12 @@ static void stereo_filter_apply_delay(int32_t *sound_buffer, size_t size) stereo_filter_delay.samples_size = tmp_buffer_size; } - for (i = 0; i < size; i++) - stereo_filter_delay.samples[i + - stereo_filter_delay.samples_pos] = sound_buffer[i]; + /* Copy current samples into the delay buffer's tail. The previous + * implementation walked the array element-by-element; memcpy is + * trivially equivalent for non-overlapping int32_t blocks and + * lets the compiler/libc dispatch SIMD where available. */ + memcpy(stereo_filter_delay.samples + stereo_filter_delay.samples_pos, + sound_buffer, size * sizeof(int32_t)); stereo_filter_delay.samples_pos += size; diff --git a/src/filter.c b/src/filter.c index 3e4f364..b2272c5 100644 --- a/src/filter.c +++ b/src/filter.c @@ -1,4 +1,3 @@ -#include #include "fceu-types.h" #include "sound.h" @@ -11,35 +10,34 @@ static uint32_t mrindex; static uint32_t mrratio; -void SexyFilter2(int32_t *in, int32_t count) { - #ifdef moo - static int64_t acc = 0; - double x, p; - int64_t c; +int64_t sexyfilter_acc1 = 0, sexyfilter_acc2 = 0; - x = 2 * M_PI * 6000 / FSettings.SndRate; - p = ((double)2 - cos(x)) - sqrt(pow((double)2 - cos(x), 2) - 1); +/* SexyFilter2's separate accumulator. Like sexyfilter_acc1/_acc2 it + * was a function-local static, which meant FCEUSND_Power couldn't + * reset it - cart B in a long-running process would inherit cart A's + * lowpass state. Lifted to file scope alongside the others so + * SexyFilter_Reset can zero it too. */ +int64_t sexyfilter2_acc = 0; - c = p * 0x100000; - #endif - static int64_t acc = 0; - - while (count--) { - int64_t dropcurrent; - dropcurrent = ((*in << 16) - acc) >> 3; - - acc += dropcurrent; - *in = acc >> 16; - in++; -#if 0 - acc=((int64_t)0x100000-c)* *in + ((c*acc)>>20); - *in=acc>>20; - in++; -#endif - } +void SexyFilter_Reset(void) +{ + sexyfilter_acc1 = 0; + sexyfilter_acc2 = 0; + sexyfilter2_acc = 0; + /* mrindex is reset in MakeFilters, which FCEUI_Sound calls on + * every cart load, so it doesn't need to be reset here. */ } -int64_t sexyfilter_acc1 = 0, sexyfilter_acc2 = 0; +void SexyFilter2(int32_t *in, int32_t count) { + while (count--) { + int64_t dropcurrent; + dropcurrent = ((*in << 16) - sexyfilter2_acc) >> 3; + + sexyfilter2_acc += dropcurrent; + *in = sexyfilter2_acc >> 16; + in++; + } +} void SexyFilter(int32_t *in, int32_t *out, int32_t count) { int32_t mul1, mul2, vmul; diff --git a/src/filter.h b/src/filter.h index e881280..17c6b12 100644 --- a/src/filter.h +++ b/src/filter.h @@ -6,6 +6,15 @@ void MakeFilters(int32_t rate); void SexyFilter(int32_t *in, int32_t *out, int32_t count); void SexyFilter2(int32_t *in, int32_t count); +/* Zero the IIR filter state. The accumulators are saved/loaded with + * the rest of the sound state (sound.c FAC1/FAC2 entries), but they + * are not reinitialised by FCEUSND_Power on cart load. Without this, + * starting cart B in a process that previously ran cart A inherits + * cart A's IIR state - audibly minor but breaks frame-determinism for + * the first samples of cart B. */ +void SexyFilter_Reset(void); + extern int64_t sexyfilter_acc1, sexyfilter_acc2; +extern int64_t sexyfilter2_acc; #endif diff --git a/src/sound.c b/src/sound.c index 2fb2981..81fb6e3 100644 --- a/src/sound.c +++ b/src/sound.c @@ -649,8 +649,12 @@ static void RDoSQLQ(void) { totalout = wlookup1[ ttable[0][RectDutyCount[0]] + ttable[1][RectDutyCount[1]] ]; if (!inie[0] && !inie[1]) { - for (V = start; V < end; V++) - Wave[V >> 4] += totalout; + /* Both squares silent: amp[x] was forced to 0 above (line + * "if (!inie[x]) amp[x] = 0"), which propagates through + * ttable[x] to make totalout = wlookup1[0] = 0. The + * previous code looped (end - start) iterations adding 0 + * to Wave[V>>4] - genuinely no-op for ~30000 NES cycles + * per frame. Skip the loop entirely. */ } else { for (V = start; V < end; V++) { /* int tmpamp=0; @@ -970,8 +974,23 @@ int FlushEmulateSound(void) { end = NeoFilterSound(WaveHi, WaveFinal, SOUNDTS, &left); + /* Slide the trailing `left` coefficient-history samples back + * to the start of the buffer for next frame's filter, then + * clear the area between left and SOUNDTS so next frame's + * channel accumulators start at zero. + * + * The previous code cleared all the way to sizeof(WaveHi), + * but only indices [left, SOUNDTS) were dirtied this frame - + * everything past SOUNDTS is still zero from the prior + * frame's clear (or from FCEUSND_Power on first frame). + * WaveHi is 40000 entries = 160 KB; SOUNDTS is bounded by + * NES cycles per frame (~30000), so this saves ~40 KB of + * memset per HQ frame. The (SOUNDTS > left) guard handles + * the degenerate case of a very short frame where SOUNDTS + * may not have advanced past the coefficient history. */ memmove(WaveHi, WaveHi + SOUNDTS - left, left * sizeof(uint32_t)); - memset(WaveHi + left, 0, sizeof(WaveHi) - left * sizeof(uint32_t)); + if ((uint32_t)SOUNDTS > (uint32_t)left) + memset(WaveHi + left, 0, (SOUNDTS - left) * sizeof(uint32_t)); if (GameExpSound.HiSync) GameExpSound.HiSync(left); for (x = 0; x < 5; x++) @@ -1069,6 +1088,13 @@ void FCEUSND_Power(void) { soundtsoffs = 0; IRQFrameMode = 0x1; /* Only initialized by power-on reset, not by soft reset. NRS: don't start with Frame IRQ enabled for greater compatibility. Any game that actually uses frame IRQ will explicitly enable it, anyway. */ LoadDMCPeriod(DMCFormat & 0xF); + + /* Reset post-mix filter accumulators. These are file-scope in + * filter.c and were not previously cleared on cart load, so a + * second cart loaded in the same process inherited the first + * cart's IIR state. Audibly minor on its own but breaks + * frame-determinism for the first samples of a new run. */ + SexyFilter_Reset(); } @@ -1096,12 +1122,23 @@ void SetSoundVariables(void) { DoSQ1 = RDoSQ1; DoSQ2 = RDoSQ2; } else { - DoNoise = DoTriangle = DoPCM = DoSQ1 = DoSQ2 = Dummyfunc; - DoSQ1 = RDoSQLQ; - DoSQ2 = RDoSQLQ; + /* In LQ mode the squares are mixed by RDoSQLQ in a single + * call, and the triangle/noise/PCM channels are mixed by + * RDoTriangleNoisePCMLQ in a single call - both functions + * advance their respective ChannelBC and process all the + * channels they handle in one pass. The previous code + * pointed DoTriangle/DoNoise/DoPCM all at + * RDoTriangleNoisePCMLQ, which meant the second and third + * call entered the function only to be rejected by the + * "if (end <= start) return;" guard at the top. Same for + * DoSQ1/DoSQ2 -> RDoSQLQ. Map only one entry each to the + * real worker and stub the others so we save the redundant + * call+early-return overhead. */ + DoSQ1 = RDoSQLQ; + DoSQ2 = Dummyfunc; DoTriangle = RDoTriangleNoisePCMLQ; - DoNoise = RDoTriangleNoisePCMLQ; - DoPCM = RDoTriangleNoisePCMLQ; + DoNoise = Dummyfunc; + DoPCM = Dummyfunc; } } else { DoNoise = DoTriangle = DoPCM = DoSQ1 = DoSQ2 = Dummyfunc; @@ -1221,6 +1258,7 @@ SFORMAT FCEUSND_STATEINFO[] = { { &wlcount[3], sizeof(wlcount[3]) | FCEUSTATE_RLSB, "WLC4" }, { &sexyfilter_acc1, sizeof(sexyfilter_acc1) | FCEUSTATE_RLSB, "FAC1" }, { &sexyfilter_acc2, sizeof(sexyfilter_acc2) | FCEUSTATE_RLSB, "FAC2" }, + { &sexyfilter2_acc, sizeof(sexyfilter2_acc) | FCEUSTATE_RLSB, "FAC3" }, { &lq_tcout, sizeof(lq_tcout) | FCEUSTATE_RLSB, "TCOU"}, /* 2018-12-14 - Wii and possibly other big-endian platforms are having