grimmdude / MidiPlayerJS

♬ MIDI parser & player engine for browser or Node. As a parser converts MIDI events into JSON. Works well with single or multitrack MIDI files.
https://grimmdude.com/MidiPlayerJS/
MIT License
357 stars 52 forks source link

New Tempo bug (possibly related to others) #54

Closed Andrew-J-Larson closed 4 years ago

Andrew-J-Larson commented 4 years ago

Possible relations: https://github.com/grimmdude/MidiPlayerJS/issues/28

When starting a midi file with a tempo of 58, for some reason it doesn't take it, and then starts playing at the 120 tempo speed. I am 100% sure the midi isn't corrupt and even made another file testing it.

Please fix this.

Andrew-J-Larson commented 4 years ago

I can confirm that using the tempo trick (pause setTempo play) used in the listener for the "Set Tempo" event fixes this issue, in case anyone needs the fix.

sylv256 commented 4 years ago

For me, using the tempo trick doesn't help that much. While it does let me set the tempo properly, it makes the midi play really choppy.

sylv256 commented 4 years ago

So I found out that the SetTempo() bug could be due to either incorrect note timing (incorrectly parsing or calculating time signatures) or it could be incorrect tempo. It appears it's happening in specific parts of each song, but I'm not entirely sure. It's most likely the incorrect tempo being passed in the Set Tempo event.

Another theory for why this is happening is that maybe the duration of notes are not being timed correctly.

Andrew-J-Larson commented 4 years ago

You're not alone, I did also notice that using that set tempo trick does make the playback choppy in the places where the song naturally changes tempo.

grimmdude commented 4 years ago

Hey @TehcJS & @TheAlienDrew,

Could you provide a MIDI file that's affected by this? Can't seem to replicate with the files I have on my end. Agree that changing the tempo while playing you need to pause/change/play. Suppose I can work that process into the setTempo function.

-Garrett

Andrew-J-Larson commented 4 years ago

Well initially when encountering this bug, when it sets the tempo for the file, it sets it wrong for only the first tempo set, any further tempo changes sound as they would in the original file... However, since the beginning tempo change wouldn't work, I thought maybe only setting that one would fix it, but when ever I did, the entire files tempo would change with it (because of not pausing, setTempo, and resuming)... But then tempo would be too fast or slow in other areas in the song, so I found that doing the trick for every "Set tempo" event would fix the tempo issue... But like noted changing the tempo with the pause and play tends to make play back seem choppy.

Andrew-J-Larson commented 4 years ago

Here's an example MIDI where it occurs: http://www.filedropper.com/11-miceonvenussample

It doesn't sound normal until the second normal tempo set... the first tempo doesn't look like it works.

grimmdude commented 4 years ago

Hi @TheAlienDrew,

Just published 2.0.12 which automatically changes player tempo based on tempo events. I think this should fix the issue for you.

-Garrett