thenfour / digifujam

32 stars 1 forks source link

notes stuck in sequencer due to noteID vs. midiNoteValue #258

Closed thenfour closed 2 years ago

thenfour commented 2 years ago

there is a subtle bug in the sequencer, known about for a while but it's pretty rare to repro. but now with the new mapping modes, it's more likely to occur.

the sequencer schedules notes to be played in batches. each time this happens we have to bridge the gap between batches, and the scheduler needs to know if some notes still exist in the pattern (or should they be note-off'd and deleted). therefore there is a seqNoteID property of scheduled notes, which is sequencer pattern view cell id, which is a "hash" of underlying note IDs.

But this is flawed: it assumes that noteIDs always result in the same MIDI note value (so the generated note offs will correspond).

therefore, anything that can change the output midi note value of a sequencer note can result in a disconnect between note off and note on. For sequencer mode, octave & transposition can cause this.

to repro, it's not very straight-forward. you need to invoke the scenario where a "batch" happens while a note is on, and the new batch has the same noteID but with a different value. And even then i found it hard to repro. Easiest was to create a short sequence with 1 long note, played as fast as possible. Then as its playing, adjust transposition. eventually a note will get stuck without note-off.

one solution to this is to ensure that the pattern cell noteIDs include ALL variables which affect the midi note value.

And any time those variables change, the pattern view needs to be reconstructed.


edit: with fixing #259, conveniently this becomes much less of a problem. de-sync'd notes no longer get stuck, they just last longer than they should. it's actually a better outcome than playing it "too safe" and note-offing all scheduled notes.

what happens is that the note still exists in the pattern, but the sequencer remembers the note value from the corresponding note on event. So it re-schedules it in the future as-is, with the old note value. and it even corrects for overlapping notes.