Skip to content

Ignore osc references that reach outside the voice being configured - #1127

Merged
dpwe merged 4 commits into
mainfrom
oob_voice_oscs
Aug 20, 2026
Merged

Ignore osc references that reach outside the voice being configured#1127
dpwe merged 4 commits into
mainfrom
oob_voice_oscs

Conversation

@dpwe

@dpwe dpwe commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

The bug

An osc number in a command addressed to a synth is voice-relative: base_osc is added to reach the real osc. Nothing checked the result, so a number past the end of the voice addressed — or pointed at — whatever happened to live there, which is another voice of the same synth, or another synth entirely.

amy.send(synth=1, num_voices=2, oscs_per_voice=1)
amy.send(synth=2, num_voices=2, oscs_per_voice=1)
amy.send(synth=2, osc=0, freq=4)
amy.send(synth=1, osc=0, freq={'mod': 0.1}, mod_source=1)   # synth 1 has ONE osc

That last line used to FM synth 1 from synth 2's oscillator. The new ctest, run against the pre-change sources, puts it plainly: FAIL osc base+1 untouched (OVERWRITTEN).

The change

Every place that combines an osc number with a base_osc now takes the voice's osc count and checks against it, through one helper:

bool osc_ref_within_voice(int rel_osc, uint16_t oscs_per_voice, const char *what);
  • amy_event_to_deltas_queue(e, base_osc, oscs_per_voice, queue) — checks the osc the event addresses, plus each reference it carries: chained_osc, mod_source[], algo_source[]. An out-of-range reference is dropped and the rest of the event still applies; an out-of-range target drops the osc-addressed part.
  • add_deltas_to_queue_with_baseosc(d, base_osc, oscs_per_voice, queue, time) — the same for a stored patch replayed into a voice.
  • oscs_per_voice == 0 means no voice context — absolute osc numbers from the C API and the MIDI-mapping path, or a patch being described rather than played — and nothing is checked.

The message names the parameter and the bound (mod_source osc 1 is outside this voice's 1 osc, ignored). It prints once per voice, which is where the combination happens.

EVENT_TO_DELTA_WITH_BASEOSC is gone; its last user was reset_osc, whose payload is an osc number sometimes and a mask of RESET_* bits the rest of the time. That choice now sits at the call site, decided the way play_delta decides it: an osc number goes through the checked macro, a mask through EVENT_TO_DELTA_I untouched. Incidentally that stops something meaningless — the old macro added base_osc to masks too, so reset=RESET_ALL_OSCS on a synth arrived as 8192 + base_osc and only worked because the mask bit survives the addition.

The subtlety: the bound has to be read live

A patch string can re-shape its own voice as it runs. The drum kits open with if3iv1in38Z, so patch_oscs[384] says 1 osc until that first command executes and turns the voice into 38. My first version passed the count known before the string ran and rejected the entire rest of the patch — 14 tests failed, every drum kit silent. oscs_in_voice_now() re-reads the owning voice's size per command instead.

Testing

  • tests/test_voice_osc_range.c (new, in make ctest): what is refused (mod_source, chained_osc, algo_source, an addressed osc, a reset carrying an osc number — each past the voice), what is kept (the same in range; a reset carrying a mask), that a no-synth event is absolute and unbounded, that the osc a refused reference would have reached is untouched, and that a patch re-shaping its own voice is not fought by the bound (all 38 kit oscs configured). 5 checks fail against pre-change sources.
  • amy/test.py: TestModOscOOB — the rendering case this started from. Its reference was generated only after confirming the audio with the out-of-range mod_source is bit-identical (max abs diff 0.0) to the same patch with that parameter never sent: "ignored" really means ignored.
  • make ctest 11/11, make test 133/133.

Docs

docs/synth.md gains a note that osc numbers in synth-addressed commands are voice-relative, that mod_source/chained_osc/algo_source are too, and that anything outside the voice is refused rather than landing on a neighbour. docs/api.md's v row points at it.

🤖 Generated with Claude Code

dpwe and others added 4 commits August 19, 2026 21:08
An osc number in a synth-directed command is voice-relative: base_osc is
added to reach the real osc. Nothing checked the result, so a number past
the end of the voice silently addressed -- or pointed at -- whatever
happened to live there, which is another voice of the same synth, or
another synth entirely. amy.send(synth=1, osc=0, mod_source=1) on a
one-osc voice FM'd itself from a neighbour's oscillator.

Every place that combines an osc number with a base_osc now takes the
voice's osc count and checks against it:

- amy_event_to_deltas_queue() takes oscs_per_voice, and checks the osc
  the event addresses plus each reference it carries (chained_osc,
  mod_source, algo_source). An out-of-range reference is dropped and the
  rest of the event still applies; an out-of-range target drops the
  osc-addressed part of the event.
- add_deltas_to_queue_with_baseosc() checks the same for a stored patch
  replayed into a voice.
- 0 means no voice context -- absolute osc numbers from the C API and
  the MIDI mapping path, or a patch being described rather than played
  -- and nothing is checked.

RESET_OSC is deliberately not checked: its payload is a mask of RESET_*
bits as often as it is an osc number, so a range test would reject
things like RESET_ALL_OSCS.

The bound has to be read live, not captured. A patch string can re-shape
its own voice as it runs: the drum kits start with `if3iv1in38Z`, so
patch_oscs says 1 osc until that first command executes and turns the
voice into 38. Binding to the value known before the string ran rejected
the whole rest of the patch and silenced every drum test.

amy/test.py's TestModOscOOB (the case this started from) now has a
reference. It was generated only after checking that the rendered audio
with the out-of-range mod_source is bit-identical to the same patch with
that parameter never sent -- "ignored" really means ignored.

make ctest 10/10, make test 133/133.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It had one user left. reset_osc's payload is an osc number sometimes and
a mask of RESET_* bits the rest of the time, so the choice belongs at the
call site, decided the same way play_delta decides it: an osc number gets
EVENT_TO_DELTA_OSC_REF (voice-relative -- range-checked and offset by
base_osc), a mask gets EVENT_TO_DELTA_I (through untouched).

That also stops something meaningless: the old macro added base_osc to
whatever it was given, so a synth-directed reset=RESET_ALL_OSCS arrived
as 8192 + base_osc. It kept working only because the mask bit survives
the addition.

OSC_REF keeps the "don't allocate for a reset" exemption the old macro
had: reset_osc() is a no-op on an unallocated osc, which is already at
its defaults, so materializing one to clear it would undo #1106.

Checked both payload forms by hand: reset=1 on a 2-osc voice clears that
voice's osc 1 and leaves osc 0; reset=3 on a 1-osc voice is refused and
changes nothing; reset=RESET_ALL_OSCS through a synth still clears it.

make ctest 10/10, make test 133/133.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tests/test_voice_osc_range.c covers what is refused (mod_source,
chained_osc, algo_source, an addressed osc, a reset carrying an osc
number -- each past the end of the voice), what is kept (the same
references in range, and a reset carrying a RESET_* mask, which is not
an osc number at all), that an event with no synth is absolute and
unbounded, and that the osc a refused reference would have reached is
left untouched.

That last one is the point of the exercise: against the pre-change
sources it reports OVERWRITTEN, because the number reached into the
neighbouring synth's oscillator. Five checks fail there in total.

It also pins the case that must NOT be refused -- a patch string that
re-shapes its own voice as it runs. The drum kits open with `if3iv1in38Z`
and a bound captured before the string ran rejected everything after it,
silencing every kit; the test asserts all 38 of the kit's oscs end up
configured.

docs/synth.md gains a note that osc numbers in synth-addressed commands
are voice-relative, that mod_source/chained_osc/algo_source are too, and
that anything outside the voice is refused rather than landing on a
neighbour. api.md's `v` row points at it.

make ctest 11/11, make test 133/133.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bound the rest of a patch string is held to was worked out by
re-reading osc_to_voice/num_oscs_for_voice per command, which noticed a
mid-string re-shape only as a side effect: correct, but you had to know
the voice tables were being consulted behind the loop to see why a drum
kit's 38 oscs were not rejected by a bound of 1.

The loop now watches for it directly. An event carrying oscs_per_voice
re-shapes the voice, so it updates the local bound as it goes by, and
everything after it answers to the new size. That is the thing that
actually happens, written where it happens.

Applying it from that event rather than the next is right: the event
carrying oscs_per_voice is synth-level and names no oscs of its own.

add_deltas_to_queue_with_baseosc goes back to the bound it is passed --
its deltas are already parsed, and oscs_per_voice is a parse-time
command, so nothing in a delta list can re-shape anything. oscs_in_voice_now
is gone with both of its callers.

make ctest 11/11 (including the drum kit case), make test 133/133.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@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 @ 317f73d this PR Δ
1 1043 1063 +20
2 1206 1226 +20
3 1800 1812 +12
4 1977 1995 +18
5 2641 2656 +15
6 2789 2800 +11

Full chord settled render μs: 2800 (was 2791, Δ +0.3%) (peak 2811, 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 d2785b4 into main Aug 20, 2026
12 checks passed
@dpwe
dpwe deleted the oob_voice_oscs branch August 20, 2026 01:57
@bwhitman

Copy link
Copy Markdown
Collaborator

⛓️ tulipcc integration PR opened

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

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