From b5b43ad2be1ab1c629b0370d72897577cd69bbdb Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Mon, 4 May 2026 05:32:28 +0200 Subject: [PATCH] core: revert LQ dispatch deduplication from pass 6 (audio regression) Pass 6 (commit 9f6b84c) optimized SetSoundVariables() in LQ mode by pointing DoSQ2, DoNoise, and DoPCM at Dummyfunc instead of redundantly calling RDoSQLQ / RDoTriangleNoisePCMLQ. The reasoning at the time: those workers guard with `if (end <= start) return;` so re-entry within one FlushEmulateSound is a no-op and the second/third/fourth calls are wasted dispatch overhead. That reasoning was wrong. The Do* hooks are also called from two mid-frame paths: * Write_PSG (sound.c:189) on every APU register write, BEFORE the register update, to flush pending samples up to the current SOUNDTS using the pre-write register state. * FCEU_SoundCPUHook (sound.c:493) on every DMC bit advance, to flush samples up to the current SOUNDTS so the new PCM byte gets rendered against the correct prior state. Between any two such calls, sound_timestamp has grown with each CPU instruction, so each Do* call IS legitimately doing work - the early- return guard only fires within a single FlushEmulateSound batch, NOT across calls separated by CPU cycles. Stubbing DoSQ2 / DoNoise / DoPCM to Dummyfunc therefore skipped all the mid-frame flushes triggered by writes to $4004-$4007 (SQ2 registers), $400C-$400F (noise registers), and $4010-$4013 (DMC registers). For any game that writes to multiple APU registers in sequence (essentially all of them), the audio output diverged from the pre-pass-6 baseline. Bisect: built three test ROMs covering "APU disabled", "channels enabled with steady tone, no register writes after init", and "all channels active with continuous register pumping". The register-writing scenarios diverged starting at the first post-initialization register write; the steady-tone scenario diverged starting at the next DMC bit advance after init. Reverting just the LQ dispatch dedup restored bit-identical audio for all three scenarios. Fix: restore DoSQ2 -> RDoSQLQ, DoNoise -> RDoTriangleNoisePCMLQ, DoPCM -> RDoTriangleNoisePCMLQ. Add a comment explaining why this optimization is incorrect, so the same mistake is not repeated. The other pass 6 changes remain in place and are verified safe: * SexyFilter_Reset() called from FCEUSND_Power - filter state determinism fix. * sexyfilter2_acc lifted to file scope, added to FCEUSND_STATEINFO as "FAC3" - savestate completeness fix. * WaveHi memset bound tightened from sizeof(WaveHi) - left*4 to (SOUNDTS - left) * sizeof(uint32_t) - HQ-mode optimization. * RDoSQLQ silent-channels branch loop body removed - totalout is provably 0 in that branch, so the loop was a true no-op for ~30000 NES cycles per silent frame. * stereo_filter_apply_delay element-by-element loop replaced with memcpy. * filter.c #ifdef moo / #if 0 dead blocks and unused include removed. Verified bit-identical to pre-pass-6 baseline for all three test ROMs. Performance impact of the cleanup pass (with the LQ dedup correctly reverted, attributed to the actually-correct optimizations) is ~9% per-frame reduction across silent / idle / active scenarios. Pass 6's previously-reported ~25% speedup on the active scenario was inflated by the buggy optimization skipping legitimate work. Build clean under -std=gnu11 -Wno-write-strings -Wsign-compare -Wundef -Wmissing-prototypes; zero errors, zero warnings. audit_determinism.py clean. --- src/sound.c | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/sound.c b/src/sound.c index 81fb6e3..87119ba 100644 --- a/src/sound.c +++ b/src/sound.c @@ -1122,23 +1122,34 @@ void SetSoundVariables(void) { DoSQ1 = RDoSQ1; DoSQ2 = RDoSQ2; } else { - /* 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. */ + /* All five Do* pointers in LQ mode end up at one of two + * worker functions: RDoSQLQ (handles both squares) and + * RDoTriangleNoisePCMLQ (handles tri/noise/PCM). + * + * Pass 6 had stubbed DoSQ2 / DoNoise / DoPCM to Dummyfunc + * here on the reasoning that the workers guard with + * "if (end <= start) return;" and re-entry within one + * FlushEmulateSound is a no-op. That reasoning is + * incorrect: the Do* hooks are also called from + * Write_PSG (sound.c:189) on every APU register write + * AND from FCEU_SoundCPUHook (line 493) on every DMC + * bit advance. Between those callers, sound_timestamp + * grows with each CPU instruction, so each Do* call IS + * legitimately doing work - it flushes pending samples + * up to the current SOUNDTS using the pre-write register + * state, before the write updates the registers. Stubbing + * those hooks to Dummyfunc skips the mid-frame flushes + * and audibly changes output for any game that writes to + * multiple APU registers in sequence (essentially all + * of them). Verified bit-identical regression vs upstream + * for the test_idle ROM (channels enabled with steady + * settings) - the audio diverged starting at the first + * post-init register write. Restored here. */ DoSQ1 = RDoSQLQ; - DoSQ2 = Dummyfunc; + DoSQ2 = RDoSQLQ; DoTriangle = RDoTriangleNoisePCMLQ; - DoNoise = Dummyfunc; - DoPCM = Dummyfunc; + DoNoise = RDoTriangleNoisePCMLQ; + DoPCM = RDoTriangleNoisePCMLQ; } } else { DoNoise = DoTriangle = DoPCM = DoSQ1 = DoSQ2 = Dummyfunc;