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
`<math.h>` 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.
This commit is contained in:
@@ -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;
|
stereo_filter_delay.samples_size = tmp_buffer_size;
|
||||||
}
|
}
|
||||||
|
|
||||||
for (i = 0; i < size; i++)
|
/* Copy current samples into the delay buffer's tail. The previous
|
||||||
stereo_filter_delay.samples[i +
|
* implementation walked the array element-by-element; memcpy is
|
||||||
stereo_filter_delay.samples_pos] = sound_buffer[i];
|
* 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;
|
stereo_filter_delay.samples_pos += size;
|
||||||
|
|
||||||
|
|||||||
50
src/filter.c
50
src/filter.c
@@ -1,4 +1,3 @@
|
|||||||
#include <math.h>
|
|
||||||
#include "fceu-types.h"
|
#include "fceu-types.h"
|
||||||
|
|
||||||
#include "sound.h"
|
#include "sound.h"
|
||||||
@@ -11,35 +10,34 @@
|
|||||||
static uint32_t mrindex;
|
static uint32_t mrindex;
|
||||||
static uint32_t mrratio;
|
static uint32_t mrratio;
|
||||||
|
|
||||||
void SexyFilter2(int32_t *in, int32_t count) {
|
int64_t sexyfilter_acc1 = 0, sexyfilter_acc2 = 0;
|
||||||
#ifdef moo
|
|
||||||
static int64_t acc = 0;
|
|
||||||
double x, p;
|
|
||||||
int64_t c;
|
|
||||||
|
|
||||||
x = 2 * M_PI * 6000 / FSettings.SndRate;
|
/* SexyFilter2's separate accumulator. Like sexyfilter_acc1/_acc2 it
|
||||||
p = ((double)2 - cos(x)) - sqrt(pow((double)2 - cos(x), 2) - 1);
|
* 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;
|
void SexyFilter_Reset(void)
|
||||||
#endif
|
{
|
||||||
static int64_t acc = 0;
|
sexyfilter_acc1 = 0;
|
||||||
|
sexyfilter_acc2 = 0;
|
||||||
while (count--) {
|
sexyfilter2_acc = 0;
|
||||||
int64_t dropcurrent;
|
/* mrindex is reset in MakeFilters, which FCEUI_Sound calls on
|
||||||
dropcurrent = ((*in << 16) - acc) >> 3;
|
* every cart load, so it doesn't need to be reset here. */
|
||||||
|
|
||||||
acc += dropcurrent;
|
|
||||||
*in = acc >> 16;
|
|
||||||
in++;
|
|
||||||
#if 0
|
|
||||||
acc=((int64_t)0x100000-c)* *in + ((c*acc)>>20);
|
|
||||||
*in=acc>>20;
|
|
||||||
in++;
|
|
||||||
#endif
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
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) {
|
void SexyFilter(int32_t *in, int32_t *out, int32_t count) {
|
||||||
int32_t mul1, mul2, vmul;
|
int32_t mul1, mul2, vmul;
|
||||||
|
|||||||
@@ -6,6 +6,15 @@ void MakeFilters(int32_t rate);
|
|||||||
void SexyFilter(int32_t *in, int32_t *out, int32_t count);
|
void SexyFilter(int32_t *in, int32_t *out, int32_t count);
|
||||||
void SexyFilter2(int32_t *in, 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 sexyfilter_acc1, sexyfilter_acc2;
|
||||||
|
extern int64_t sexyfilter2_acc;
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
|
|||||||
54
src/sound.c
54
src/sound.c
@@ -649,8 +649,12 @@ static void RDoSQLQ(void) {
|
|||||||
totalout = wlookup1[ ttable[0][RectDutyCount[0]] + ttable[1][RectDutyCount[1]] ];
|
totalout = wlookup1[ ttable[0][RectDutyCount[0]] + ttable[1][RectDutyCount[1]] ];
|
||||||
|
|
||||||
if (!inie[0] && !inie[1]) {
|
if (!inie[0] && !inie[1]) {
|
||||||
for (V = start; V < end; V++)
|
/* Both squares silent: amp[x] was forced to 0 above (line
|
||||||
Wave[V >> 4] += totalout;
|
* "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 {
|
} else {
|
||||||
for (V = start; V < end; V++) {
|
for (V = start; V < end; V++) {
|
||||||
/* int tmpamp=0;
|
/* int tmpamp=0;
|
||||||
@@ -970,8 +974,23 @@ int FlushEmulateSound(void) {
|
|||||||
|
|
||||||
end = NeoFilterSound(WaveHi, WaveFinal, SOUNDTS, &left);
|
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));
|
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);
|
if (GameExpSound.HiSync) GameExpSound.HiSync(left);
|
||||||
for (x = 0; x < 5; x++)
|
for (x = 0; x < 5; x++)
|
||||||
@@ -1069,6 +1088,13 @@ void FCEUSND_Power(void) {
|
|||||||
soundtsoffs = 0;
|
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. */
|
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);
|
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;
|
DoSQ1 = RDoSQ1;
|
||||||
DoSQ2 = RDoSQ2;
|
DoSQ2 = RDoSQ2;
|
||||||
} else {
|
} else {
|
||||||
DoNoise = DoTriangle = DoPCM = DoSQ1 = DoSQ2 = Dummyfunc;
|
/* In LQ mode the squares are mixed by RDoSQLQ in a single
|
||||||
DoSQ1 = RDoSQLQ;
|
* call, and the triangle/noise/PCM channels are mixed by
|
||||||
DoSQ2 = RDoSQLQ;
|
* 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;
|
DoTriangle = RDoTriangleNoisePCMLQ;
|
||||||
DoNoise = RDoTriangleNoisePCMLQ;
|
DoNoise = Dummyfunc;
|
||||||
DoPCM = RDoTriangleNoisePCMLQ;
|
DoPCM = Dummyfunc;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
DoNoise = DoTriangle = DoPCM = DoSQ1 = DoSQ2 = Dummyfunc;
|
DoNoise = DoTriangle = DoPCM = DoSQ1 = DoSQ2 = Dummyfunc;
|
||||||
@@ -1221,6 +1258,7 @@ SFORMAT FCEUSND_STATEINFO[] = {
|
|||||||
{ &wlcount[3], sizeof(wlcount[3]) | FCEUSTATE_RLSB, "WLC4" },
|
{ &wlcount[3], sizeof(wlcount[3]) | FCEUSTATE_RLSB, "WLC4" },
|
||||||
{ &sexyfilter_acc1, sizeof(sexyfilter_acc1) | FCEUSTATE_RLSB, "FAC1" },
|
{ &sexyfilter_acc1, sizeof(sexyfilter_acc1) | FCEUSTATE_RLSB, "FAC1" },
|
||||||
{ &sexyfilter_acc2, sizeof(sexyfilter_acc2) | FCEUSTATE_RLSB, "FAC2" },
|
{ &sexyfilter_acc2, sizeof(sexyfilter_acc2) | FCEUSTATE_RLSB, "FAC2" },
|
||||||
|
{ &sexyfilter2_acc, sizeof(sexyfilter2_acc) | FCEUSTATE_RLSB, "FAC3" },
|
||||||
{ &lq_tcout, sizeof(lq_tcout) | FCEUSTATE_RLSB, "TCOU"},
|
{ &lq_tcout, sizeof(lq_tcout) | FCEUSTATE_RLSB, "TCOU"},
|
||||||
|
|
||||||
/* 2018-12-14 - Wii and possibly other big-endian platforms are having
|
/* 2018-12-14 - Wii and possibly other big-endian platforms are having
|
||||||
|
|||||||
Reference in New Issue
Block a user