search

YuriSizov / gdsion

A software synthesizer library for Godot (GDExtension).
Other
117 stars 2 forks source link

Timer interval/interrupt is described to be measured in 16th of a beat, but is in 4th instead #11

Closed YuriSizov closed 6 months ago

YuriSizov commented 6 months ago

This is a bug in the original implementation of SiON, so changing anything about it would be a breaking change compared to the original version of the synthesizer. However, only Bosca Ceoil Blue would be affected, and it should be trivial to update it accordingly.

Here's what happens. In the original code SiONDriver.setTimerInterruption takes two arguments: the length of each interruption and the callback. The length is described as

length16th Interupting interval in 16th beat.

This is, however, not true, as evident by the code of the method itself. The length of the timer event is set according to the following formula:

length = sequencer resolution * argument length * 0.0625

In other words, we take n/16ths of the sequencer's resolution. So, one sixteenths, right? No exactly, as each beat is equal a quarter of the sequencer's resolution (by default, at least). In MMLParserSettings the resolution is described as

Resolution of note length. 'resolution/4' is a length of a beat.

4 here is actually a variable, not a fixed value, so even that is not exactly true. But that already means that by default the timer is set in 4ths of a beat, not 16ths. You can verify it with Bosca Ceoil and an external metronome. Set both the metronome and Bosca to 40 bpm, keep the pattern at 16/4, and watch how the external metronome ticks on each thick bar, every 4 notes, every 4 timer interruptions.

This makes one timer interruption to be equal a 1/4th of a beat.

So, what can be done? The compatibility can be preserved, for one, as we would simply rename the argument and document it for what it is. But we can also change it to make it work as suggested, and then change Bosca Ceoil to use a different value for its ticker, keeping existing tracks intact.

I think changing the implementation is a more reasonable approach, while we can do it freely. Having the timer fixed to 1/4th of a beat for no apparent reason is just weird. Even if we keep the logic, the implementation should be changed to actually reflect it.

YuriSizov commented 6 months ago

Resolved by https://github.com/YuriSizov/gdsion/commit/0a3cc4dc2b8c41742734c0eda37149eaca7a4bec. The breaking changes was made and now SiONDriver::play_sound, SiONDriver::note_on, and SiONDriver::set_timer_interval expect values in the 1/16th of a beat unit instead of 1/4th of a beat (or 1/16th of note resolution).

The example project was also updated to make sure it sounds the same as before.