cifkao / html-midi-player

🎹 Play and display MIDI files on the web
https://cifkao.github.io/html-midi-player/
BSD 2-Clause "Simplified" License
663 stars 60 forks source link

Notes out of range cause other notes on the same tick to skip playing. #27

Closed majorcyto closed 3 years ago

majorcyto commented 3 years ago

I am not sure if this should be reported here, or at Magenta. But when you have a song playing that has notes out of range, any other notes on the same tick / vertical slice are also skipped even though they are valid and have valid sound files.

This was reported to me by a user that noticed the issue. We then tested by removing the out of pitch notes and now the other notes on the same tick / vertical slice play correctly.

The Inspector Console does make note that it detects notes out of range: SoundFont Pitch 26 is outside the valid range for Lute (28-100)

But when it actually gets to the note it produces this error and causes all notes on the same tick to skip, even the valid ones:

Defaults.ts:122 Uncaught Error: buffer is either not set or not loaded
    at Xn (Defaults.ts:122)
    at Go.start (index.ts:116)
    at N.playNote (jsdelivr-separator.js:1)
    at T.playNote (jsdelivr-separator.js:1)
    at q.playNote (jsdelivr-separator.js:1)
    at ia.callback (jsdelivr-separator.js:1)
    at ia._tick (index.ts:116)
    at sa._tick (index.ts:116)
    at zo.invoke (index.ts:116)
    at index.ts:116

It produces an error print for each note out of range, Which I think is part of the issue on why it is skipping valid notes to instead of only ignoring the out of pitch notes.

image

I kind of wish we had an attribute that would just let us completely skip reading out of pitch notes so they don't show on the visualizer either. But the real need here is to not skip valid notes if they are on the same tick as an out of range note pitch.

cifkao commented 3 years ago

Yes, this is definitely a Magenta.js issue and I think I've already encountered it too. Unfortunately there is not much we can do about it here. Which pitches are valid depends on which SoundFont is used. And if the oscillator-based synth is used (i.e. without sound-font), all pitches should be valid.

It seems like it should be easy to fix that in https://github.com/magenta/magenta-js/blob/master/music/src/core/soundfont.ts The code is already trying to remove the invalid pitches, but it looks like there might be a bug in that.

cifkao commented 3 years ago

And as a general rule, whenever you have a problem with a specific MIDI file, you can try uploading it e.g. to the Magenta visualizer demo. If you get the same issue there, it's most likely a Magenta.js issue.

majorcyto commented 3 years ago

Ah ok,

I will report it there then as well. And yes, this is a Custom Soundfont I am making so I know all of the exact pitch ranges of the instruments. I just wasn't sure if it could be 'worked around' here on your end or not. But no worries, I will report it there.

majorcyto commented 3 years ago

Reported it here: https://github.com/magenta/magenta-js/issues/581

cifkao commented 2 years ago

Should be fixed now after upgrading to @magenta/music@1.23.1!