search

lathoub / Arduino-AppleMIDI-Library

Send and receive MIDI messages over Ethernet (rtpMIDI or AppleMIDI)
Other
306 stars 66 forks source link

Running status issue? Midi messages mangled and subsequent communication broken/out of sync. #125

Closed hallvardkristiansen closed 3 years ago

hallvardkristiansen commented 3 years ago

Context

Using the English language, please answer a few questions to help us understand your problem better and guide you to a solution:

Describe your project and what you expect to happen:

The Teensies are controlling lights and buttons on a sequencer, controlled by a computer running PD, Ableton and MidiPipe. The Teensies connect automatically using sendInvite in a staggered sequence which works fine. The Teensies send and receive midi without issue for the most part. Midi notes are mapped to particular LEDs and switches.

Describe your problem (what does not work):

Perhaps "bug" is not correct here, but I don't have any better way of describing the issue. Midi messages sent by MidiPipe to reset LEDs and other functions get dropped by the Teensy. When sending eight messages with noteOff, only two arrive correctly and occasionally a third arrives as noteOn. Sometimes when spamming a button, the messages can make it through, but this only happens in one out of 20 attempts.

Steps to reproduce

Send batches of the same type of message to the Teensy using MidiPipe. We have checked all the hardware, tested replacing the Teensy with a macbook which received all the messages fine on the same ethernet cable. We also checked serial output of raw midi data which looked correct. I have a suspicion that we are dealing with running status messages not being parsed by the Teensy, and thus making the subsequent messages nonsense.

Right now I do not have direct access to the hardware, but I could request a serial dump of the raw midi messages as soon as possible.

Would appreciate any suggestions for solutions!

lathoub commented 3 years ago

Thanks @hallvardkristiansen , sounds like a buffer overrun/flow that occurred before: #100 Quite a long thread, but from what you describe, you could be hitting the same issue (DAW's typically generate a lot of background reset messages, combined with 'spamming a button' (hitting a button repeatedly, like in the Hyper Olympics arcade :-))

Here is a summary of the buffer overflow issue (and how to solve): https://github.com/lathoub/Arduino-AppleMIDI-Library/wiki/Enlarge-Ethernet-buffer-size-to-avoid-dropping-UDP-packages

If this is not the issue, then a serial dump is needed.

hallvardkristiansen commented 3 years ago

Hi! Thanks for the fast reply! Yes that is what I thought too. But I read the documentation so I was aware of this and tried the ethernet3 library instead. This didn’t change anything regarding incoming messages(exact same behavior), but it caused the teensy to lock up when it sends a midi message. I will ask for a serial dump of the issue and post it here asap.

I suppose in order to enable running status support I would need to initialize the midi class separately first and then do a custom initialization of the applemidi class? It should pass the modified midi reference to applemidi then, correct?

lathoub commented 3 years ago

Running status is enabled (always) as part of parsing incoming AppleMIDI messages, but I noticed that Running Status is not on in the Settings of the underlying MIDI library, see https://github.com/FortySevenEffects/arduino_midi_library/blob/d501f4bb3dfff548a7cef4d85371bec0b438bdd7/src/midi_Settings.h#L51-L56

So you could override the Setting when creating the MIDI object, see https://github.com/lathoub/Arduino-AppleMIDI-Library/blob/1f095b72014043bb1bf6f46642ecb5c46ce9aab6/src/AppleMIDI.h#L339-L341

hallvardkristiansen commented 3 years ago

Thank you. I will try writing a custom initialization that enables running status in the midi library. Meanwhile my colleague is extracting a serial dump of the raw midi when the issue is occurring. Will post it here as soon as it arrives.

lathoub commented 3 years ago

if you can, please provide the dump as:

byte buffer[] = {
                 0x80, 0x61, 0xbf, 0xa2, 0x12, 0xb, 0x5a, 0xf7, 0xaa, 0x34, 0x96, 0x4a,
                 0xc0, 0x2b,
                 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x0, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x0,
                 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x00, 0xf8, 0x0, 0xf8, 0x00, 0xf8, 0xc0, 0xbf, 0x89, 0x90, 0x05, 0xd0, 0x7a, 0xd5 };

(I'll use using the xcodeproj project in the /test dir to do the debugging)

hallvardkristiansen commented 3 years ago

Here is one of the latest outputs that has been cut of in the middle.

byte buffer[] = {0x92, 0x70, 0x00, 0x71, 0x00, 0x72, 0x00, 0x73, 0x00, 0x74, 0x00, 0x9A, 0x60, 0x78, 0x61, 0x00, 0x61, 0x78, 0x60, 0x00, 0xBA, 0x33, 0x32, 0x94, 0x71, 0x7F, 0x72, 0x00, 0x73, 0x00, 0x75, 0x7F, 0x76, 0x00, 0x94, 0x70, 0x00, 0x74, 0x00, 0x78, 0x00, 0x90, 0x70, 0x00, 0x92, 0x70, 0x00, 0x71, 0x00, 0x72, 0x00, 0x73, 0x00, 0x74, 0x00, 0xBA, 0x33, 0x32, 0x94, 0x71, 0x7F, 0x72, 0x00, 0x73, 0x00, 0x75, 0x7F, 0x76, 0x00, 0x77, 0x00, 0x79, 0x7F, 0x7A, 0x00, 0x7B, 0x00, 0x70, 0x00, 0x74, 0x00, 0x78, 0x00, 0x90, 0x71, 0x00, 0x92, 0x70, 0x00, 0x71, 0x80, 0x92, 0x74, 0x00, 0x90, 0x70, 0x78, 0x71, 0x78, 0x72, 0x00, 0x73, 0x00, 0x74, 0x00, 0x75, 0x00, 0x76, 0x00, 0x77, 0x00, 0x70, 0x00, 0x71, 0x00, 0x72, 0x00, 0x73, 0x00, 0x74, 0x00, 0x75, 0x00, 0x76, 0x00, 0x94, 0x71, 0x00, 0x72, 0x7F, 0x73, 0x00, 0x94, 0x70, 0x78, 0x75, 0x00, 0x76, 0x7F, 0x77, 0x00, 0x79, 0x00, 0x7A, 0x7F, 0x7B, 0x00, 0x74, 0x78, 0x78, 0x78, 0x70, 0x00, 0x74, 0x00, 0x78, 0x00, 0xBA, 0x32, 0x28, 0x90, 0x72, 0x00, 0x90, 0x73, 0x00, 0x90, 0x74, 0x00, 0x90, 0x75, 0x00, 0x90, 0x76, 0x00, 0x90, 0x77};

Will that work for you? It seems it contained about 500 bytes of actual data values, the other characters would maybe push that over the default buffer size? The messages are definitely running status style, but they do at least for the most part seem to be handled well by the teensy.

lathoub commented 3 years ago

Thanks - this looks like a raw MIDI stream, not AppleMIDI. Can you provide the Wireshark log?

hallvardkristiansen commented 3 years ago

Right, we misunderstood. I’ll try to get the wireshark log. Thanks

hallvardkristiansen commented 3 years ago

Problem-Occurs-afte-7-sec.pcapng.zip

Here is the log I received. It shows that the issues begin when the RTP-MIDI messages start arriving from the computer(10.0.0.6) from message nr. 1015

The RTP type messages are coming from PureData, while the RTP-MIDI messages come from Ableton via MidiPipe. It's not obvious to me why this would cause a problem, but maybe you see something here?

Thank you!

hallvardkristiansen commented 3 years ago

Looks like a lot of the RTP-MIDI messages are too large and there are too many of them going out. This would surely cause a buffer overflow?

hallvardkristiansen commented 3 years ago

It’s pretty obvious that there are major issues with the messages coming from the computer so I don’t think there was anything wrong on the Teensys in the first place. Sorry for taking up your time and thank you for trying to help :)

lathoub commented 3 years ago

Thanks for the wireshark dump, very helpful @hallvardkristiansen.

There are indeed a lot of messages being send (see screenshot below, all within 1/10s - from 1 source (10.0.0.6) to 7 targets), but they all look valid (and sending NoteOff as NoteOn with velocity = 0, to preserve running status) and they are all valid. I will check try to use the hex dump and see if the (small W5500) buffer overruns.

image

The parser can report exceptions, have you enabled them? (see the exceptions example). I'm might see *** ReceivedPacketsDropped messages.

Did you set the largest buffer (2048 bytes) for the W5500 in Ethernet.h? (see the wiki) #define ETHERNET_LARGE_BUFFERS

hallvardkristiansen commented 3 years ago

Hi, thank you for looking into the logs. I spent some time deeper into the messages last night and it’s difficult to say what might be going wrong.

I previously tried switching to the ethernet3 library and tested various socket numbers, primarily 4, but also 2 and 8, and enabled the exception output and tried to use the raw midi output sketch from examples instead of our code, but the results were the same and no exceptions were triggered as far as we could see. With the ethernet3 lib the teensys froze when they tried to send midi for some reason. But since the issue with receiving from midipipe didn’t improve we didn’t spend anymore time debugging that.

I’m away now and the guys got things working again by simply not sending any messages from midipipe, since the midi from PureData works fine. It’s all quite confusing!

hallvardkristiansen commented 3 years ago

Enabling Running Status in the base midi class just made things worse they said, so that was also abandoned, and it was already supported by applemidi.

hallvardkristiansen commented 3 years ago

I didn’t make any changes to the core ethernet lib as I didn’t know how to do that without editing the class in the teensyduino installer package and reinstalling the software. Is that really the only way to change that constant?

hallvardkristiansen commented 3 years ago

My best guess is that something is going wrong with the connections between the teensys and the computer, as they will start falling off and trying to reconnect after a while of not responding, so they must still be working like normal. The reconnect attempts fail though and they get stuck in a disconnect/reconnect loop. The invitations go out from the teensys every roughly 30 seconds with a minor additional delay of 1 sec times their midi channel to keep them from trying to connect at the exact same time. Sending the invites from all 7 of them at the same moment caused some problems before, so we spaced them out. This works fine now until messages start coming in from midipipe.

hallvardkristiansen commented 3 years ago

They only send out invitations if the disconnected callback has been triggered, and they stop sending them if the connected callback is triggered. That code was not there in the raw midi output and exceptions sketch code, where we connected from the computer by ip address btw.

lathoub commented 3 years ago

Enabling Running Status in the base midi class just made things worse they said, so that was also abandoned, and it was already supported by applemidi.

Correct - do not set running status to true (sorry for suggesting, it's been a while since i used it)

lathoub commented 3 years ago

I just tried parsing (from your dump, message #1035)

        byte NoteOffAsterix[] = {
            0x80, 0x61, 0x19, 0x05, 0x00, 0x16,
            0xcf, 0xe0, 0xf2, 0x2d, 0x56, 0xc3, 0x4c, 0x94, 0x63, 0x00, 0x81, 0x6a, 0x90, 0x63, 0x00, 0x02,
            0x92, 0x63, 0x00, 0x23, 0x18, 0xf5, 0x00, 0x19, 0x08, 0x89, 0xcd, 0x63, 0x7f, 0xf0, 0x78, 0xf1,
            0x78, 0xf2, 0x78, 0xf3, 0x78, 0xf4, 0x78, 0xf5, 0x78, 0xf6, 0x78, 0xf7, 0x78, 0xe0, 0x1f, 0x10,
            0x11, 0x08, 0x85, 0xcd, 0x63, 0x7f, 0xf0, 0x78, 0xf2, 0x78, 0xf3, 0x78, 0xf4, 0x78, 0xe0, 0x1f,
            0x20, 0x15, 0x08, 0x87, 0xcd, 0x63, 0x7f, 0xf0, 0x78, 0xf1, 0x7f, 0xf4, 0x78, 0xf5, 0x7f, 0xf8,
            0x78, 0xf9, 0x7f, 0xe0, 0x1f, 0xd0, 0x07, 0x08, 0x81, 0xf1, 0xe0, 0x78 };

and that produces 3 note offs, nicely capture (using MIDI.begin(MIDI_CHANNEL_OMNI);)

Also message #1045 that has a Delta Time passes well (never seen the Delta Time before)

        byte NoteOffAsterix1[] = {
            0x80, 0x61, 0x19, 0x07, 0x00, 0x16,
            0xd5, 0x87, 0xf2, 0x2d, 0x56, 0xc3, 0xc0, 0x14, 0x92, 0x71, 0x00, 0x8f, 0xff, 0xff, 0xfe, 0x00,
            0x94, 0x64, 0x00, 0x81, 0x5a, 0x90, 0x64, 0x00, 0x02, 0x92, 0x64, 0x00, 0x23, 0x18, 0xf7, 0x00,
            0x19, 0x08, 0x89, 0xcd, 0x64, 0x7f, 0xf0, 0x78, 0xf1, 0x78, 0xf2, 0x78, 0xf3, 0x78, 0xf4, 0x78,
            0xf5, 0x78, 0xf6, 0x78, 0xf7, 0x78, 0xf0, 0x0f, 0x10, 0x13, 0x08, 0x86, 0xcd, 0x64, 0x7f, 0xf0,
            0x78, 0x71, 0x7f, 0xf2, 0x78, 0xf3, 0x78, 0xf4, 0x78, 0xf0, 0x0f, 0x20, 0x15, 0x08, 0x87, 0xcd,
            0x64, 0x7f, 0xf0, 0x78, 0xf1, 0x7f, 0xf4, 0x78, 0xf5, 0x7f, 0xf8, 0x78, 0xf9, 0x7f, 0xf0, 0x0f,
            0xd0, 0x07, 0x08, 0x81, 0xf1, 0xe0, 0x78
        };

image

So the parser behaves well. Can you identify a message that is not parser correctly? (or where it starts to misbehave)

lathoub commented 3 years ago

Message #2525 is kinda weird.

Command length is 11 bytes, but the message is much longer and contains Quarter Frames, Song Select, an undefined 0xf5 and EOX

image

lathoub commented 3 years ago

as is message #1153 in your dump (the first message that is misformed)

hallvardkristiansen commented 3 years ago

Hi! Thank you for spotting that! I see that the malformed messages are repeating about every 120 batches. This is probably a step in the sequence that is corrupted. I've told the guys about it and they'll see if they can't fix it.

hallvardkristiansen commented 3 years ago

The parser does seem to work absolutely fine. It's pretty clearly a problem with the midi messages going out of the computer.

lathoub commented 3 years ago

The parser does not survive the misformed MIDI data.

https://github.com/lathoub/Arduino-AppleMIDI-Library/blob/725b82de1f4bb38fc478af3c9440e7eaad347d16/src/rtpMIDI_Parser_MidiCommandSection.hpp#L37

consumed is larger than midiCommandLength, resulting in a large midiCommandLength (datatype is not signed, so -1 becomes 65535) so a check is needed:

if (consumed > midiCommandLength) {
    buffer.clear();
    return parserReturn::UnexpectedMidiData;
}
lathoub commented 3 years ago

I put a possible fix in this branch - can you test?

hallvardkristiansen commented 3 years ago

Thank you, we’ll test it. The machine is away for some other repairs at the moment, but will be back soon!

Hallvard Kristiansen

On 15. Jul 2021, at 21:36, lathoub @.***> wrote:

 I put a possible fix in this branch - can you test?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

lathoub commented 3 years ago

Hi @hallvardkristiansen have you been able to test the branch? It also contains a bug fix for long session names. Before I merge it, i wanted to get your feedback first

hallvardkristiansen commented 3 years ago

Hi @hallvardkristiansen have you been able to test the branch? It also contains a bug fix for long session names. Before I merge it, i wanted to get your feedback first

Hi! Sorry, I was supposed to write you about it but I forgot to do it… Yes, the branch fixes the crashing with bad midi notes. It can now handle corrupted data input fine it seems.

Also, we found the issue causing the corruption. It was multiple midi sources on the computer outputting to the same rtpmidi session. Occasionally this would cause a collision and a note would get mangled. By routing it all through midipipe the issue is gone.

Thank you so much for your help!