celtera / libremidi

A modern C++ MIDI 1 / MIDI 2 real-time & file I/O library. Supports Windows, macOS, Linux and WebMIDI.
Other
463 stars 52 forks source link

Move the sysex re-concatenation behaviour outside of the backends #77

Closed jcelerier closed 9 months ago

jcelerier commented 1 year ago

It makes much more sense as an external state machine

marinedalek commented 1 year ago

I'm having an issue that I think may be related to this. I'm using the WinMM backend and have found that any received sysex message is appended to the previous message, no matter what the previous message was.

For example, for the following sequence of input messages:

[Note On], 
[Note Off], 
[sysex 1], 
[sysex 2]

My callback will receive:

[Note On],
[Note Off],
[Note Off] + [sysex 1],
[Note Off] + [sysex 1] + [sysex 2]

I believe that this is because, currently, the library midiInputCallback method does not check whether the previous message was an incomplete sysex message before appending the current message to it.

An additional side-effect of this is that, when ignore_sysex is true, any received sysex messages will cause the last non-sysex message to be resent to the callback, except with the timestamp of the new ignored sysex message.

You may already have noticed all of this, hence this issue being opened!

jcelerier commented 9 months ago

sorry @marinedalek , just saw again this comment when checking the issues, that's indeed a bug! for now I'd say that clearing the sysex buffer when we get a 0xF0 or 0xF7 would be a good countermeasure, what do you think?

jcelerier commented 9 months ago

fixed!

jcelerier commented 9 months ago

and done - now all the backends share the exact same logic so it'll be much easier to work and fix bugs