justinlatimer / node-midi

A node.js wrapper for RtMidi providing MIDI I/O
MIT License
745 stars 119 forks source link

please update to RtMidi 2.0.1 #24

Closed agoode closed 10 years ago

agoode commented 11 years ago

http://www.music.mcgill.ca/~gary/rtmidi/index.html#download

It looks like this replaces a sleep-loop in the alsa backend with a poll, which should improve some things. It also supports multiple compiled backends, though this may complicate things.

justinlatimer commented 11 years ago

I'll look at doing this when I get a chance (I.e. sometime in the next few months / years)

If anyone needs this sooner, please do the update and test, and raise a pull request into a branch. Let me know what is and isn't working and we'll see what we can do.

On 17/06/2013, at 4:47 PM, Adam Goode notifications@github.com wrote:

http://www.music.mcgill.ca/~gary/rtmidi/index.html#download

It looks like this replaces a sleep-loop in the alsa backend with a poll, which should improve some things. It also supports multiple compiled backends, though this may complicate things.

\ Reply to this email directly or view it on GitHub.

yocontra commented 11 years ago

I went ahead and updated it and everything compiled okay. RtMidi 1.* to 2.* isn't supposed to have any API breakage. Once we get that segfault issue fixed and I can run the tests I'll PR the code

hhromic commented 10 years ago

Hi, I also tried to update RtMidi from 1.x to 2.x myself and just one extra small thing needs to be adjusted (because of the new Multi API approach). In the "binding.gyp" file, you need to provide updated flags for "defines". The following are the new ones:

For Linux: change __LINUX_ALSASEQ__ to __LINUX_ALSA__ or __LINUX_JACK__ depending on needs. I strongly recommend leaving __LINUX_ALSA__ as this is much more compatible. The old define does not work anymore and defaults to a dummy Midi API (does nothing!) so change it one of the above.

For Windows: use __WINDOWS_MM__ (the API for RtMidi 1.x) or the new __WINDOWS_KS__ (kernel streaming driver) for improved performance. As I don't use RtMidi under Windows, I don't know which one is safer.

For Mac: no changes needed.

Doing this and replacing the included RtMidi lib with the 2.x version works just fine (tested!).

Oh one more thing, I just found a very small bug in RtMidi (both versions) with ignoring Timing MIDI events. I already sent an email to Gary, the RtMidi author for fixing it. I would wait for RtMidi 2.0.2 before updating it here. Or if you want to distribute the fixed version, update to RtMidi 2.0.1 and add the following block to RtMidi.cpp:

      case SND_SEQ_EVENT_CLOCK: // MIDI timing clock
      if ( !( data->ignoreFlags & 0x02 ) ) doDecode = true;
      break;

Just before this block:

      case SND_SEQ_EVENT_TICK: // MIDI timing tick
      if ( !( data->ignoreFlags & 0x02 ) ) doDecode = true;
      break;

This will make RtMidi properly ignore all timing messages (I found out it didn't ignore Yamaha Piano clock message).

I can try to make a pull request with all these changes to properly migrate to RtMidi 2.x with this fix included. Let me know!

Cheers, Hugo.

drewish commented 10 years ago

Worth mentioning that @hhromic opened up a PR for this: https://github.com/justinlatimer/node-midi/pull/34

justinlatimer commented 10 years ago

This will be in v0.next! Working with @hhromic to get the last few fixes for this in over the next few days.