-
Notifications
You must be signed in to change notification settings - Fork 371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
jack_midi_event_get behaviour does not match documentation when receiving SysEx messages #413
Comments
Thanks for pointing this out. It would be good to have an example of long message that triggers the bug as an attachment in binary or eventually base64 encoded form feed to jack_midi_dump so we can see exactly the same behavior and talk about the expected. |
|
I can confirm that throwing arbitrary bytes to jack_midi_dump causes a segfault. It could possibly be made more robust to nicely terminate or handle invalid situations. This needs to be addressed in a separate issue. Some notes: @SpotlightKid how exactly do you receive "several events of up to 256 bytes in size" ? |
@SpotlightKid the jack_midi_dump example client does now support larger messages, please see recent commit. For this process() function:
jack_midi_dump shows this:
It looks exactly as expected so far. Two midi events, each 512 bytes long, starting with f0, ending with f7. Is that what you would expect to see from the synth patch? |
That's what I basically did to test.
I'm not sure I understand the question. I just ran You can see from the output of
and got this output:
I tried changing the JACK buffer period/size from 256 to 512 bytes, but that didn't change the outcome. My guess is that the 256 event size cap is imposed by the ALSA midi backend.
Yes. And that also strengthens my belief, that this has to do with the ALSA backend, which is out of the picture in your test. |
BTW, I stumbled on this behaviour via RtMidi, which incorrectly assumes that |
As far as I understand, the jack_midi_event_get() will return whatever was written into the buffer by another process via jack_midi_event_write() beforehand. Your synth sends something (is it one message?), this gets into jack somehow and then is chunked up. Do you use a2jmidid to get it into jack? |
No, I use JACK2 with the |
Yes. Well, actually, as written in the issue description, it sends three messages per patch, and the first is 510 bytes long. I confirmed that with a MIDI monitor (midi-ox) on Windows. |
in alsa_seqmidi.c eventually: |
Can you create a dummy output as you'd expect it for the messages in your first post? You'd expect it to collapse to 3 messages? |
If you can record the midi bytestream that you think is correct, please attach it here. I'd like to send it to JACK in various ways. |
I don't really care what |
jack_midi_dump should output pretty much what jack_midi_event_get is getting so if it works it's a good indicator of what's going on. You have to point out more clearly which output you'd expect. If you won't provide that or the data that would trigger it, it's hard to easily replicate and guess about whether it's a generic or another problem. |
From http://jackaudio.org/api/group__MIDIAPI.html#ga838c794bd1451bfd47edde1c7cd1ff4f
If that rule is strictly followed, then
It doesn't tell anything about splitting large messages. |
I would expect that If
|
Note on how to reproduce:
Found this comment in midi_unpack.h: https://github.com/jackaudio/jack2/blob/master/linux/alsa/alsa_seqmidi.c#L788 |
This is the output when leaving jackd out of the test.
It seems to do exactly the same thing as jack_midi_dump regarding the chunk sizes of the first message. |
@SpotlightKid please try this preliminary patch:
|
Yes, I can confirm that |
The bonus question is what would we consider as a too large message. The current limit of 256 bytes is a bit low. If somebody sends a 2 MB message, it could be necessary to either drop it or do the same, chunk it up at the ALSA level. In that case, the documentation would need to say that it could be necessary to concatenate the messages at client side. The problem is not yet fully solved. |
Dropping messages is always a bad idea, IMHO. At least this should be signalled by the return value of |
Yes, interesting ideas. It needs to be possible to receive large sysex without compromising real time performance. This means to create chunks at some point. It could be a user setting, a filter (OFF), or plain what ALSA has to offer. The last case means to only update documentation on the 256 bytes limit (this might vary between backends) plus a hint that if checking for sysex messages, caller can not expect a "normalized" single event. |
Would be fine by me. Like I mentioned above, I already implemented code in RtMidi to handle this situation and the same logic needs to be applied in its ALSA and CoreMIDI backends anyway. |
Note added for jack_midi_event_get() here: |
The documentation for jack_midi_event_get says:
However, the behaviour I'm observing is, when a large MIDI message (e.g. a SysEx message with a synthesizer patch) is received, several events of up to 256 bytes in size are generated, and the second to last messages do not start with a status byte. and only the last message ends with status 0xF7 (end-of-sysex). So they are not complete MIDI events.
For example, here is the output of the
jack_midi_dump
client when I send the patch edit buffer from a Yamaha DX7II synthesizer, which consists of three SysEx messages of 510, 57, and 163 bytes length respectively.(Note: I had to increase the size of the
buffer
member of themidimsg
struct on line 37 to >= 512 bytes, otherwisejack_midi_dump
will terminate with a segfault on reception of the first message.)Tested with jack2 with dbus from git (549ee2e) on Manjaro Linux (kernel
4.16.18-rt12-MANJARO
) with ALSA backend (alsa-lib1.1.7-2
).The text was updated successfully, but these errors were encountered: