Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loading all GM instruments for midi file #268

Open
Boscop opened this issue Nov 3, 2022 · 27 comments
Open

Loading all GM instruments for midi file #268

Boscop opened this issue Nov 3, 2022 · 27 comments

Comments

@Boscop
Copy link

Boscop commented Nov 3, 2022

Hi, thanks for this library, it seems very useful :)
I'm looking for a GM soundfont player js lib that supports all GM instruments, so I'm wondering:
When loading a GM file, why aren't all the instruments that loaded that the file uses?
In other words, why is this line commented out?

// instruments: midi.getFileInstruments(),

@page200
Copy link

page200 commented Dec 27, 2022

Commenting this line back in adds more instruments, but they play wrong parts. So there are probably other bugs. Possibly related: issue #226

I'm looking for the other bugs. Feel free to help. The events in the eventQueue have more diverse channelIds than the final sound output (I see about 10 channel IDs, which seems correct, but I hear only about two instruments that play the wrong parts/tracks of the file). So I'm trying to find what part of the code plays the events from the eventQueue. So far, I wasn't able to break on change of eventQueue.

@page200
Copy link

page200 commented Dec 27, 2022

Maybe MIDI.js doesn't properly use the info contained in programChange events for selecting appropriate instruments from the SoundFont.

@page200
Copy link

page200 commented Dec 27, 2022

In case you want to check whether someone fixed this in their fork of this repo, there are several ways to find repos that made changes: https://stackoverflow.com/questions/54868988/how-to-determine-which-forks-on-github-are-ahead

@page200
Copy link

page200 commented Dec 27, 2022

If I understand issue #256 correctly: the branch called "abcjs" fixes this, but introduces other problems (playing one or two notes 3 or 4 seconds after resuming from a pause).

So looking into the changes made in that branch might help.

@hmoffatt
Copy link

js/midi/player.js contains the sequencer (the code that plays the event queue).

@page200
Copy link

page200 commented Dec 28, 2022

The newest branch of this repo (named "abcjs") plays all instruments of a MIDI file correctly, but plays several unexpected notes in the first few seconds instead of the first notes of the song (when playing the song for the first time). Fixing that seems more promising than fixing wrong instruments in older branches.

The player.js of that branch is at https://github.com/mudcube/MIDI.js/blob/abcjs/js/player.js

  1. Fixed problem: I successfully removed two of the unexpected notes by commenting out the calls to MIDI.noteOn() at https://github.com/mudcube/MIDI.js/blob/abcjs/examples/MIDIPlayer-v2.html#L157 and https://github.com/mudcube/MIDI.js/blob/abcjs/examples/MIDIPlayer-v2.html#L198. (Details: Those note played at https://github.com/mudcube/MIDI.js/blob/abcjs/js/adaptors-AudioAPI.js#L138, even if I keep source.buffer at null then instead of initializing it. The caller is at https://github.com/mudcube/MIDI.js/blob/abcjs/examples/MIDIPlayer-v2.html#L149.)

  2. Remaining problem: When https://github.com/mudcube/MIDI.js/blob/abcjs/js/player.js#L262 is reached for the first 4 times, the respective notes (first four notes of the song) don't play. After that line of code is reached for the 5th time, the 5th note plays with the wrong instrument. After a few more "silent notes", player.emit() in player.js is reached for the 1st time. The subsequent notes play correctly. So player.emit() (or some code near it) fixes the sound problems? And it should be called earlier somehow?

    The line that plays notes (including the note with the wrong instrument) is https://github.com/mudcube/MIDI.js/blob/abcjs/js/adaptors-AudioAPI.js#L250. This line is never reached by the "silent notes".

    The condition if (buffer) is skipped at https://github.com/mudcube/MIDI.js/blob/abcjs/js/adaptors-AudioAPI.js#L236 during the "silent notes". Maybe something's wrong with that buffer. Or rather something's wrong with the programId that is used to index into _buffers at https://github.com/mudcube/MIDI.js/blob/abcjs/js/adaptors-AudioAPI.js#L234-L235

(Maybe the remaining problem has something to do with promise and timeout in player.js? Where is galactic defined?)

@page200
Copy link

page200 commented Dec 29, 2022

During the problems, the values MIDI.channels[channelId].program are wrong. They are equal to the respective channelId, but they shouldn't be. Where do they get initialized? They have those values as early as in the begining of onPageReady().

