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

Add an array of requested instruments #16

Closed TimMensch closed 6 years ago

TimMensch commented 6 years ago

I needed to know what instruments a file requests in advance, so I added code to collect all of the program change instrument numbers into an array instruments.

grimmdude commented 6 years ago

Hey @TimMensch,

Awesome, thanks for the PR. Do you think it would be useful to store the complete Program Change event in this array so that other info like channel & tick can be referenced?

-Garrett

TimMensch commented 6 years ago

What I needed specifically was to know, in advance, what instruments would be requested, so that I could pre-load those instruments before starting the song.

Since the Program Change event will fire when it fires, I'm not actually sure what I'd need the other information for. If I were displaying sheet music or a piano roll and needed to see that information in advance, I'd also need to be looking at the raw track data to get the note information anyway (which I'm doing right now, in fact), so again, I don't know that it helps to copy the additional information here.

Technically I could have scanned all of the tracks for Program Change events by hand, but since MidiPlayerJS was basically already DOING that, it felt reasonable to cache the information at the source. If you'd like to make the change anyway, feel free, but please don't forget to update the TypeScript information to indicate that it's an array of Event (and, it occurs to me, this will be annoying, because value is optional, so TypeScript will complain that it might be optional...I guess ideally we'd add a ProgramChangeEvent type that has value as a non-optional parameter, which implies a whole slew of other Event-derived types...). I'd prefer to leave it as-is, but it's your choice.

grimmdude commented 6 years ago

Hi @TimMensch,

Got it, caching the used instruments in the Player.instruments array sounds fair. Merged! Thanks,

-Garrett