Skip to content

Restore real drum-kit osc counts in patch_oscs (kits 385+ play a sine after any kit switch) - #1149

Merged
dpwe merged 1 commit into
mainfrom
fix-drumkit-patch-oscs
Aug 31, 2026
Merged

Restore real drum-kit osc counts in patch_oscs (kits 385+ play a sine after any kit switch)#1149
dpwe merged 1 commit into
mainfrom
fix-drumkit-patch-oscs

Conversation

@bwhitman

Copy link
Copy Markdown
Collaborator

Bug

Since ddafbd9 (2026-07-24, the #966 osc-table overrun fix), loading any Gamma9001 drum kit other than 384 onto a fragmented oscillator pool — the normal state once any synth was allocated after the drum synth's block, i.e. right after default boot — leaves the kit's oscillators completely unconfigured. Every drum note then plays its note-map template (n60…) on a default osc: a ~261 Hz sine instead of a drum. Surfaced by a user report against the turning-machine-4 AMYboard World sketch ("changing the drum kit plays a sine wave instead of the kick; 384 works, 385+ don't"), reproducible on AMYboard hardware and the web editor.

Mechanism

ddafbd9 changed patch_oscs[] for the drum patches (258, 384–390) from their real counts to a placeholder 1, relying on the patches' leading if3iv1in42Z header to size the synth. But patches_load_patch() uses patch_oscs for the initial allocation, and parse_patch_string_to_queue() queues all the per-osc kit config deltas against that block's base osc. The in-string inXX realloc then runs a different-sized free-block search — growing from 1 to 38/42 oscs — and under fragmentation lands at a different base. The config deltas hit the old base; dump_state shows i10if3iv1in42Z followed by 42 empty i10v0Z… entries.

Kit 384 (38 oscs) usually escapes: switching kits frees the old kit's 38-osc hole and a 38-osc kit fits back into it at the same base. The 42-osc kits (385–390) never fit and always relocate. Bisected: good at ddafbd9^, bad at ddafbd9.

Fix

make_patches() emits the real osc counts again (258→38, 384→38, 385–390→42), so both allocations run the same-size search over the same pool state and land at the same base. 258's single patch_oscs entry covers two build-conditional string variants (gamma808 ROM = 38 oscs, pcm_tiny ROM = 32), so it uses the larger — over-allocating 6 oscs in the non-GAMMA9001 build is harmless, under-allocating retriggers the relocation. The actual #966 fix (the block-wrap check in the allocator search) is untouched.

Verification

  • Repro (native python module, gamma bank baked in): restart(1) (default synths) + one extra synth, then send(synth=10, patch=385), note 36 → FFT dominant 263.5 Hz (sine) before, 53.2 Hz (TR-909 kick) after. All kits 384–390 and 258 verified configured via dump_state under fragmentation.
  • make test: 133 tests pass.

🤖 Generated with Claude Code

ddafbd9 (the #966 osc-table overrun fix) also changed patch_oscs[] for the
drum patches (258, 384-390) to a placeholder 1, on the theory that the
patches size themselves via their leading 'inXX' header. But the initial
allocation sizes the osc block that the patch's per-osc config deltas are
queued against; when the in-string realloc has to grow the block from 1 to
38/42 oscs it can land at a different base under pool fragmentation --
which is the normal state once any synth was allocated after the drum
synth's block. The kit then comes up as 38-42 default oscillators and every
drum note plays the note map's n60 template on an unconfigured osc: a
~261 Hz sine instead of a drum. Kit 384 (38 oscs) usually escapes because
it fits back into the 38-osc hole the previous kit freed; 385-390 (42
oscs) never do.

Emit the real counts from make_patches again so both allocations run the
same-size search and land at the same base. 258's single entry covers two
build-conditional variants (gamma808=38, pcm_tiny=32), so use the larger.
The #966 block-wrap fix in patches.c is untouched.

Repro: default synths + any extra synth, then synth=10,patch=385, note 36
=> 263.5 Hz tone before, 53 Hz TR-909 kick after. All 133 tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bwhitman
bwhitman requested a review from dpwe August 30, 2026 23:58
@github-actions

Copy link
Copy Markdown
Contributor

🎛️ AMY HW CI (AMYboard bench)

Flashed this PR's AMY (LoadTestChord: 6-voice Juno patch=1, one held note every 2 s) onto the physical AMYboard and measured the smoothed render load as the chord grows — back-to-back with the same sketch built at the PR's merge base, so Δ is this PR's own cost.

PASS — the bench ran the test to completion.

notes held main @ 81cddfa this PR Δ
1 1048 1047 -1
2 1220 1223 +3
3 1831 1831 +0
4 2005 2002 -3
5 2658 2659 +1
6 2825 2821 -4

Full chord settled render μs: 2822 (was 2822, Δ +0.0%) (peak 2824, 39 samples)

⬇️ Artifacts: serial log · load trace · report

Self-hosted bench (amyboardci). FAIL means only that the test could not run — the load values are informational, with no threshold and no audio compare. See tools/arduino_loadsweep/.

@dpwe
dpwe merged commit 1d49f10 into main Aug 31, 2026
12 checks passed
@bwhitman

Copy link
Copy Markdown
Collaborator Author

⛓️ tulipcc integration PR opened

This merge was pinned into tulipcc for full-system CI: shorepine/tulipcc#1343

Test it there and merge that PR to move tulipcc onto this AMY.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants