MIDI.noteOff does not seem to work reliably. about midi.js HOT 6 CLOSED

Weird, I cannot reproduce. Is this happening on the MIDIPlayer demo for you?

from midi.js.

Just an idea, and maybe also a cure for the problem mentioned above:

The code "as is" did not work for me when I tried to play notes in real time, to get the right delay for noteOff commands I had to make the following fix where the ramp time is defined with ctx.currentTime:

root.noteOff = function (channel, note, delay) {
delay = delay || 0;
if (delay < ctx.currentTime) delay += ctx.currentTime;
var source = sources[channel + "" + note];
if (!source) return;
source.gain.linearRampToValueAtTime(1, delay);
source.gain.linearRampToValueAtTime(0, delay + 0.2);
source.noteOff(delay + 0.3);
return source;
};

The problem was that after the script has been initialized.at time point x, and the delay time for noteOff occurs in the past when depressing a button to stop playing a note with the the normal script. The above fix takes notice of the currentTime to stop the note from playing at the time I want it it to stop.

midi files still play back nicely here as expected, and playing in real time will now work just as expected.

Just an idea: the values of 0.2 and 0.3 could ofcourse be used as "release" parameter for ADSR like time settings.

Maybe we could also catch and flag the use of damper info (controller 60 or 40h I believe) in the midi file to disable source.noteOff to get very much better playback of piano and other mid's I suppose?

from midi.js.

@Miranet, I think you've got it. Let me patch in your fix and try.

from midi.js.

@mudcube This not for the MIDIPlayer demo. I discovered this when trying to use brass (or other long sustaining) instruments in my VexFlow integration. (I didn't need to bother with noteOff for pianos or clean guitars.)

If @Miranet's patch doesn't work, I'll try to provide a short script to reproduce the bug.

from midi.js.

Excellent. Everything works as expected with @Miranet's patch. @mudcube perhaps you can patch this into head?

from midi.js.

@Miranet, thanks for the fix! It's been added into the latest master branch.

from midi.js.