Conversation
Adds opt-in NES APU emulation (frame counter, length/linear/sweep/envelope units, noise period LUT) and makes the master output stage configurable. Every feature is off by default, so the canonical golden PCM is unchanged apart from the deliberate TRIANGLE quantization below. Local accuracy: - TRIANGLE quantized to 4-bit NES levels, default on; golden regenerated - NES discrete duty cycles, opt-in per voice - NOISE LFSR seeded to 0x7FFF, matching the state after a $400F write - noiseShortMode renamed to noiseLfsrShort, deprecated alias kept in sync - Optional TRIANGLE octave doubling Frame counter and sub-units, all default off: - 4/5-step frame counter driving the quarter and half clocks - Length counter, linear counter, sweep unit and envelope unit - TRIGGER_NES_LENGTH command, plus the $400B linear-counter reload side effect that the length counter deferred and the linear counter never picked up - initNesSafe and InstrumentPreset::nesAccurate for a canonical silent starting state NOISE period LUT: - Canonical NTSC table stored in CPU cycles and converted at the current sample rate, so one index means one timbre at 22050, 44100 and 48000 Output stage: - Configurable HPF corner in Hz. The previous hardcoded coefficient pinned R rather than the cutoff, so the filter drifted with the sample rate (~35 Hz at 44100 but ~17.6 Hz at 22050). The default keeps the historical coefficient bit-for-bit - Configurable soft-clip: None, Rational, Tanh and HardClip. HardClip models the NES DAC saturating ahead of the analog high-pass - VoiceNesOptions and SET_NES_OPTIONS to arm a voice atomically, reachable from the game thread through the command queue Also fixes a sweep unit defect: it evaluated every PULSE voice regardless of opt-in, and the default timer of 0 satisfied the "period < 8" muting rule, so enabling the frame counter silenced every PULSE voice still driven by the legacy float frequency path. Now gated per voice by NesSweepUnit::unitEnabled. Verified: 172 tests in test_apu_core, 14 in test_q15_smoke, 5/5 ctest targets passing, golden PCM byte-for-byte identical to its regenerated baseline.
The directory is regenerated by tooling and was showing up as untracked on every status. Ignoring it keeps the working tree clean without touching the files themselves.
The integer path stacked two independent attenuations, so FPU-less targets rendered at ~0.225x the level of ESP32/ESP32-S3 for identical input. MIXER_SCALE was applied twice: once per channel before summation, and again inside audio_mixer_lut. The table maps a RAW four-channel sum -- its curve is 32767*x/(1+|x|*0.5) with x = sum*1.6/131072, so LUT[0] = -29126 is the curve at -1.6, not at -1.0. Pre-scaling capped the accumulator at +-52428, leaving the outer 60% of the table unreachable and the compressor knee misplaced. On top of that sat the documented -6 dB pad, which has no float equivalent. ABI.md only ever accounted for the 0.5. The accumulator now carries the raw sum and MIXER_SCALE is applied exactly once downstream: by the LUT for Rational, explicitly for the computed curves. Removing the pad exposed a latent overflow it had been masking -- it kept the value below half scale, so the final int16 cast could not wrap. It can now, when an unshaped over-unity mix meets a bypassed HPF whose own clamp is skipped. Saturate before the cast, as the float branch always did. The canonical golden PCM is byte-for-byte identical and was NOT regenerated: test_apu_mirror compiles with PR32_APU_HAS_FPU=1, so the fixture comes from the float path, which this does not touch. Desktop, ESP32 and ESP32-S3 are unaffected; no-FPU consumers will hear ~4.4x and should re-check master volume. Tests pin both binaries to the closed-form value of the mixing chain (13107 / 10922 / 12519 at S = MIXER_SCALE), which is the only way to assert cross-path parity from two separate executables.
…struct Hito 0 inserted the alias immediately after noiseLfsrShort, at position 15 of InstrumentPreset. That pushed dutySweep, pitchSweepEndHz and pitchSweepDurationSec down one slot. Both consumers build presets with positional aggregate initializers written against the published 1.0.1 field order, so every one of those literals either failed to compile -- a float landing on the new bool is a narrowing conversion, ill-formed in aggregate init -- or silently rebound its trailing values. Measured before this fix: 45 literals across 7 files in the Game Engine, and 6 sites in the Tool Suite, two of them the factories behind ~65 catalog entries. With the alias last, positions 1..17 mean exactly what they meant in 1.0.1, so all 51 sites compile and map correctly untouched. Verified by compiling Engine literals and both Tool Suite factories verbatim against the header, without -Wno-narrowing (which this repo's own test targets do set, and which is why the shift surfaces here as a failed assertion rather than a build error). The alias is inert on InstrumentPreset -- only noiseLfsrShort drives synthesis -- so moving it changes no behaviour. INSTR_SNARE and INSTR_HIHAT set it explicitly as the trailing member so it stays in sync; deleting the element would have let it default to false and silently broken consumer tests that read it off those constants. Three positional literals in test_apu_core.cpp lose the extra element Hito 0 had forced on them -- the same patch the consumers were about to be asked to apply. Golden PCM byte-for-byte identical.
A `#if defined(UNIT_TEST)` block that legitimately wraps the *ForTesting accessors had grown to enclose 17 production methods too: the frame-counter setters, all four tickAllNes* dispatchers, every setVoiceNes* setter and the per-sample tickNesFrameCounter. Their call sites sit outside the guard, so libpixelroot32-apu.a compiled cleanly and then failed to link with 13 undefined references the moment a consumer used it. Nothing here caught it. Every test target recompiles src/ApuCore.cpp with UNIT_TEST=1, so the definitions always resolved; the library target is built with no UNIT_TEST and never linked by anything in this repo, and a static archive does not resolve symbols at build time. The defect was only visible downstream, which is where it was reported from. So the fix is two things. The guard now closes before the NES section and reopens after it -- and two test-only counters that production functions were incrementing unguarded get their own guards, since they were only compiling by virtue of sitting inside the block. The second is test_library_link: a ctest target that links pixelroot32-apu the way a consumer does instead of recompiling the source, compiled deliberately without UNIT_TEST and with an #error so it cannot drift. It calls the whole public surface and asserts almost nothing -- what it proves is that the symbols exist in the artifact consumers receive. Restoring the old guard boundary makes it fail to link. Golden PCM byte-for-byte identical.
Hito 5 (M12) made the integer render path about 4.4x louder. That path is what FPU-less targets run -- the ESP32-C3 the music-demo builds for -- so the level has to be checked for clipping and for parity with the float path. PR32_APU_HAS_FPU is a plain compile definition, so the exact integer arithmetic the C3 executes runs fine on the host. The probe builds from one source twice, plays a real music-demo track through each, and reports peak, RMS and the count of samples pinned at the rails. --wav-dir also dumps both renders for the listening comparison. No hardware needed for the part that matters. The demo's generated asset headers depend only on the APU types, so real tracks replay with no SDL and no game loop. RMS is the verdict; peak is informational, and the README explains why. The paths were never bit-identical: the float one evaluates the compressor curve continuously while the Q15 one looks it up in audio_mixer_lut, 1025 entries with no interpolation. Measured across the four demo tracks the RMS ratio lands both above and below 1.0 (0.990 to 1.094) -- scattered, so quantisation, not a gain error, which would be one-directional. Peak runs consistently high because it is a max over ~10^6 samples and catches the side the LUT rounds up. Exact gain staging stays pinned bit-exactly by test_q15_softclip_curve_shapes_match_the_float_path; this measures behaviour on real material instead.
Closes the Hito 0-5 roadmap. Three breaking changes, detailed in the CHANGELOG: 4-bit TRIANGLE quantisation on by default, the Q15 path renders ~4.4x louder to match the float path (FPU-less targets only), and the NOISE LFSR seeds to 0x7FFF. Everything else added is opt-in and default-off. CMakeLists.txt was declaring VERSION 1.0.0 while library.json said 1.0.1; both now read 2.0.0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the Hito 0-5 roadmap. Tag
v2.0.0is already pushed (b603b3a), whichthe publish workflow requires before this merges.
Breaking changes
triangleQuantize4Bit = false. Golden PCM regenerated.path. FPU-less targets only — ESP32-C3 and any
-DPR32_APU_HAS_FPU=0build.Desktop, ESP32 and ESP32-S3 are unchanged. Consumers on those targets should
re-check their master volume.
0x7FFFinstead of0x4000.Everything else is opt-in and default-off: the NES frame counter, the
length / linear / sweep / envelope units, the NES duty and noise period tables,
a configurable HPF and soft-clip, and per-voice NES options.
Two defects found while migrating the consumers
#if defined(UNIT_TEST)block had grown to enclose 17 production methods, sothe static lib compiled and then failed to link with 13 undefined references.
Invisible here because every test target recompiles
ApuCore.cppwithUNIT_TEST=1and nothing linked the library.test_library_linknow closesthat gap permanently.
noiseShortModealias sat mid-struct inInstrumentPreset,which broke 45 positional literals in the Game Engine and 6 in the Tool Suite.
Moved to the end, so positions 1..17 mean what they meant in 1.0.1 and all of
them compile untouched.
Verification
esp32-c3-devkitm-1compile check.confirmed by ear.
Merging this publishes 2.0.0 to the PlatformIO Registry.