Wohlstand / libOPNMIDI

A Software MIDI Synthesizer library with OPN2 (YM2612) emulator
GNU Lesser General Public License v3.0
96 stars 6 forks source link

prevent duplicate locations in users sets #85

Closed jpcima closed 5 years ago

jpcima commented 5 years ago

This prevents duplicates of (MidiChannel,Note) from entering users lists. It is possible to have identical notes as distinct users, when sustain is used. Then, killOrEvacuate may move the note to a chip channel where it becomes duplicate.

The code which searches users by location gets confused, because it does not expect a duplicate. It leads to the crash in the arpeggio function.

Wohlstand commented 5 years ago

Is it's same on ADLMIDI too? Looks like it's ancient issue... but, thanks for a fix! :fox_face: :+1:

jpcima commented 5 years ago

ADLMIDI has a map, and this does not permit duplicates. In this case, insert will fail and possibly the note will stick. I'm not sure completely.

This has a relation with bisqwit's "This is a design flaw" note regarding the sustain. (not having activenotes information, because activenotes require to be unique)

I think such things call for some redesign at a future time, and simplifying the whole state management.

This problem took some hours to find. How the assert was triggered: I ran papiezak's Ring.mid, while having changed the goodness in relation to held state:

        s -= (jd.sustained == OpnChannel::LocationData::Sustain_None) ?
            kon_ms : (kon_ms / 2);

to s -= kon_ms;.