Note chaining#947
Merged
Merged
Conversation
Contributor
🎛️ AMY HW CI (AMYboard bench)Flashed this PR's AMY (LoadTestChord: 6-voice Juno ✅ PASS — the bench ran the test to completion.
Full chord settled render μs: 3502 (was 3495, Δ +0.2%) (peak 3509, 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 |
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.
When getting rid of voices in tests, I had a problem with portamento (which works best if you can change the note of a single voice without retriggering it - see #946). So I was tracking exactly when an osc receives a note, and I saw that sending a note to synth voice caused certain oscs to get the note written to them many, many times. Here's what happened:
amy.send(synth=1, note=60, vel=1)lands up in the "osc-directed with no explicit osc" part ofpatches_event_has_voices(aroundpatches.c:1045).event_to_deltas_queue, each of these generates aMIDI_NOTEdelta to the corresponding osc.play_delta, which is, forMIDI_NOTEdeltas, specially configured to walk downchained_oscchains setting themidi_notefield of all of them. (MOD_SOURCEandALGO_SOURCEoscs are excluded, needed for the "osc broadcast" feature to work).MIDI_NOTEdeltas for all the subsequent oscs in the chain arrive, and their tails of the chain are similarly walked.My solution was to introduce a
SYNTH_IS_CHAINEDstate, similar to theSYNTH_IS_MOD_SOURCEandSYNTH_IS_ALGO_SOURCE. Now, efforts to explicitly set themidi_noteof aCHAINEDosc are ignored, since that will have been handled by the head of the chain.This revealed some confusion about the purpose of
synth[osc]->status- it was holding both the audible status of active oscs (SYNTH_AUDIBLE/INAUDIBLE) as well isSYNTH_IS_*. These are really different things.So I retained
synth[osc]->statusforAUDIBLE/INAUDIBLEand added a new field,synth[osc]->roleto holdSYNTH_IS_*fields. I definedSYNTH_IS_NORMALfor oscs that don't have a special role.(n.b.: the confusion about "synth" and "osc" is probably something we should clean up. There have always been oscs, and their parameter structure has always been called
synth[osc]. When we introduced voice management structures, it was originally calledinstrumentto avoid the confusion, but for the Python interface, it had to besynth, and now this is the general term throughout AMYland. We should rename the venerablestruct synthinfo **synthto bestruct oscinfo **oscsor something.)(note 2: In the process, I was studying the use of the similar-sounding
statusfield inamy_event, which it turns out is unrelated tosynth[osc]->status. In fact, as far as I could tell the various things it had been used for - marking events as sequencer-related, or transfer-related - had been made redundant. So I removed it entirely, and everything still appears to work.)(note 3: You might be tempted to check the role of oscs in the
patches_event_has_voicesosc-broadcast loop, rather than going to all the bother of issuing the deltas, only to have them ignored. The problem with this is that when the event is being processed, we don't actually know what the osc status will be when the delta is finally fired - the event can be defined for some time in the future, and we don't know what osc param changes will occur in the interim. So the only strictly correct thing is to issue the deltas, then see what state the osc is in when the deltas land. This is a bit pedantic, though. However, it's revealed by thetest.pytests, where all of the events are issued before any deltas are processed, so this ontological delay is made very very obvious.)