arduino-libraries / MIDIUSB

A MIDI library over USB, based on PluggableUSB
GNU Lesser General Public License v2.1
491 stars 89 forks source link

Integration with MIDI Library #21

Closed franky47 closed 3 years ago

franky47 commented 8 years ago

Hi,

I'm working on an update of FortySevenEffects/arduino_midi_library, which until now handled only serial-based protocols, to integrate native USB MIDI as an option.

I have a working example that receives USB data using MIDIUSB, passes the packets to my library for parsing and invoking the application callbacks (printing data in this case).

However, the latency is terrible, because of the extra layer of buffering and parsing required to interface both our libraries.

I'd be glad to collaborate on this issue, to make sure we provide the best solution for our users. Ideally I'd like to avoid duplicating this library into mine just to provide USB support.

See FortySevenEffects/arduino_midi_library#52

Thanks !

fab672000 commented 8 years ago

Hi Francois, sounds great. I also developed this WE some work you may be interested in:

On my side I just developed a fully working 1x1 midi interface (compatible with bus powered iPads!) solely based on the arduino MIDIUSB and MIDI libs (adapts any serial midi IN/OUT port and bridges it to usb midi bidirectionally), the lib implements low latency midi on USBMIDI compatible devices (ie leonardo, pro micro, should also work on due) and also takes care of extra power saving while idle. I developed this lib originally to recycle my old casio digital piano that does not have usb midi, works great for that purpose !

Maybe it can be a good example to add to MIDIUSB ? Would someone be interested to see this work to be contributed here ? If yes, I can fork this project and add my MIDI interface as an example of MIDIUSB.

Thanks, -Fabien

franky47 commented 8 years ago

@fab672000, that sounds interesting !

Did you implement your interface by simply packing/unpacking the midiEventPacket_t objects in and out of MIDIUSB, or is there more logic behind the scenes to recompose messages, handle collisions etc ?

fab672000 commented 8 years ago

This first implementation simply wraps and unwraps (bi-directionally as it handles both IN and OUT serial ports) and also takes extra code to care for leds managements (rx + tx when available) as well as power saving. Can you be more specific on what could lead to collisions with only a 1x1 port interface (as an example I can tell that I did not see any message drops while both rx and tx messages were sent but is it what you are referring on?). Sorry in advance for my naive question as it is my first (electronics+software) MIDI design on arduino and I would like to learn more about collisions.

-Fabien

franky47 commented 8 years ago

