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.8k stars 640 forks source link

First note not played (again) #165

Open eqyiel opened 8 years ago

eqyiel commented 8 years ago

Every MIDI file I give to MIDI.Player.loadFile will skip playing the first quarter note.

For example, using "Test 1" (commented out here), only three notes are played back.

The file clearly has four:

Reproduced here (master branch as of 2016-01-19).

eqyiel commented 8 years ago

For what it's worth, I can't reproduce this behaviour with jasmid.

paulrosen commented 8 years ago

I'm seeing this issue, too.

paulrosen commented 8 years ago

I don't understand the code yet, but it looks like startAudio() is counting the time of meta events. Here are the first four events in my file. The second, third, and fourth events are all caught by the if test that contains the continue statement.

data[0]:
[
    {
        event: {
            deltaTime: 0,
            microsecondsPerBeat: 277778,
            subtype: "setTempo",
            type: "meta"
        },
        ticksToEvent: 0,
        track: 0
    },
    0
]

data[1]:
[
    {
        event: {
            deltaTime: 0,
            subtype: "trackName",
            text: "Title of tune",
            type: "meta"
        },
        ticksToEvent: 0,
        track: 0
    },
    0
]

data[2]:
[
    {
        event: {
            deltaTime: 0,
            subtype: "endOfTrack",
            type: "meta"
        },
        ticksToEvent: 0,
        track: 0
    },
    0
]

data[3]:
[
    {
        event: {
            deltaTime: 0,
            channel: 0,
            deltaTime: 0,
            noteNumber: 64,
            subtype: "noteOn",
            type: "channel",
            velocity: 64
        },
        ticksToEvent: 0,
        track: 1
    },
    0
]
paulrosen commented 8 years ago

I still don't understand exactly what is going on in startAudio(), but changing this line appears to fix the problem for me:

if ((queuedTime += obj[1]) < currentTime) { // was "less than or equal", changed to "less than" I'd be willing to create a pull request, but I'd like to know how I can tell if I broke something else.

eqyiel commented 8 years ago

@paulrosen thanks for looking into this!

Can confirm that it fixes the issue, at least partly.

For some reason I can't get it to play inlined base64 data at all with the above change, but loading midi data from an acutal file works.

Could you tell me if something like this works for you?

window.addEventListener("load", function () {
  // works:
  // var data = "test.mid";
  // doesn't work:
  var data = "data:audio/mid;base64,TVRoZAAAAAYAAQABAMBNVHJrAAAARwD/WAQEAhgIAP9RAwehIAD/AwlOZXcgVHJhY2sAwHMAkDxkMoA8MIEOkDxkMoA8MIEOkDxkMoA8MIEOkDxkgT+APDAB/y8A";
  MIDI.loadPlugin({
    soundfontUrl: "soundfonts/",
    instrument: "acoustic_grand_piano",
    onprogress: function(state, progress) {
      console.log(state, progress);
    },
    onsuccess: function() {
      MIDI.setVolume(0, 127);
      MIDI.Player.loadFile(data, MIDI.Player.start);
      console.log(MIDI.Player.playing);
    }
  });
});

That code is pretty much straight from the example above. Tarball here to make it easier.

@mudcube does this sound familiar at all?

paulrosen commented 8 years ago

I seem to recall that you need to strip off the first part of your midi string. (I actually construct my own structure so I don't use the referenced code any more.) If you step through it you'll see where it is expecting to find the header.

On 3/13/16 8:41 PM, Ruben Maher wrote:

@paulrosen https://github.com/paulrosen thanks for looking into this!

Can confirm that it fixes the issue, at least partly.

For some reason I can't get it to play inlined base64 data at all with the above change, but loading midi data from an acutal file works.

Could you tell me if something like this works for you?

|window.addEventListener("load", function () { // works: // var data = "test.mid"; // doesn't work: var data = "data:audio/mid;base64,TVRoZAAAAAYAAQABAMBNVHJrAAAARwD/WAQEAhgIAP9RAwehIAD/AwlOZXcgVHJhY2sAwHMAkDxkMoA8MIEOkDxkMoA8MIEOkDxkMoA8MIEOkDxkgT+APDAB/y8A"; MIDI.loadPlugin({ soundfontUrl: "soundfonts/", instrument: "acoustic_grand_piano", onprogress: function(state, progress) { console.log(state, progress); }, onsuccess: function() { MIDI.setVolume(0, 127); MIDI.Player.loadFile(data, MIDI.Player.start); console.log(MIDI.Player.playing); } }); }); |

That code is pretty much straight from the example above. Tarball here https://demo.rkm.id.au/midi-js-bug.tar.gz to make it easier.

@mudcube https://github.com/mudcube does this sound familiar at all?

— Reply to this email directly or view it on GitHub https://github.com/mudcube/MIDI.js/issues/165#issuecomment-196089559.

Paul Rosen --- Life is a musical, every once in a while the plot stops and you start singing and dancing --- http://abcjs.net/ http://zuzushotfive.com/ http://itsfloorplay.com/

eqyiel commented 8 years ago

@paulrosen thanks for the hint!

brownian commented 8 years ago

I'm seeing this issue, too. Tested with some wild dowloaded midi file and a midi file produced by lilypond.

@paulrosen, thank you, that fixed this issue for me, too :)

brownian commented 8 years ago

But this seems to double the last note .(

hmoffatt commented 8 years ago

@paulrosen I get double notes with your suggested change. I think it's possible because startAudio loops over midi.data until it has queued 100 messages or reached the end of data, but there may be more events at the same time.

Next time startAudio is called it resumes at the same TIME, not the same place in midi.data, meaning some events may get repeated or skipped. If the time comparison is < then some events are repeated, if the time comparison is <= then some events are skipped.

I think it would be better if startAudio kept a track of where it was up to in the queue, but I guess that makes it more difficult to seek around. With the current algorithm it should not exit in the middle of events at the same time.

vee-L commented 8 years ago

I had a problem with the suggested change where it would not really stop playing, seemingly bypassing the end of the data (probably some kind of doubling issue) and eventually crashing the browser, so I had to force it to stop by making the previously suggested line fix like this

    if ((queuedTime += obj[1]) < currentTime || currentTime >= midi.endTime) { // added endTime check

but that is possibly just my personal implementation that's behaving oddly, or something else that was interfering. Also getting double notes.

jcortez commented 8 years ago

I was having problems where my MIDI files would not even play in the browser until I changed the code to what @vee-L proposed. Actually, even @paulrosen 's fix worked for me. Thanks @vee-L and @paulrosen for the fix. I'm not getting double notes playing my MIDI files.

nhatier commented 7 years ago

I got a file where the first note isn't played, and if I apply the temporary fix, the song doesn't play - all notes are silent.

Here's the first measure of the file:

'data:audio/mid;base64,TVRoZAAAAAYAAQAHAMBNVHJrAAAAwwD/AytzY2h1dHpfd2VpaG5hY2h0c2hpc3RvcmllXzA1X2ludGVybWVkaXVtX2lpAP8BEkJ5IEhlaW5yaWNoIFNjaHV0egD/AiZUaGlzIHZlcnNpb246IENvcHlyaWdodCDvv70gMjAxMSwgSm9obgD/AhNBbGwgUmlnaHRzIFJlc2VydmVkAP8BIEdlbmVyYXRlZCBieSBOb3RlV29ydGh5IENvbXBvc2VyAP9RAwOFcQD/WQIAAAD/WAQGAhgIAP8vAE1UcmsAAABYAP8hAQAA/wMGVHJlYmxlAMBIAOAAQACwB38AsApAAJBIboEgkEgAIJBKboEgkEoAIJBIboEgkEgAIJBFboEgkEUAIJBGboEgkEYAIJBDboEgkEMAAP8vAE1UcmsAAABQAP8hAQAA/wMKU3RyaW5ncy1SMQDIMADoAEAAuAd/ALgKQACYPG4AmEVuhVCYPAAAmEUAMJg6bgCYPm4AmENuglCYOgAAmD4AAJhDAAD/LwBNVHJrAAAAUAD/IQEAAP8DClN0cmluZ3MtUjIAyjIA6gBAALoHfwC6CkAAmjxuAJpFboVQmjwAAJpFADCaOm4Amj5uAJpDboJQmjoAAJo+AACaQwAA/y8ATVRyawAAADgA/yEBAAD/AwpTdHJpbmdzLUwxAMsyAOsAQAC7B38AuwpAAJs1boVQmzUAMJs3boJQmzcAAP8vAE1UcmsAAAA4AP8hAQAA/wMKU3RyaW5ncy1MMgDMMADsAEAAvAd/ALwKQACcNW6FUJw1ADCcN26CUJw3AAD/LwBNVHJrAAAAIAD/IQEAAP8DC1RlbXBvIFRyYWNrALAHfwCwCkAA/y8A'
powertieke commented 7 years ago

The temporary fix by @paulrosen causes timing problems later on. What is happening is that the first event with a time value of 0 is being processed, after which the currentTime variable is changed to the queuedTime - offset (which is 0.5 after this event). All following events that start at 0 will be caught in the continue loop because the currentTime == 0.5.

I'll see if I can think up a way to prevent it from skipping these notes. Maybe:

if (currentTime != 0 && obj[1] != 0 ) { 
    currentTime = queuedTime - offset;
} // currentTime stays 0 if it was 0 and the time value of the event is 0
vee-L commented 7 years ago

I had a somewhat lengthy fix to the timing issues but it is very particular to the application i was using this with. I can take a look at it again and see if I can make a PR with a more general version of it.

xrochoa commented 6 years ago

I'm getting this same issue and it's been open for almost a year. Are there plans for fixing this @mudcube ? This is a very cool project but I might have to use another library due to this issue. Please let me know what the plans are. Thanks!

silverhawk184 commented 6 years ago

I am looking for a fix for this too. @mudcube @vee-L

silverhawk184 commented 6 years ago

I was able to fix this issue and a few others I discovered along the way. I have submitted a pull request, but in the meantime, please see my repository. https://github.com/silverhawk184/MIDI.js

nhatier commented 6 years ago

@silverhawk184 Your fix, specifically line 300, somehow makes one of my midi files restart from the beginning after 1/3 is played, and another one not start at all.

This file just doesn't play with line 300 enabled, but plays if I comment it: schubert_d452_sanctus.zip

silverhawk184 commented 6 years ago

@nhatier - Thank you for bringing that up. I have not been able to test with longer songs. I believe I found a fix to the issue. Please give my latest commit a try.

nhatier commented 6 years ago

@silverhawk184 much better, thanks.