After a few seconds, they get updated, and the problems stop. They get updated at https://github.com/mudcube/MIDI.js/blob/abcjs/js/adaptors.js#L145-L151. Why is this code reached too late? In the MIDI file, programs/instruments get defined, and then the notes play. In MIDI.js, several notes attempt to play and only a few seconds later the program values get set correctly.

A place in the code where you can see the wrong program values initially (causing trouble such as skipping if (buffer)) and correct values later on is https://github.com/mudcube/MIDI.js/blob/abcjs/js/adaptors-AudioAPI.js#L232-L236

@page200
Copy link

page200 commented Dec 29, 2022

Replacing if (delay) by if (false) at https://github.com/mudcube/MIDI.js/blob/abcjs/js/adaptors.js#L145 so that the program values are set correctly too early (as a temporary workaround) reveals more about the problem:

The following things are executed too late and in the wrong order:

Do these scheduled tasks trip over each other? What's weird is that there are no such problems anymore after clicking "stop" and "start" again.

Or is delay computed wrong for these tasks? delay values seem very similar after stopping and restarting the song, where the problem don't appear.

@mk-pmb
Copy link

mk-pmb commented Dec 30, 2022

setTimeout is not meant to have precise timing. The delay argument it takes is meant as the minimum delay. It might thus not be appropriate for music events.

@page200
Copy link

page200 commented Dec 30, 2022

@mk-pmb:

As far as I know, requestAnimationFrame might have more reliable timing than setTimeout, but the music would get interrupted when the user switches to another window or browser tab, which is not desired. The user might want to enjoy the music playing in the background.

Overall, setTimeout has reliable timing in MIDI.js: restarting the song works well. Only the first seconds of the first start are problematic. The question is what happens there. If we identify the difference between the first and second start, we might find solutions.

So far, I couldn't find the cause of the differences in the Chrome Profiler etc. Maybe you can help.

@urobot2011
Copy link

urobot2011 commented Jan 2, 2023

Maybe MIDIPlayerJS and SoundFont-Player are useful. As far as I know, that doesn't seem to be the problem.

@urobot2011
Copy link

urobot2011 commented Jan 2, 2023

Oh, I forgot the link. The link is here:

@urobot2011
Copy link

Maybe you can use midi.js and soundfont-player together. This will require some code modifications. I am also thinking about this issue. However, while making Kalimba-sheet-music-writer, I found out about the fatal flaw of MidiPlayerJS. So I think the combination of midi.js and soundfont-player will be more powerful.

@urobot2011
Copy link

@page200
Copy link

page200 commented Jan 2, 2023

Welcome and thank you for the comments! :)

Regarding soundfont-player and MidiPlayerJS, please see my comments in danigb/soundfont-player#107

@acosme
Copy link

acosme commented Aug 10, 2023

hi @page200 how did you load your soundfounts in 'abc' branch? i followed:

https://github.com/mudcube/MIDI.js/blob/abcjs/examples/Basic.html
and
https://github.com/mudcube/MIDI.js/blob/abcjs/examples/MIDIPlayer-v2.html

as follow:

` MIDI.setup({
soundfontUrl: 'assets/soundfont/',
instrument: "acoustic_grand_piano",
onprogress: (state, progress)=> {
console.log(state, progress);
}
}).then(()=> {

  // soundfont loaded

  MIDI.setVolume(0, 127);

  var file = "assets/sound/quartet/1.mid"

  MIDI.player.load({
    src: file
  }).then(()=> {

    MIDI.noteOn(0, 88, 127);
    MIDI.player.start();

  }).catch((err)=> {
    console.log("------------- err")
    console.log(JSON.stringify(err))
  })   
});  `

using ionic + capacitor

@page200
Copy link

page200 commented Aug 10, 2023

@acosme I followed https://github.com/mudcube/MIDI.js/blob/abcjs/examples/MIDIPlayer-v2.html as well. Before I look into the details of your question and code, may I ask what you are getting at: does your code solve the problems described here, or are you having other problems?

@acosme
Copy link

acosme commented Aug 11, 2023

@acosme I followed https://github.com/mudcube/MIDI.js/blob/abcjs/examples/MIDIPlayer-v2.html as well. Before I look into the details of your question and code, may I ask what you are getting at: does your code solve the problems described here, or are you having other problems?

