peterswimm/potatochips
Watch
1
0
Fork
You've already forked potatochips
 0
Code Issues Pull requests Projects Releases Packages Activity Actions

Unblock the DSP unit tests, fix the built-in sample, and add CI #2

Merged
peterswimm merged 3 commits from claude/port-distong-nt-native-c-2yibw6 into master 2026-08-20 16:01:52 +00:00
Conversation 0 Commits 3 Files changed 18 +231 -88
peterswimm commented 2026-08-19 22:44:15 +00:00 (Migrated from github.com)
Copy link

Description

Follow-up to #1, on a branch restarted from master. Three changes.

src/dsp/exceptions.hpp is now self-contained. It derived its two exception types from a base class Exception that the same file leaves commented out, so the DSP headers only compiled with rack::Exception in scope. Two of the repo's twelve test suites therefore could not be built at all — test/dsp/test_blip_buffer.cpp and test/dsp/sony_s_dsp/test_processor.cpp — and one of those covers blip_buffer.hpp, the file that turned out to hold an out-of-bounds write on every audio sample (fixed in #1). That test being unbuildable is a plausible reason the bug survived.

Restoring the class under its own name does not work — the Rack build does using namespace rack;, which makes every unqualified use ambiguous, and that is presumably why it was commented out in the first place. I verified that empirically against the Rack SDK before choosing a name. It is now DSPException; nothing anywhere catches these, so the rename changes no behaviour. The disting NT compat shim follows it, and still replaces the header rather than using it, because the real one builds its messages with std::string and nothing in that port allocates.

Super Sampler's built-in sample was packed wrongly, in two compounding ways. Neither nibble was masked to four bits, so a negative sample's sign extension filled its neighbour; and the pair was written in the opposite order to the one the decoder reads — BRR_SamplePlayer takes a byte's high nibble before its low one, so the earlier sample belongs in the high nibble. Decoding the shipped bytes back reproduces 5560 of the 13082 packed samples incorrectly (42.5%). Masking alone makes it slightly worse (44.7%), because it fixes the corruption while leaving the order reversed. Both together reproduce the wave exactly. This changes how the sample sounds — it sounds like the recording now.

CI. Nothing ran on pull requests; .travis.yml has been dead for years. .github/workflows/build.yml adds three jobs: the twelve Catch2 DSP suites, the Rack plugin against the Rack SDK, and the sixteen disting NT algorithms for the Cortex-M7 — the last gated on the Arm build staying warning-free and on no object referencing an allocator or the C++ ABI runtime, then the host harness under the sanitizers over six seeds. Each gate was run locally against this tree first.

A correction

distingNT/README.md and #1's description both claimed Super VCA's loudness compensation ran the wrong way round. That was wrong, and this PR corrects it. GaussianInterpolationFilter::getFilterLabel() names the emulator's filter value, where 0 is "Barely Audible" and 3 is "Loud", so setFilter(3 - mode) correctly pairs the quietest filter with the largest 2^mode boost. Measured with a 100 Hz tone, raw output halves as the filter weakens — 0.271, 0.138, 0.065 RMS — and the compensation brings those to 0.271, 0.275, 0.261, equal within 5%. The mapping is left exactly as it was.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change alters existing audio output (Super Sampler's built-in sample)

How Has This Been Tested?

  • All twelve Catch2 suites build and pass — 160 assertions, no shim. Two of them could not be built before this branch; verified they fail on master too (9 and 4 errors)
  • The Rack plugin builds against the Rack 2.5.2 SDK, no errors — this is the first time the Rack side of the #1 DSP changes has actually been compiled
  • The sixteen disting NT algorithms build warning-free for the Cortex-M7, and none references an allocator or __cxa_*
  • The host harness passes for all sixteen under ASan + UBSan
  • Each CI gate was run verbatim against this tree before being committed
  • The packing fix was validated by decoding the packed RAM the way BRR_SamplePlayer does and diffing against the source wave, across all 13,083 samples
  • The Super VCA claim was checked by measuring output RMS per filter mode rather than by reading the code again

Still not tested on hardware, and no algorithm's sound has been compared against its Rack counterpart on a real module.

Test Configuration

  • Operating System: Linux 6.18.5
  • C++ compiler version: arm-none-eabi-g++ 13.2.1 (target), g++ 14 with ASan/UBSan (host)
  • Rack SDK 2.5.2, Catch2 2.13.1

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation following the
    Doxygen style
  • I have tested that my fix is effective or that my feature works

One thing I did not do

.travis.yml is dead and now sits beside a live workflow, which is confusing. Deleting it felt beyond the ask — say the word and it goes.

Generated by Claude Code

### Description Follow-up to #1, on a branch restarted from `master`. Three changes. **`src/dsp/exceptions.hpp` is now self-contained.** It derived its two exception types from a base class `Exception` that the same file leaves commented out, so the DSP headers only compiled with `rack::Exception` in scope. Two of the repo's twelve test suites therefore could not be built at all — `test/dsp/test_blip_buffer.cpp` and `test/dsp/sony_s_dsp/test_processor.cpp` — and one of those covers `blip_buffer.hpp`, the file that turned out to hold an out-of-bounds write on *every audio sample* (fixed in #1). That test being unbuildable is a plausible reason the bug survived. Restoring the class under its own name does not work — the Rack build does `using namespace rack;`, which makes every unqualified use ambiguous, and that is presumably why it was commented out in the first place. I verified that empirically against the Rack SDK before choosing a name. It is now `DSPException`; nothing anywhere catches these, so the rename changes no behaviour. The disting NT compat shim follows it, and still replaces the header rather than using it, because the real one builds its messages with `std::string` and nothing in that port allocates. **Super Sampler's built-in sample was packed wrongly, in two compounding ways.** Neither nibble was masked to four bits, so a negative sample's sign extension filled its neighbour; and the pair was written in the opposite order to the one the decoder reads — `BRR_SamplePlayer` takes a byte's high nibble before its low one, so the earlier sample belongs in the high nibble. Decoding the shipped bytes back reproduces **5560 of the 13082 packed samples incorrectly (42.5%)**. Masking alone makes it slightly *worse* (44.7%), because it fixes the corruption while leaving the order reversed. Both together reproduce the wave exactly. **This changes how the sample sounds** — it sounds like the recording now. **CI.** Nothing ran on pull requests; `.travis.yml` has been dead for years. `.github/workflows/build.yml` adds three jobs: the twelve Catch2 DSP suites, the Rack plugin against the Rack SDK, and the sixteen disting NT algorithms for the Cortex-M7 — the last gated on the Arm build staying warning-free and on no object referencing an allocator or the C++ ABI runtime, then the host harness under the sanitizers over six seeds. Each gate was run locally against this tree first. ### A correction `distingNT/README.md` and #1's description both claimed Super VCA's loudness compensation ran the wrong way round. **That was wrong, and this PR corrects it.** `GaussianInterpolationFilter::getFilterLabel()` names the *emulator's* filter value, where 0 is "Barely Audible" and 3 is "Loud", so `setFilter(3 - mode)` correctly pairs the quietest filter with the largest `2^mode` boost. Measured with a 100 Hz tone, raw output halves as the filter weakens — 0.271, 0.138, 0.065 RMS — and the compensation brings those to 0.271, 0.275, 0.261, equal within 5%. The mapping is left exactly as it was. ### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [x] This change alters existing audio output (Super Sampler's built-in sample) ### How Has This Been Tested? - [x] **All twelve Catch2 suites build and pass** — 160 assertions, no shim. Two of them could not be built before this branch; verified they fail on `master` too (9 and 4 errors) - [x] **The Rack plugin builds** against the Rack 2.5.2 SDK, no errors — this is the first time the Rack side of the #1 DSP changes has actually been compiled - [x] **The sixteen disting NT algorithms build warning-free** for the Cortex-M7, and none references an allocator or `__cxa_*` - [x] **The host harness passes for all sixteen** under ASan + UBSan - [x] Each CI gate was run verbatim against this tree before being committed - [x] The packing fix was validated by decoding the packed RAM the way `BRR_SamplePlayer` does and diffing against the source wave, across all 13,083 samples - [x] The Super VCA claim was checked by measuring output RMS per filter mode rather than by reading the code again Still not tested on hardware, and no algorithm's sound has been compared against its Rack counterpart on a real module. ### Test Configuration - Operating System: Linux 6.18.5 - C++ compiler version: arm-none-eabi-g++ 13.2.1 (target), g++ 14 with ASan/UBSan (host) - Rack SDK 2.5.2, Catch2 2.13.1 ### Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation following the [Doxygen](https://www.doxygen.nl/manual/docblocks.html) style - [x] I have tested that my fix is effective or that my feature works ### One thing I did not do `.travis.yml` is dead and now sits beside a live workflow, which is confusing. Deleting it felt beyond the ask — say the word and it goes. --- _Generated by [Claude Code](https://claude.ai/code/session_018iqKYxro8ghrTPGr5Z4S51)_
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
bug
Something isn't working

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
wontfix
This will not be worked on

No labels
accessibility
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
peterswimm/potatochips!2
No description provided.