TapGhoul
This is from some reading of the SDK DLLs, it is not validated, but you may wanna double check:
Reading through
MidiRawDataProcessor()
, there are a couple bugs in parsing out blocks here in the Midi on/off block processing:
First, the noteTime dictionary is being accessed without a check if an item exists within the
MidiOff
processing - so if a
MidiOff
event appears when no
MidiOn
event has yet appeared, the processor will crash. There is a check, however it occurs after this access, which includes a member access of the start time and start velocity. This check does throw an exception, but it's a much more useful one -
note off event - could not find matching on event
.
This also occurs in the case of an
[On, Off, Off]
sequence.
Secondly, in the case of an equal but overlapping amount, say
[On, On, Off, Off]
, the result will be that the first On is ignored and will instead create 2 blocks which start at the second On but at the given Off currently being processed. The exact way you want to handle this is undefined I believe, but imo you should not be disposing of the first On event here regardless. See the last part for a potential fix.
Lastly, you've got an unused
this.allBlocks
member variable - why I bring this up is this actually has some (but not all) of the logic to do the above correctly - though it seems like you tried to update Velocity and StartTimeMs to the old event, which would fix #2 - except you've already overwritten that data, so the operation is redundant aside from the Count - 1. See the last part for a potential fix.
In general, the solution I see here is to keep a list of
NoteOn
events to feed to
NoteOff
events in your dictionary's value (or however you decide to structure it) to use as a queue (FIFO vs LIFO doesn't matter, this is unspecified in the MIDI standard afaik) instead of just keeping the latest value, and move the proper checks for this above the construction of
midiBlock
in processing your
NoteOff
event to avoid an NPE/null ref member access.