search

chrisguttandin / midi-json-parser-worker

The worker which is used by the midi-json-parser package.
MIT License
6 stars 1 forks source link

Is the setTempo interface showing the correct property name ? #329

Closed stephaneeybert closed 4 years ago

stephaneeybert commented 4 years ago

When using the interface I'm surprised by the property name as I expected it to be per quarter and not per beat.

Having just read the MIDI documentation I was expecting:

setTempo: {
  microsecondsPerQuarter: number;
};

instead of:

setTempo: {
  microsecondsPerBeat: number;
};
chrisguttandin commented 4 years ago

Hi Stephane,

I'm not sure. I always thought of a beat and a quarter note as synonyms. Do you have an example file where this is not the case?

stephaneeybert commented 4 years ago

Hi Chris, Thanks for your very prompt response. I am a newbie in music theory and notation so I'm quite careful in my wording when I say something on the subject. I reckon the quarter note is one beat if the time signature denominator is 4 but if it's different than 4 then a quarter note is not any longer one beat as seen in the example 20.

chrisguttandin commented 4 years ago

Yes, you're right. The time signature event allows to change the numerator and then a quarter is not necessarily a beat anymore.

Do you want to submit a PR to change microsecondsPerBeat to microsecondsPerQuarter as you suggested?

stephaneeybert commented 4 years ago

Sure ! I understand the correction is only about renaming the property. I could not touch at the API internals for fear of breaking something, my knowledge of music theory being so limited.

chrisguttandin commented 4 years ago

Thanks again for fixing this. #330 is merged and v8 is published on npm. I will continue to update packages which depend on this.

stephaneeybert commented 4 years ago

Glad I could help ! I shall update my dependency now.