mudcube / MIDI.js

:musical_keyboard: Making life easy to create a MIDI-app on the web. Includes a library to program synesthesia into your app for memory recognition or for creating trippy effects. Convert soundfonts for Guitar, Bass, Drums, ect. into code that can be read by the browser. Supports multiple simultaneous instruments and perfect timing.
http://mudcu.be/midi-js/
MIT License
3.82k stars 641 forks source link

Delay in noteOn() can be arbitrarily offset #168

Open espadrine opened 8 years ago

espadrine commented 8 years ago

Even for something as simple as the basic example which plays a single note, we can notice the following by playing around in the JS console:

var wac = MIDI.getContext()
wac.currentTime  // 26.485333333333333
wac.currentTime  // 27.68

The WebAudioContext's currentTime is perpetually increasing, starting from a value of zero. So far so good.

However, the following line of code, present since the very beginning, will negatively impact the delay parameter of the noteOn() method:

/// convert relative delay to absolute delay
if (delay < ctx.currentTime) {
  delay += ctx.currentTime;
}

Consider the case where we play two notes, one with a delay of 2, the other with a delay of 4, with two calls to noteOn() at the same moment, when currentTime is 3. The first note will be played in 5 seconds (because of delay += ctx.currentTime), while the second will be played in 4 seconds (before the first)!

What is the point of modifying the delay? It is unclear to me what the difference between the relative delay and the absolute delay is, but the end result is that the following code gives results that are highly different from what I would assume it to do:

MIDI.noteOn(channel, note1, velocity, 2);
MIDI.noteOff(channel, note1, velocity, 4);
MIDI.noteOn(channel, note2, velocity, 4);
MIDI.noteOff(channel, note2, velocity, 6);

Could you help me understand and hopefully fix this issue in MIDI.js, be it a matter of documentation or a bug in the code?

espadrine commented 8 years ago

It appears that this PR assumes that this is a bug.

gagern commented 8 years ago

Seems I missed this issue when I filed #188. As I wrote there, it seems as if timing in WebAudio is sometimes absolute sometimes relative, as you pointed out here. Compared to that, AudioTag is always relative, and WebMIDI appears to be always absolute. What a mess! The longer I think about it, the more I believe that the correct fix would be to

  1. always use absolute time and
  2. provide a way to compute absolute time from current time and a relative offset, as I did in #189.

In a sense this is talking the solution opposite to #150, by making things absolute by default instead of relative. The main point is that otherwise it's impossible to perfectly time notes, since while you are computing the method invocations you need, the ctx.currentTime will continue to increase so you don't have a common basis for all the notes you enqueue. If backwards compatibility is an issue, this could be made configurable. I guess I'll write yet another pull request.