nfroidure / midifile

A MIDI file parser/writer using ArrayBuffers
http://karaoke.insertafter.com
MIT License
199 stars 30 forks source link

Running status not allowed #10

Closed argiepiano closed 8 years ago

argiepiano commented 8 years ago

Web MIDI API (http://www.w3.org/TR/webmidi) does not allow for MIDI Running Status. JavaScript is throwing an Uncaught Type Error for running status when using the index.html in the test folder of the MIDIFile project to play a MIDI file that contains running status.

nfroidure commented 8 years ago

If i remind well i had that issue for my poc Karaoke player: http://karaoke.insertafter.com

I thought i got rid of that problem, are you using the latest version?

nfroidure commented 8 years ago

Looks like running status are decoded there so i assume your issue is related to something else https://github.com/nfroidure/MIDIEvents/blob/master/src/MIDIEvents.js#L259-L268 .

Can you share with me the MIDI file causing that troubles?

argiepiano commented 8 years ago

I'm not sure what is going on exactly. I've tested the latest version with a slightly modified version of test/index.html to open and send a file you provide in the demo (MIDIOkFormat0.mid). (There are other unrelated issues with that index.html file, which I'll post separately).

When I use Chrome 47 (Mac), which is the only browser that supports Web MIDI API natively (does not rely on Jazz-plugin), I get: Uncaught TypeError: Failed to execute 'send' on 'MIDIOutput': Running status is not allowed at index 2 (0). I created a file that doesn't have running status events, and I don't get the error.

I don't get the error if I use a browser that relies on Jazz-plugin, like Firefox. I hope this helps. I intend to look a bit more deeply into this in the near future.

nfroidure commented 8 years ago

@argiepiano feal free to share the file with me by email if it is copyrighted. You can also test if with this Karoke player that is using my MIDIPlayer project to see if it is related to the fact the test file is a very poor implementation.

I'm very open to any contribution since i do not find a lot of time to update this library myself.

nfroidure commented 8 years ago

I just saw the test.html file is using the browserified build, maybe it is outdated. Can you pull my recent changes and try again?

argiepiano commented 8 years ago

I'll try a bit later.

Also I just did a pull request fixing a few things in the index.html version - the index.html test was not working correctly.

argiepiano commented 8 years ago

As for the files producing the running status error when sent to the output, it's happening with many of the files you provided in the index.html demo, for example MIDIOkFormat0.mid. I'm attaching two files, one that does not produce the error (trial.mid) and one that does (dablues.mid).

Again, this error is happening in version 1.0.2 when SENDING the info to the output in Chrome, using the demo in test/index.html. I'll try 1.0.3 later.

midifiles.zip

argiepiano commented 8 years ago

OK, I just figured out what was going on. The test/index.html player was sending 2 data bytes to the device regardless of MIDI event. Some MIDI events (like Program Change) only have one data byte, while Note On, etc, have 2. This confused the native Web MIDI API in Chrome into thinking that the second data byte (always 0) was a running status message. I just did a Pull Request that fixes a couple of lines. The test/index.html is now working correctly for me!

Thanks for a great library.