lathoub / Arduino-AppleMIDI-Library

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

When receive too many MIDI data, it will crash #71

Closed ZZZ8 closed 4 years ago

ZZZ8 commented 5 years ago

I used rtpMIDI V1.1.11 , Esp32 DevKit v1 ,

When ESP32 receive too much MIDI data,(Receive about 10 midiNotes or midiController at the same time) 1:send 10 Notes https://i.loli.net/2019/09/11/mfhn8NFlvRYpLKP.png midiMessage.png 2.receive 3 note on message https://i.loli.net/2019/09/11/XKuYFTlEVRe9kro.png receive .png

  1. Connection failed https://i.loli.net/2019/09/11/XKuYFTlEVRe9kro.png miss.png

the esp32 will crash. What is the reason?

thx

arduino code: https://pastebin.com/z5mH5mYb

lathoub commented 5 years ago

Not enough information, you will have to expand. Provide code, method to reproduce, etc

ZZZ8 commented 5 years ago

Not enough information, you will have to expand. Provide code, method to reproduce, etc

It has been added.

ZZZ8 commented 5 years ago

single note 40notes/s Perfect single note 53notes/s Perfect single note 60notes/s Dead

lathoub commented 4 years ago

I have partially rewritten the library, in the 2.0.0 branch. Can you test it? (I have not tested it on a ESP32 - the endianness might not be correct, not sure)

ZZZ8 commented 4 years ago

arduino 1.8.10 some error

error: macro "ntohl" passed 4 arguments, but takes just 1 invitation.initiatorToken = ntohl(a[0], a[1], a[2], a[3]);

error message

lathoub commented 4 years ago

I added more ESP32 code (especially on the endianness) and I can get the ESP32 to participate in a session :-). Hopefully you get better results now (not all MIDI messages are parsed in the AppleMIDI parser, still some way to go -looking for volunteers!)

designerfuzzi commented 4 years ago

a NoteOff is actually a NoteOn Message with Velocity lower then 64 or more often (usually) with velocity (zero) 0. Meaning the Callbacks for both can be handled at once and your amazing lib could follow the standard and handle NoteOff as NoteOn with 0 velocity. This way you can also handle stuck notes that have been never closed and implementation of active sensing makes sense then.

Why? Because as it is now the lib or programmer using it have to compare NoteOn and NoteOff Messages to know which notes are open and largely depend on AllNoteOff messages to close all open while AllNoteOff messages are not send by all AppleMidiNetwork emitters.

Hint: solution maybe instead - receiving Opening Note Messages on already open Notes would re-trigger the corresponding callback while NoteOff (aka velocity 0 NoteOn) would close the same note-pitch instead of triggering a separate callback which does know nothing about the open notes.

lathoub commented 4 years ago

Branch 2.0.0 (heavily modified the parser for combined messages, incl large SysEx)) has been stress tested (Active Sensing at 270ms, Timing Clock at 250 BPM, large repeated Sysex and continuous NoteOn/Off). Can you please test branch 2.0.0 @ZZZ8 ?

ZZZ8 commented 4 years ago

OK . tomorrow ,i test it.

lathoub commented 4 years ago

Thanks - make sure you test the latest 2.0.0 branch

ZZZ8 commented 4 years ago

applemidiTest.xlsx

@lathoub Test results in the Excel document

Thank you for your support

ZZZ8 commented 4 years ago

More stable than last time , No crashes when I send about 20 Notes.

lathoub commented 4 years ago

Thanks @ZZZ8 Interesting results!

Do I understand it correctly that notes are dropped? (miss). Missing notes without seeing errors in the log is strange - a note in rtpMIDI is surrounded by protocol bytes and counters. So missing a note without causing other protocol error is strange.

Are you able to include Wireshark in your tests?

lathoub commented 4 years ago

@ZZZ8 Are you sure you are using the latest branch v.2.0.0?

I installed Reaper v6.03 earlier and that revealed some Journal section errors in the code - fixed and uploaded. (Journal section was much larger than what I have seen in the past)

ZZZ8 commented 4 years ago

666, You modify very fast .Thanks .

I haven’t had time to test the latest version,today. I will test it,these two days

发送自 Windows 10 版邮件https://go.microsoft.com/fwlink/?LinkId=550986应用

发件人: lathoubmailto:notifications@github.com 发送时间: 2020年2月15日 19:32 收件人: lathoub/Arduino-AppleMIDI-Librarymailto:Arduino-AppleMIDI-Library@noreply.github.com 抄送: ZZZ8mailto:ZLC@MSN.CN; Mentionmailto:mention@noreply.github.com 主题: Re: [lathoub/Arduino-AppleMIDI-Library] When receive too many MIDI data, it will crash (#71)

@ZZZ8https://github.com/ZZZ8 Are you sure you are using the latest branch v.2.0.0?

I installed Reaper v6.03 earlier and that revealed some Journal section errors in the code - fixed and uploaded. (Journal section was much larger than what I have seen in the past)

― You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/lathoub/Arduino-AppleMIDI-Library/issues/71?email_source=notifications&email_token=AJYDUI2RUQQGOROBTB7U7KLRC7HDPA5CNFSM4IUTXXVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEL3IJBA#issuecomment-586581124, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AJYDUI2VVA4W4BMZ7Q26ULLRC7HDPANCNFSM4IUTXXVA.

lathoub commented 4 years ago

Let me know how the tests go - I tried with REAPER as well (v6.03/64) on MacOS with no drops (branch 2.0.2 rc1)

ZZZ8 commented 4 years ago

[cid:image001.png@01D5ECC9.863E2920] 1\ Input notes in the DAW,8notes,9notes,10notes. and play it . 2\ look at the arduino Serialprot , Number of note.On

发送自 Windows 10 版邮件https://go.microsoft.com/fwlink/?LinkId=550986应用


发件人: lathoub notifications@github.com 发送时间: Monday, February 24, 2020 12:15:28 AM 收件人: lathoub/Arduino-AppleMIDI-Library Arduino-AppleMIDI-Library@noreply.github.com 抄送: ZZZ8 ZLC@MSN.CN; Mention mention@noreply.github.com 主题: Re: [lathoub/Arduino-AppleMIDI-Library] When receive too many MIDI data, it will crash (#71)

Let me know how the tests go - I tried with REAPER as well (v6.03/64) on MacOS with no drops (branch 2.0.2 rc1https://github.com/lathoub/Arduino-AppleMIDI-Library/tree/v2.0.2-rc.1)

― You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/lathoub/Arduino-AppleMIDI-Library/issues/71?email_source=notifications&email_token=AJYDUI6TJOEYSMENUXYC35LREKOKBA5CNFSM4IUTXXVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMV73PI#issuecomment-590085565, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AJYDUI3UVXSLOW5WBHWZQ3LREKOKBANCNFSM4IUTXXVA.

lathoub commented 4 years ago

added Session Initiator in branch 2.0.2 (and example)

lathoub commented 4 years ago

more Initiator testing (see Initiator example), can you test the latest branch feat/2.0.0 (I have not made a release - fetch the latest from feat/2.0.0) @ZZZ8

lathoub commented 4 years ago

Fixed in the latest release 2.0.3 (now in the master branch)