Better rawMIDI parser and support for Virtual RawMIDI port

[patriciogonzalezvivo]

Context for this PR can be found in:

• this thread: https://llllllll.co/t/opening-midi-port-from-external-c-program-using-rtmidi/36456/34
• this previous PR: #1202

This PR introduce:

• Virtual RawMIDI initialization. This is mostly useful for devices where you have access through ssh and you are willing to send MIDI events to matron from other programs in the same device like aplaymidi or thorough the network with rtpmidi

• Reimplement the MIDI parser. Why? non-USB midi devices don't constrain into the 3 bytes packages for CC and NOTE_ON, NOTE_OFF, etc. For example they don't send the event type if is the same of the previous (unless it change the channel which efectively change the type unsigned char value). Also they will pack as much events in a single call. For example you can get one type byte and two pair of parameters in a package of a total of 5 bytes. All this deviate from the previous implementation where the parsing was every other or every 3bytes. This represent a problem for all non-usb midi devices. With this PR that's solve that and open the doors for better MIDI support. Much of this re-implementation repurposed some code from FluidSynth that can be found here: https://github.com/FluidSynth/fluidsynth/blob/master/src/midi/fluid_midi.c. The reasoning behind that is that FluidSynth is a mature open source project that already have accounted for common and edge cases mitigating the risk of introducing regressions into the code

I test this with:

• a physical device NanoKONTROL2
• sending MIDI events internally from another program (aplaymidi)
• sending MIDI events over the network (local wifi) from an iOS device (iPad)

The most exciting use case I found is the use of RTPMIDI. This allows MIDI over network using this protocol compatible with iOS/OSX devices. There is no need for Bluetooth! Right now, with the code as is, this require some wiring through a ssh session (which obviously is not user friendly):

1. First to install RTPMIDI

wget https://github.com/davidmoreno/rtpmidid/releases/download/v20.07/rtpmidid_20.07_armhf.deb
sudo dpkg -i rtpmidid_20.07_armhf.deb

2. Make sure your iOS/OSX device is sending MIDI events to Network
3. Check that you can "see" the device from Norns. In this example I can see booth my linux laptop (razor) and my iPad (pilot)

[~]$ aconnect -lio
client 0: 'System' [type=kernel]
    0 'Timer           '
    1 'Announce        '
client 14: 'Midi Through' [type=kernel]
    0 'Midi Through Port-0'
client 128: 'rtpmidi norns' [type=user,pid=350]
    0 'Network         '
    1 'razor           '
    2 'pilot           '
client 129: 'Client-129' [type=user,pid=28138]
    0 'Virtual RawMIDI '

4. Link the out put of the RTPMIDI client to the "Virtual RawMIDI" so it can be read by any norns script:

aconnect 128:2 129:0

I think in furthers PRs and through collaboration, we could work on simplifying this process through the nice UI Norns have : )

[schollz]

This change is great for me, namely the ability to target a "Virtual Port". I often use a text-based MIDI sequencer I wrote and have been really interested in running one directly from my norns to feed MIDI into my scripts.

The virtual port introduced here does exactly what I need. I tested
a97e37a using my MIDI sequencer on the new virtual port and it works great. Regular MIDI still works without a hitch as well. Can't attest to sending MIDI events over Wifi yet, but I can give that a shot later.

[artfwo]

There's no need to re-open PRs if you'd like to squash or edit commits. one way to do that is to rebase your branch on the upstream, while squashing commits if necessary.

See the following link for an example of rebasing: https://github.com/servo/servo/wiki/Beginner%27s-guide-to-rebasing-and-squashing

(so we can keep the discussion continuous)

As to your new PR, consider the above comments and I have a follow-up question to #1202 (comment) - how easy it will be for a norns user to configure rtpmidi and/or connect other programs to matron?

[patriciogonzalezvivo]

Ok... I'm failing collapsing all the commits into one after they are pushed. I apologize for that.

[patriciogonzalezvivo]

How do I add matron/src/device/device_midi_parser.c to wap compilation rutine??

[catfact]

i think having an rtmidi endpoint is a great use case. but agree that there needs to be some way for end users (not developers) to acheive this. (and of course - this can be left for future work.)

re: fluidsynth code. we unfortunately can't do this. fluidsynth is GPL2. norns is GPL3 which is more permissive, it's not "backwards compatible." we would have to adopt the older license for norns, or ask the fluidsynth authors for a "GPL2+" license.

i'd prefer to lose the parsing code if possible. we are already doing parsing in Lua. if we need to extend the lua parser or pass more bytes to it, let's just do that.

How do I add matron/src/device/device_midi_parser.c to wap compilation rutine??

add it to the source list here
https://github.com/monome/norns/blob/main/matron/wscript#L4

[patriciogonzalezvivo]

Isn't parsing in lua at this level extremely slow (never used lua before, but guessing it's a script language there could be a serious performance hit)

[patriciogonzalezvivo]

The AUTHORS documentation points to @derselbst as contact

[catfact]

i guess it depends what you mean by "extremely" - it is slower for sure. i doubt it's significant. the main performance hits in lua come from memory management (e.g. string processing.)

parsing midi messages in lua happens here:
https://github.com/monome/norns/blob/main/lua/core/midi.lua#L255

this table of 3 bytes is just copied to the stack through weaver.c. it could probably by slightly optimized with a dispatch table rather than ifelse. but this is definitely a case of "measure before optimize."

but in any case - events generated from your new code end up hitting the lua parsing code anyway, so its a totally moot point. the only way around that would be to have a separate lua glue function for each midi event type.

as you've pointed out, there are cases that we are missing, like double-precision CCs (i think?).
other things alluded to in the fluidsynth parsing code:

• handling incomplete or mangled frames (this of course doesn't really happen with USB devices, maybe it's a problem for BT or RTP)
• handling realtime MIDI bytes that may be interspersed with standard control sequences... is this a thing?? i've never heard of that. is it particular to the fluidsynth environment? maybe someone more expert in contemporary software MIDI standards can enlighten me.

[ed] ok, seems to indicate that there are plenty of opportunities for e.g. clock bytes to show up in other control sequences:
http://www.personal.kent.edu/~sbirch/Music_Production/MP-II/MIDI/midi_system_real.htm

can anyone confirm that this breaks our current system?

if it seems best to handle everything in C and add many, many more cases to the event processing loop (beyond generic MIDI_EVENT) then that is fine with me, but let's not pretend we're speeding things up by adding a second parsing layer.

paging @derselbst the question here is whether it would be OK to include some of fluidsynth's MIDI stream parsing code in a GPL3 project. (fluidsynth being GPL2.) none of us are laywers, but it seems like it is "technically" not allowed without specific permission.

[patriciogonzalezvivo]

Please don't get me wrong, I'm not pretending that a second parse is faster. It's for sure more expensive that the previous implementation. I'm not familiar with LUA at all, so I thought resolving this at a lower level will be faster that doing the same inside an interpreter. My only experience with them is through Python which is surprisingly slow.

The reasoning to bring FluidSynth parse to the table is mostly because it's a mature project that already resolve a lot of common and edge cases.

[catfact]

oh indeed, understood. sorry if that sounded aggressive. the rest of my comment is more relevant maybe: if we are gonna do all stream parsing in C, then the C->lua interface should deal with parsed data and not raw bytes. this of course is way less convenient when it comes to creating that interface.

(or i dunno, maybe i'm wrong and there is some benefit to having the C side catch edge cases like interleaved messages, dropped bytes etc, and have the lua interface handle "cleaned-up" 3- or 4-byte packets.)

in any case, i don't think performance is the main issue - certainly would need profiling to support any decision around that.

[patriciogonzalezvivo]

It have been a very interesting trip of discoveries : ) (I'm really new to audio/midi, I have more experience on the CG world). I notice that only USB devices send 3 bytes packages. They look like this:

...
IO  hw:2,0,0  nanoKONTROL2 MIDI 1
[~]$ amidi -p hw:2,0,0 -d                                                                                     
B0 40 7F
B0 40 00
B0 40 7F
B0 40 00
B0 30 7F
B0 30 00
B0 20 7F
B0 20 00
B0 41 7F
B0 41 00
B0 31 7F
B0 31 00
B0 21 7F
B0 21 00
B0 42 7F
B0 42 00
...

While a program like aplaymidi running on the same system will send events to the Virtual RawMIDI port like this:

[~]$ amidi -p virtual -d                                                                                              
 
B0 07 64
90 3A 5A
C1 18
B1 07 64
C1 18
91 3A 5A
90 3A 00
   39 5A
91 3A 00
   39 5A
90 39 00
   3A 5A
91 39 00
   3A 5A
90 3A 00
   3C 5A
91 3A 00
   3C 5A
90 3C 00
   3A 5A
91 3C 00
   3A 5A
90 3A 00
91 3A 00
90 3A 5A
91 3A 5A
...

This packages comes in pairs, groups of 3 or even 5 bytes. depending if the last type of event is different and if it can package other bytes under the same package. Took me a while to understand that it was the previous parsing reading every 3 bytes was the reason I was getting events only when channels were change and usually the order of the parameters was weird.

As I said, I'm new to audio/midi processing, so VERY probably this is totally obvious. I guess I'm sharing my own learning process : )

In any case I leave it to the experts : )

[catfact]

wooooo ok gotcha. i had no idea that the format was totally different and that ALSA would omit status bytes like that. how horrible! (obviously i am no ALSA midi expert either, most of my MIDI experience is with hardware.)