I was coming to it, but i think my ionic+capacitor app has some issue that don't allow to load the soundfount correctly, in branch master all good, but not in 'abc'. Something change in abc about load in 'mobile', its why i ask you how did you just load, but i used the same example as well.

@page200
Copy link

page200 commented Aug 13, 2023

I haven't tried 'mobile'. Thank you for mentioning the 'mobile' problem with the abcjs branch. This seems to make MIDI.js even more difficult to get to work.

What do you think about https://github.com/surikov/webaudiofont as an alternative? There's a recent example for getting it to play in the background if needed (surikov/webaudiofont#92 (comment)) and I recently fixed its pitch-bend functionality (surikov/webaudiofont#95).

@mk-pmb
Copy link

mk-pmb commented Aug 14, 2023

What do you think about https://github.com/surikov/webaudiofont as an alternative?

Last time I tried it myself, the surikov repo had huge advantages in the "make stuff work" aspect, but it was very complicated to verify whether the apparent license grants would be legally effective in Europe. Doesn't help that some crucial license information is scattered in various places in the repo that contradict some of the machine-readable license statements. The reactions in closed license issue threads make it seem like he's a lot less concerned about licences than I have to be, and the home country stated in his GitHub profile gives a good idea why.

Nonetheless it's a nice resource for learning and private use. Seeing how actual soundfonts work, helped me make my own converted sound font. Unfortunately, I still haven't found a good solution for hosting the result. The huge storage requirement (6 MB original becomes 124 MB converted) made me realize that we need a structurally better way to encode sound fonts.

@page200
Copy link

page200 commented Aug 16, 2023

some crucial license information is scattered in various places in the repo that contradict some of the machine-readable license statements.

Which license statements would you like to be changed to what? In order to better understand, I also wonder how that influences your use case?

The huge storage requirement (6 MB original becomes 124 MB converted) made me realize that we need a structurally better way to encode sound fonts.

Are SoundFonts stored inefficiently by webaudiofont? And do you have any idea why? As far as I know, the original SoundFont files use lossless audio compression, so a 6 MB original wouldn't be much larger than 40 MB when decompressed.

@mk-pmb
Copy link

mk-pmb commented Aug 16, 2023

Which license statements would you like to be changed to what?

I don't know, because I have no idea what the real, legally effective license status currently would be. It might even vary by country.

Are SoundFonts stored inefficiently by webaudiofont?

I'm not sure about the format names here, but…

And do you have any idea why?

I know the major part of the conversion is to encode all instruments as a waveform, which destroys any potential cleverness of the original storage format. Then, in the next step, a modern audio codec is called to the rescue and tries to mitigate the disaster. But since it's a general-purpose audio codec, it cannot achieve the original level of cleverness that the synthesizer had available.

@mscuthbert
Copy link
Contributor

I presume people know that MIDI.js is abandonware and whatever is decided from this thread is unlikely to be implemented. I have an actively developed fork at https://github.com/mscuthbert/midicube

@page200
Copy link

page200 commented Dec 28, 2023 via email

@mscuthbert
Copy link
Contributor

Midicube uses the same soundfont in JS/base64 format as MIDI.js. If you want to load all instruments you'll need to iterate over all 255 sounds but I think that is a bad idea for user experience.

My company www.artusimusic.com uses the Midicube fork and loads instruments as they are required for various assignments.

@page200
Copy link

page200 commented Aug 28, 2024

@urobot2011 The discussion at danigb/soundfont-player#107 (comment) is locked, and it's more relevant here, so I'll reply here.

There, I wrote:

@urobot2011 Color Piano uses MIDI.js, but probably an old branch of MIDI.js that doesn't play all instruments correctly. (Color Piano only plays the piano.)

I might have been wrong with my "probably" statement there. #256 (comment) and https://github.com/KomeijiSatori/3d-piano-player/blob/master/README.md imply that KomeijiSatori got a good solution by using code from Color Piano (redirected from http://mudcu.be/piano) instead of using branches from GitHub whose respective problems had been described at #256 (comment).

I haven't checked whether all problems discussed in that comment and here above are indeed solved in https://github.com/KomeijiSatori/3d-piano-player/. I've inquired at #256 (comment)

@page200
Copy link

page200 commented Aug 28, 2024

To sum up some of the discussions so far:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants