Support the Sustain pedal

[e7mac]

Currently the MIDI Player ignores all signals for the sustain pedal (Midi control signal 64). It'd be great if the player could respect the sustain pedal as piano music without the sustain pedal sounds way poorer.

[cifkao]

This would be best solved in Magenta.js, so you can try creating an issue there.

A workaround here would be to modify the note sequence by extending the durations of notes, similarly to the apply_sustain_control_changes function.

Also note that the Magenta.js SoundFonts have a maximum note duration (2 seconds I think), after that the note is released anyway.

[e7mac]

Thanks for the links! I attempted to create a similar function and found that magenta.js doesn't return the control changes data at all. I opened a PR in magenta.js, adding that data. Once that merges, I can add that function in here.

[cifkao]

OK. You can also consider submitting another PR to magenta to make their player handle the sustain pedal correctly, which would be a cleaner fix I think.

[e7mac]

Makes sense. I'll plan to fix it in the BasePlayer class then.

I'm new to JS so one challenge for me is that I'm not really sure how to run and test the magenta-js codebase locally. Might you have any pointers on that ?

[cifkao]

I'm new to JS so one challenge for me is that I'm not really sure how to run and test the magenta-js codebase locally. Might you have any pointers on that ?

That should be straightforward, just clone the repo, go to the music subdirectory and then follow the readme there (see the Example Commands section). Take a look at the yarn link command for how you can include your local copy of the package in another project (e.g. this package or a test project).

[e7mac]

Thanks! I opened a PR adding a method to the magenta-js project. I still need to figure out where to call that method from. Might you have an opinion on that already?

[cifkao]

Thanks! But unfortunately I don't know if this can just be solved by calling your applySustainControlChanges. There are two ways I can think of and neither of them really works:

• The first one would be to apply the sustain pedal inside PlayerElement.start() at (or just before) this line:
  

  html-midi-player/src/player.ts

  Line 212 in 2ad098d

  const promise = this.player.start(this.ns);

  
  That way the modified sequence would get passed directly to this.player.start() without affecting this.ns. However, this will break visualizers, because they need the notes of the played and the visualized sequence to match exactly in order to correctly display the active note and for automatic scrolling to work (see this and this bit).
• The second possibility would be to apply the sustain pedal whenever assigning a new sequence to this.ns. I don't think this is desired either, for two reasons:
  • One might want to retrieve the value of the noteSequence property after assigning to it, and it would be unexpected if this value was different than what was assigned.
  • I'm not sure if we want to visualize the modified sequence, but I'd say probably not, I would like to see the original one.

So the only way I see is to modify Magenta.js's BasePlayer to actually take control changes into account when scheduling the notes. This is the only proper fix anyway, since anything that we can do here are just workarounds for BasePlayer not handling this. That being said, I know that might be difficult, so I'm also open to other workarounds (maybe something that will still require changes to the Magenta code, but only small ones) if you can think of any.