much becomes clear.

[ngwese]

If I remember correctly the USB encapsulation for MIDI disallows “running status” which why the messages are always 3 bytes (in reality I believe the full USB encapsulation is 4 bytes which includes the endpoint information).

It looks like the virtual raw MIDI might be using running status which is disappointing.

(apologies for the open/close on the ticket, was adding a comments via a phone)

[catfact]

thanks @ngwese. "running status" was the key concept that i had totally forgotten.

but if that is the only edge case, it seems like it would be really simple to handle in parser: check the top bit 0x80: if set, it's a status byte; if clear, re-use last status byte and continue.

in terms of updating the existing system to handle this: it seems like it would be sufficient to (A) include a flag for "running status" in the midi event, or even (B) just pass the 2 bytes to lua and let the lua parser check for status bit on first byte.

[derselbst]

paging @derselbst the question here is whether it would be OK to include some of fluidsynth's MIDI stream parsing code in a GPL3 project. (fluidsynth being GPL2.) none of us are laywers, but it seems like it is "technically" not allowed without specific permission.

Glad that you guys care :) I'm not a lawyer either, but note that fluidsynth is LGPL-2.1 licensed. And according to the license compatibility matrix: "LGPLv2.1 gives you permission to relicense the code under any version of the GPL since GPLv2."

So, I would say, just take whatever code you need, keep the "FluidSynth - Peter Hanappe and others" comment (so that people know where this is coming from) and everything will be fine :)

[patriciogonzalezvivo]

Thank you @derselbst

@catfact @artfwo @ngwese how should we move forwards?

[catfact]

i think the present consensus is, this needs a little more work:

• we can best address the new use case by performing a running-status check (which is cheap) on all reads in dev_midi objects. no changes to event loop or lua handlers.

• we can keep the parser state in the dev_midi struct, which has a well-defined lifecycle (via dev_midi_deinit) rather than allocating it in the RX thread.

• if i understand correctly, the parser state really only needs to be a single byte (latest status byte value) and we don't really need the new parser code. (we can handle things like sysex on the scripting end.)

if we do end up with edge cases like corrupted or split MIDI packets, then we could revisit adding a more robust parsing mechanism to matron or using fluidsynth's. in that case we would also want to make substantial changes to how MIDI messages are passed between C and lua layers, which is beyond the scope of this PR.

---

oh, BTW: you mention above that you have seen 5-byte packets as well. that seems strange; or at least, doesn't fit with the running-status behavior. can you capture an example of this so we can determine what's going on and how best to handle it?

[patriciogonzalezvivo]

The 5 bytes were a header, 2 pairs and other 2 pairs. But all resolve in the same event.

About your points, is there someone willing to take that work? My original goal wasn't to deal with byte parsing but to get Virtual RawMIDI working, it just happens that the parser was the issue. I found the original parsing a bit confusing to read and deal with. So I went for the best efficient and low lever OSS implementation I could found. I'm happy to pass the torch, or break this PR into the Vitual port stuff and the parser issue, so someone else can follow the consensus.

[ngwese]

I'm mildly curious about unlocking rtpmidi so I'll see if I can squeeze in some time this weekend to help get this over the finish line.

[patriciogonzalezvivo]

Thanks!

[ngwese]

Progress update. I have a first pass at reworking the lower level midi input handling such that it reads data into a buffer instead by individual bytes and the changes include running status support. The next step is to adapt the virtual device support from @patriciogonzalezvivo to using this new input handling scheme.

[patriciogonzalezvivo]

@ngwese Exciting! Thanks for picking this up! If you need help with RTPMIDI or the my code feel free to DM through twitter

[ngwese]

I was able to get the virtual midi device change and rtpmidi setup working without any significant problems.

I have a bit of cleanup I’d like to do, including trying to find a way to name the virtual midi device ant the alsa level much like snd_seq interface appears to do. After that I’ll put something up for review.

[patriciogonzalezvivo]

That's fantastic progress! Did you see how network devices appear under the RTPMidi port? Do you think in a future we can monitor those and expose them also?

[ngwese]

I did see how rtpmidi sources appear under the rtpmidi device.

Overall I found rtpmidi itself to be fairly fragile. Hosts coming and going weren’t handled consistently and 2 of the 3 sessions I connected for testing randomly dropped the network connection without warning.

The virtual port stuff seems fine but there looks to be a good amount of testing and experimentation needed before rtpmidi itself would be a candidate for direct support out of the box.

[ngwese]

Closing this out as a result of merging #1206

@patriciogonzalezvivo if you pull down main and rebuild hopefully should have a working virtual midi device to pair with your rtpmidi setup. I will continue looking at the possibility of deeper rtpmidi integration time permitting.