Configurable note-priority / trigger / voicing for the polyphonic engine (fixes #252) - #1273
Configurable note-priority / trigger / voicing for the polyphonic engine (fixes #252)#1273thopman wants to merge 1 commit into
Conversation
…ame-cncm#252) Adds a held-note stack and a reconciliation core to mydsp_poly so the polyphonic engine behaves correctly for monophonic and duophonic instruments, not just true polyphony. Fixes the long-standing issue grame-cncm#252: with a single sounding voice, holding note A, pressing then releasing B now falls back to the still-held A instead of going silent. New per-DSP 'declare options' metadata (all default to the historical behavior, so existing patches are unaffected): [voicing:mono|duo|poly] - simultaneously-sounding note budget [note_priority:last|low|high] - which held notes sound (Prophet / Moog / Korg) [trigger:multi|single] - envelope re-strike vs legato pitch-steer [steal:oldest|quietest|low|high] - voice-reuse tiebreaker on overflow Design: * A held-note stack (NoteInfo) tracks every physically-down key, independent of voice allocation (the missing "memory"). * updateVoices() selects the target set (<= fMaxSounding held notes, by priority) and reconciles voices: new target notes are assigned (attacking, or legato pitch-steer under single trigger), notes that left the set are released. mono/duo/poly are the same mechanism with a different budget. * voicing is separate from nvoices: mono/duo cap the sounding count but keep the physical voice pool so the legato crossfade still has a spare instance. * getFreeVoice() honors the steal policy. The policy is parsed by a new MidiMeta::analyse() overload. mydsp_poly reads it from the DSP's own metadata in its constructor (an explicit PolyPolicy argument overrides), so every host architecture honors the options with no per-architecture changes. Validated by a deterministic host harness (31 assertions: mono last/low/high fall-back, duophony, single-vs-multi trigger, poly note-stack overflow, and a default-poly regression) plus an end-to-end metadata-parsing test.
|
Thanks for the proposal, some questions:
|
|
Hi Stephane, Preparing for a short vacation. Got my laptop with me. Will come back to this somewhere over the weekend. Greetings, Thomas |
|
Hi Stephane,
Duo is classic duophony like the ARP Odyssey: at most 2 notes sound at the same time, chosen from all held keys by the same Disclaimer: mono needs 3 voices (one sounding, one for the legato/steal hand-off, one for the ringing release tail) and duo needs 4 voices (two sounding, plus the same hand-off and tail spares) — voicing caps how many notes sound, not how many voices the engine keeps alive.
Different problem (stale voice-internal state vs. note-to-voice assignment), so no — but the same bug exists in the C++ engine too, and this PR's single keyOn choke point is exactly where your snap-on-keyOn fix would go, in both codebases. Happy to do that as a follow-up.
You're correct i did a bunch of tests and the event path in this PR is not good enough. I benchmarked it against stock — the audio path is untouched, but keyOn cost up to ~3 µs worst case where stock takes 40 ns, it allocated memory on every note event, and it silently changed default poly behaviour on voice overflow. I don't want to ship that. So I reworked it, two changes: The engine is opt-in via the metadata. Declare none of the options and the original code runs — the new engine isn't just cheap then, it doesn't run at all. I verified this byte-for-byte against an unpatched Faust on every commit, and it measures the same 40 ns everywhere. How should we go forward? I can rework this PR in place or close it and open a fresh one with the final design. Your call — I'd rather tailor it to what fits Faust than defend my first version.
very happy to help with this.
same applies here. |
So
Better open a fresh one, thanks.
OK thanks.
OK thanks. |
Summary
Faust's
-nvoicesengine currently has one fixed voice-allocation behaviour. It works wellfor true polyphony but is wrong for monophonic and duophonic instruments: with a single
sounding voice, holding note A, pressing then releasing B leaves silence instead of
falling back to the still-held A — the long-standing complaint in #252. Classic mono synths
instead return to the held note (last/low/high-note priority), and let you choose the
retrigger behaviour.
This PR adds a small, opt-in note-allocation policy to
mydsp_polyso the polyphonic enginebehaves correctly for mono, duo, and poly as one unified mechanism. Everything is
selected per-DSP via
declare optionsmetadata and all defaults reproduce the currentbehaviour, so existing patches are unaffected.
Fixes #252.
New
declare optionsmetadataExample — an authentic last-note-priority mono synth with legato:
Design
NoteInfo) tracks every physically-down key, independent of voiceallocation — the "memory" that was missing.
updateVoices()picks the target set (≤fMaxSoundingheld notes, by priority) andreconciles voices: notes newly in the set are assigned (attacking, or legato pitch-steer
under single trigger); notes that left it are released. mono/duo/poly are the same code
with a budget of 1/2/N.
voicingis kept separate fromnvoices: mono/duo cap the sounding count but keep thephysical voice pool, so the existing half-buffer legato crossfade still has a spare
instance to fade between.
mydsp_polyreads its policy from the DSP's own metadata in itsconstructor (an explicit
PolyPolicy*argument still overrides). This is why the changetouches only the engine — every architecture (jack, alsa, pipewire, coreaudio, portaudio,
bela, juce, vst, max, unity, the
dsp_poly_factoryfor llvm/interp/wasm, …) picks up theoptions with no per-architecture changes.
Scope
Two files, both in
architecture/faust/:dsp/poly-dsp.h— held-note stack,updateVoices(), steal-mode selection, self-configuringconstructor.
gui/MidiUI.h— the policy enums,PolyPolicy, and a metadata-parsingMidiMeta::analyseoverload.
No other architecture files are modified.
Backward compatibility
With default options the engine is behaviourally identical to before. Verified at the sample
level: the
tests/architecture-testsorgan regression (2- and 8-voice, viafaust2dummy)produces bit-identical output to
master-dev— including the 2-voice case that exercisesvoice-stealing.
Testing
duophony, single-vs-multi trigger, poly note-stack overflow, and a default-poly regression.
MidiMeta::analyse.faust2jackpoly+MIDI build (compiles and links against the real architecture,with
-DPOLY— an early version of this branch had anenumvalue namedPOLYthatcollided with that macro; the enum values are now PascalCase to avoid any macro clash).
architecture-testsorgan 2/8-voice output bit-identical tomaster-dev(above).git clang-formatagainst the repo.clang-format(no changes reported).Documentation
I've kept this PR to the code. If the direction looks good, I'm happy to follow up with a
docs PR to
faustdocdescribing the new[voicing]/[note_priority]/[trigger]/[steal]options — happy to align on wording/placement first.if it wasn't obvious: co-authored by Claude, but fully validated by me manually and automated deterministic tests.