search

mmitch / gbsplay

gameboy sound player
https://mmitch.github.io/gbsplay/
Other
98 stars 20 forks source link

fix playback speed in generated MIDI files #122

Closed mmitch closed 6 months ago

mmitch commented 7 months ago

110 showed that I had a bugfix lying around for over a year, that is not cool…

I have verified both the bug and the fix by

  1. running gbsplay -o wav -1 to create a WAV file that does not contain channel 1
  2. running gbsplay -o midi -2 -3 -4 to create a MIDI file that only contains channel 1
  3. running timidity -Ow -o midi.wav to convert the MIDI file to a WAV (using the default piano sound, but that's sufficient)
  4. running gbsplay -o altmidi -2 -3 -4 to create another MIDI file that only contains channel 1 5.running timidity -Ow -o altmidi.wav to convert the second MIDI file to a WAV, too
  5. import all three resulting WAV files in audacity and play them back together (I had the MIDI tracks panned to all left/all right and ramped up their volume to actually hear anything)

Without this fix, the sound was getting out of sync quite quickly: While the MIDI parts were in sync to each other, they did not match the other 3 channels.

With the fix, everything was fine over the whole 2 minute duration of default gbsplay runtime.

Originally this was just a refactoring to put a proper name on the magic number 124 in the MIDI header.

Understanding the meaning of the value (TIME_DIVISION of 124 with a default TEMPO of 500_000 ms) pointed out a serious bug: 3906 ms of audio data were spread to 4032 ms in the MIDI file, resulting in all generated MIDI files playing ~3% slower than desired.

This has now been fixed by:

mrehkopf commented 7 months ago

I think the time units need to be stated as µs instead of ms (4194304 / 16384 = 256Hz (not 1/256Hz), which amounts to 3.90625ms or 3906.25µs (oh no, .25))

mrehkopf commented 7 months ago

Actually, is it necessary to deviate from the 500000us/beat time? 4194304 / 16384 = 256; 1/256 = 0.00390625 (or 3906.25 us); 499968 / 128 = 3906. 500000 / 128 = 3906.25. I think the 499968us per beat have been derived from an incorrectly assumed value of 3906.0us per tick.

mmitch commented 7 months ago

@mrehkopf you're completely right on all accords :-) I've added another commit to fix the issues, I have recalculated to the same values as you.

I have not yet repeated the tests playing MIDI and WAY side by side, but I'll do that later.

If everything turns out alright, I will squash both commits into one and merge it to master and after that I think I'll release version 0.0.97 because we don't have anything else going on currently.

mmitch commented 7 months ago

Regarding the three timings, we were not 3% off originally, but only 1.5% – I'll update that in the documentation as well.

value tempo [us/beat] divisor [1/beat] tick [us] percent of fixed error related to fixed
fixed 500000 128 3906.25 100% 0%
nearly fixed 499968 128 3906 99.99% 0.01%
original 500000 126 3968.25 101.59% 1.59%
mmitch commented 7 months ago

:ok: playing the exported file alongside the exported WAV file was successful, everything sound in sync