One thing I can think of is the fact that System Real Time messages (such as Clock, Start, Stop, Continue, ActiveSensing et al) can appear at any point in the MIDI stream, even between bytes of other messages. Since the USB MIDI spec defines the packet to be 4 bytes long, first of which should be the header indicating the type of message (potentially to allow parsers to be more proficient, I don't know), inserting the Real Time byte into the frame could be problematic, unless you take care of extracting it into a higher priority packet of its own, then keep recomposing the current packet.

The same goes for SysEx, which has arbitrary length. The header should then be persisted to following packets until an EOX (0xF7) is received, while taking care of interleaved Real Time messages.

There's a bit of a state machine to be implemented to have a fully compliant MIDI implementation, which I did over the time with my MIDI library, hence why I'm contacting the authors of MIDIUSB for a possible collaboration.

fab672000 commented 8 years ago

Interesting, thanks for the clarification. So from my humble readings of the arduino MIDI API involved, and similarly as for compressed midi notes ON/OFF modern MIDI handling that is taken care of by the MIDI lib; these Clock, Start, Stop, Continue messages should work as I make no assumptions on the message type. That said, I still do assume that the resulting unwrapped real midi message is [at max] 3 bytes (type, data1, data2). Now I would expect (and hope!) that the arduino midi code simply does not use extra data1 or data2 if packet size < 3, or if it's not the case yet we could make the lib code better to be the case :).

Now on the other hand, I do not handle sysex yet (as it was not needed to interface my digital piano to my iPad). So I would expect my first implementation to not work well with SysEx ATM.

Thanks a lot for getting these aspects to my attention, I'll test more with these in mind soon.

franky47 commented 8 years ago

Have a look at #19, which covers the SysEx case. Moreover, the Section 4 of the USB MIDI Specification lists all the header values for every kind of situation.

fab672000 commented 8 years ago

Excellent thanks! I'll test this code shortly and see if I can extend my support to Sysex as well, with respect of my pro micro (atmega32u4) limitations in term of buffer size as I can anticipate it could be a problem.

Any use case / recommendations I could re-use for testing these realtime messages on iOS (i.e. ipad) plaforms ? Is it handled by GarageBand for ios9 ? (my ipad does not upgrade to ios10, but like the piano was offered as a gift to me!)

fab672000 commented 8 years ago

Hi Francois, So I added sysex handling to my code and will soon publish the solution as a contribution to your excellent MIDI library as it reuses it as well as MIDIUSB. I also added the inverse function of #19 to parse usb formatted sysex and send it to the serial midi output:

void UsbToSerialSysEx(const byte *data, unsigned size)
{
     if (data == NULL || size == 0 || (size % 4) != 0) return ; // usb packets must be DWORDs
    unsigned midiDataSize = (size >> 2) * 3;
    byte lastPacketHdr = data[size - 4];
    if (lastPacketHdr > 4) midiDataSize -= (7-lastPacketHdr); //  adjust size  if needed

    uint8_t midiData[midiDataSize];
    uint8_t *p = midiData;
    size_t bytesRemaining = midiDataSize;

    for (const uint8_t *d = data+1; d<data+size;d+=4)
    {
        switch (bytesRemaining) {
            case 1:
                *p = d[0];   // SysEx ends with following single byte
                break;
            case 2:
                *p++ = d[0];   // SysEx ends with following two bytes
                *p = d[1];
                break;
            default:
                *p++ = d[0];   // SysEx with following three bytes
                *p++ = d[1];
                *p++ = d[2];
                break;
        }
        bytesRemaining =bytesRemaining>3 ? bytesRemaining-3 : 0;
    }
    midiA.sendSysEx(midiDataSize, midiData, true);
}

EDIT: updated to latest code version2

facchinm commented 8 years ago

Hi guys, first of all thanks for all the effort you are putting in this issue, that's really great! I only wanted to tell you that, when I started this library, it was intended as a proof-of-concept and validator for PluggableUSB infrastructure. I'm not a MIDI expert and I'd really appreciate if this lib could get merged in a bigger and more usable one (more suitable to musicians too). If it's not possible, I'd ask if anyone of you is interested in becoming the maintainer of this lib, so patches can be applied steadily and it can evolve nicely.

fab672000 commented 8 years ago

Thank you facchinm, MIDIUSB is a great starting point we could extend to handle all midi needs that are for now splitted into MIDIUSB and MIDI.

I would vote for Francois (if he's interested) for taking the lead because I think his MIDI library code is not only well written and extensive ; it also contains a mocked environment and unit testing following the TDD standards that I also like to use as much as possible.

My humble work on the other hand only represents few (happy) hours sparsed on the two last week ends, where I implemented a fully functional midi interface that now allows my old casio piano to talk to my iPad. For that project to work ; I had to develop a new helper class (MidiBridge) that bridges serial midi to usb bi-directionally. I would be glad to be part of the team too but think Francois would be the best person for coordinating that effort and would offer my humble contributions ; including my complete midi interface soon to be submitted (currently finishing testing sysex new functionality, working better now since recent MIDI commits from Francois allow to filter colliding real time events (i.e. F9) events) . I would also have suggestions to extend the MIDIUSB lib to handle multiple in/outs and extend the functionality there (by using preprocessing while declaring the MIDI descriptor tables as an example).

franky47 commented 8 years ago

For the v5.0 of my library I'm planning a big refactoring that would let the interface work on any kind of transport layer, be it serial (byte by byte IOs) or packet-based (like USB, bluetooth, ethernet and whatnot). At the same time when I'll be focusing on native USB MIDI, and for that we have to decide what to do with both our libs.

At the moment, since there is no dependency system for Arduino libs, users would need to manually download libraries separately (with all the issues that can create for version requirements). I don't know if that's in the works, planned or even though about on the Arduino team, so at the moment, I would rather focus on maintaining a single library that provides MIDI for all kinds of transport layers (serial, usb etc), which would mean merging MIDIUSB into the MIDI Library. I'm not sure how that would work regarding GitHub though..

fab672000 commented 8 years ago

I agree that the longer term vision should be to make the midi sources generic, I had a very similar rationale recently when creating my MidiBridge derived class. I also agree that the MIDI lib should be the top level lib including MIDIUSB as a particular MidiSource. In order to do so though, it also means that MIDIUSB and and MIDI libs should share a common abstract interface (i.e. the way we read write data should share same base methods for any concrete MidiSource).

Now in terms of SOLID considerations, SRP (Single responsibility principle) rule tells me we should consider as well the decoupling of the Midi interface into more classes (as an example MidiSource, MidiEvent and MidiParser entities) ? Would now help managing this new (bigger) superset of functionality.

fab672000 commented 8 years ago

Which would mean merging MIDIUSB into the MIDI Library. I'm not sure how that would work regarding GitHub though..

Not necessarily needed to merge the MIDIUSB lib, we could use the Decorator Design Pattern to wrap the existing code from this lib into a new Decorated class with no MIDIUSB modification (or few if access to internals is now needed) that would delegate its calls to this concrete MidiUSB instance of interest (could be more than one in future) but externally present the abstract MidiSource interface/contract.

facchinm commented 8 years ago

About lib dependencies, we are working towards a solution in the next few months. In the meantime, you can add a link like http://librarymanager/category#Keyword&Keywork2 in the sketch; by clicking it the library manager is opened and displays only the correct dependency. About the library maintenance, take your time to decide the overall architecture and then I'll add you (all) to the maintainer (if you wish of course :smile: )

franky47 commented 8 years ago

One more thing we should discuss before diving in is licensing.

My library is licensed under MIT, which is a bit more permissive than LGPL. From what I saw, most Arduino libs seem to use (L)GPL licensing models. I switched from GPL to MIT as it allows more flexibility in the interaction of various libraries, and I'm fine with forks/modifications of my code, or commercial uses. Also, I did not like the idea that people who use a piece of code have to be constrained to a particular licensing scheme for their own work downstream.

I'll have to dig in deeper into legal implications of license compatibilities to see what approach would work out best.

fab672000 commented 8 years ago

My humble preference and vote would be to harmonize MIDIUSB to use MIT instead of (L)GPL licensing if ok by facchinm ? @facchimn: I'm in if you will ; can also start to add a doxygen autodocumentation contrib this week-end and we could all start creating Unit Tests before we attempt to change anything here ?

facchinm commented 8 years ago

As far as I understand about licensing on embedded platforms (mainly Arduino, which uses none to few precompiled archives) mixing libraries with different licenses is not a problem because we are all distributing the sources. With "mixing" I mean that one depends on the other, not borrowing code from the other implementation. @fab672000 : feel free to document the library, I'll be really glad to merge the PR :smile:

fab672000 commented 8 years ago

@facchinm both licenses work for me, also : the documentation add-ons doxygen + sources comments are already on my forked version! I'll find the pull request number now ...

fab672000 commented 8 years ago

@facchinm please review the PR #15

fab672000 commented 8 years ago

I agree that the longer term vision should be to make the midi sources generic, I had a very similar rationale recently.In order to do so though, it also means that MIDIUSB and and MIDI libs should share a common abstract interface (i.e. the way we read write data should share same base methods for any concrete).

Now in terms of SOLID considerations, SRP (Single responsibility principle) rule  tells me we should consider as well the decoupling of the Midi interface into more classes MidiSource, MidiEvents and MidiParser entities ?  -Fab

  From: Francois Best <notifications@github.com>

To: arduino-libraries/MIDIUSB MIDIUSB@noreply.github.com Cc: fab672000 fabien@onepost.net; Mention mention@noreply.github.com Sent: Thursday, October 20, 2016 11:06 AM Subject: Re: [arduino-libraries/MIDIUSB] Integration with MIDI Library (#21)

For the v5.0 of my library I'm planning a big refactoring that would let the interface work on any kind of transport layer, be it serial (byte by byte IOs) or packet-based (like USB, bluetooth, ethernet and whatnot). At the same time when I'll be focusing on native USB MIDI, and for that we have to decide what to do with both our libs.At the moment, since there is no dependency system for Arduino libs, users would need to manually download libraries separately (with all the issues that can create for version requirements). I don't know if that's in the works, planned or even though about on the Arduino team, so at the moment, I would rather focus on maintaining a single library that provides MIDI for all kinds of transport layers (serial, usb etc), which would mean merging MIDIUSB into the MIDI Library. I'm not sure how that would work regarding GitHub though..— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

franky47 commented 8 years ago

I've just released v4.3 of my lib, so I'll be more available for USB investigations soon. I also have ordered an Arduino Zero and a 101 for USB and Bluetooth transport protocol investigations.

fab672000 commented 7 years ago

Sorry for the delay, back to open source contributions, I am pleased to share my midi 1x1 interface here: https://github.com/fab672000/MIDIUSB_1x1 Note: test folder is only for those interested in running TDDs, all the code to flash with the Arduino IDE is at the root.

Enjoy!

sanotronics commented 6 years ago

@franky47 have you worked this out????

franky47 commented 6 years ago

I've resumed development on this, at the moment I'm hitting an issue with the integration regarding the types used in the MIDI-USB library being defined in a file heavily loaded with platform-specific code, which makes unit testing difficult (in a non-Arduino environment).

I'll propose a PR to move the declaration of the midiEventPacket_t into a separate header file which can be included in 3rd party code without pulling all the USB interface with it.

franky47 commented 5 years ago

Ok so I have a working implementation of an interface between the MIDI library (as a client API) and this MIDI USB library (as transport layer), which should work transparently for users of the MIDI library on supported USB boards.

It's not the absolute optimal code (there's an extra bit of buffering done due to the way the MIDI library works on serial streams and not packet-based data), but I've done a few tests on Ableton Live which sounded promising. Introducing an optimised packet-based transport layer would probably require a large rewrite, so I'll consider this for a future major update.

I'm waiting on a release of the changes from #47 to proceed with release on my side.

Anyone who wants to test right now can use the feat/4.4.0 branch in FortySevenEffects/arduino_midi_library, and the master branch of this repo (as #47 is now merged). The example is unchanged: https://github.com/FortySevenEffects/arduino_midi_library/blob/feat/4.4.0/examples/MidiUSB/MidiUSB.ino

sanotronics commented 5 years ago

Francois!

This is great! will it work on SAMD21 controllers?

franky47 commented 5 years ago

If it is supported by this library, it should integrate seamlessly.

sanotronics commented 5 years ago

I got this trying to compile the MidUsb.ino example, sorry I can't seem to be able to format it properly:

In file included from C:\Users\Franco\Documents\Arduino\libraries\MIDI-LIB-USB\src/midi_UsbTransport.h:32: from C:\Users\Franco\Documents\Arduino\libraries\MIDI-LIB-USB\examples\MidiUSB\MidiUSB.ino:4:
C:\Users\Franco\Documents\Arduino\libraries\MIDI-LIB-USB\src/midi_UsbPacketInterface.h:32:26: fatal error: MIDIUSB_Defs.h: No such file or directory
#include <MIDIUSB_Defs.h>
                          ^
compilation terminated.

I looked in the MIDIUSB library folder and couldn't find this file :/

I commented line 32 from _midiUsbPacketInterface.h but started getting other errors.

I am using windows 10 with IDE 1.8.5

PaulStoffregen commented 5 years ago

@franky47 - Any chance you could test on a Teensy 3.x board? (do you still have the boards I sent a while ago?) The Teensy code library has a MIDIUSB.h header that emulates compatibility with this library. It should "just work" if you're using this API. Emphasis on "should". ;) I'm crazy busy with another hardware project at the moment, so I can't get involved much... but if you've lost the hardware and need another board, please email me directly and I'll get you set up with hardware.

franky47 commented 5 years ago

Hi Paul, yes I still have the board you sent me (thanks again for that !), it's next on my test list. I might have to play a bit with the includes to adapt with your flavour of MIDIUSB, but that should not be a problem if there is a unique way to detect a teensy environment from the preprocessor (anything like #if TEENSY ?)

@sanotronics did you update your install of the MIDIUSB library from the master branch of this repo ? Download an archive of the master branch and replace it in C:\Users\Franco\Documents\Arduino\libraries\MIDIUSB.

gdsports commented 5 years ago

Maybe not the time for USB host MIDI but here it is.

MIDIUSB Host <-> UART bi-directional converter

https://github.com/gdsports/midiuartusbh/blob/master/MIDIUARTUSBH/MIDIUARTUSBH.ino

Nothing new here but my version of MIDIUSB Device <-> UART bi-directional converter. Does not have USB power management like MIDIUSB_1x1.

https://github.com/gdsports/MIDIUARTUSB/blob/master/MIDIUARTUSB.ino

fab672000 commented 5 years ago

Thanks for your work @franky47, it is going to probably help a lot my MIDIUSB_1x1 contrib to support better sysex too, I'll check the feat branch out this week-end!

sanotronics commented 5 years ago

@franky47 thank you, that was it, I did not have the updated MIDIUSB lib.

I tested it and it works like a charm! sent sysex messages (a few bytes long) and received 64 byte sysex messages too, changing the variable sUsbTransportBufferSize to 64 in the example.

lathoub commented 4 years ago

This issue is resolved in https://github.com/lathoub/Arduino-USBMIDI (fully supporting receiving and sending SysEx) that uses the underlaying FortySevenEffects Arduino MIDI Library (USB-MIDI is one of the transport mechanisms ).