Non-integer note values do not work with Soundfont class

NoTalkOnlyProx commented 8 months ago

Hey, ran into something interesting. If you pass a non-integer note value to Soundfont.start, no sound will play.

I did some digging, this essentially is because spreadRegions produces regions like this:

{
  midiHigh: 36
  midiLow: 36
  midiPitch: 36
  sampleName: "C2"
}

This means that any note that isn't exactly 36 cannot trigger this region (or any other, for that matter).

No region is found, so start no-ops.

The fix, here, I think would be the set midiHigh to 37 instead of 36, and then correspondingly the following line in findSampleInRegion:

- const matchMidi = midi >= (region.midiLow ?? 0) && midi <= (region.midiHigh ?? 127);
+ const matchMidi = midi >= (region.midiLow ?? 0) && midi < (region.midiHigh ?? 128);

This way the entire number line from 36 up to (but not including) 37 gets mapped to region 36.

After that, I think the logic you already have written should take care of detuning the sample appropriately.

I would submit a PR, but I am not in a great position to test this change, so I wanted to float this as an issue first.

Does this seem reasonable, or am I missing something?

NoTalkOnlyProx commented 8 months ago

Pinging @danigb as presumed code-owner.

I'd be happy to submit a PR if you are on-board with this change.

danigb commented 8 months ago

Hi! Thanks for digging into this issue. Happy to accept a PR 👍

danigb commented 8 months ago

Hey,

I did some housekeeping for this project and decided to fix the issue. Your detailed information was key to do it, so thanks a lot!!

NoTalkOnlyProx commented 8 months ago

Sick! Yeah, sorry I offered a PR and then didn't produce one in a timely manner -- been a busy week. Glad it helped, thanks for fixing it!

danigb / smplr

Non-integer note values do not work with Soundfont class #61