mmitch / gbsplay

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

MIDI output slowly gets out of sync if written to separate tracks #93

Closed mmitch closed 1 year ago

mmitch commented 1 year ago

Based on the discussion from issue #88 I have done this:

At the start all three tracks are perfectly in sync, but after some time they drift apart. I suspect we have some problems with rounding errors when encoding the gbsplay ticks to a MIDI delta timestamp. Exporting all channels together hides these rounding errors, but when single channels are exported, they add up (perhaps a busy baseline accumulates more rounding errors than long string notes on another channel).

I use this script for testing:

#!/bin/bash

gbsplay=~/git/gbsplay/gbsplay
plugout=altmidi
timeout=5

die() {
    echo "$@" >&2
    exit 1
}

gbs=$1
track=${2:-1}

[ -z "$gbs" ] && die "need gbs file as first parameter"

"$gbsplay" -o"$plugout" -234 -T $timeout "$gbs" "$track" "$track"
mv "gbsplay-$track.mid" "gbsplay-$track.1.mid"

"$gbsplay" -o"$plugout" -134 -T $timeout "$gbs" "$track" "$track"
mv "gbsplay-$track.mid" "gbsplay-$track.2.mid"

"$gbsplay" -o"$plugout" -124 -T $timeout "$gbs" "$track" "$track"
mv "gbsplay-$track.mid" "gbsplay-$track.3.mid"

Give a GBS file as first parameter. Second parameter is track; it's optional and defaults to track 1.

mmitch commented 1 year ago

I think I've found it: In midi_write_event() in midifile.c we throw away the lowest 14 bits of the internal clock cycles to convert to the lower resolution of a MIDI file:

static int midi_write_event(cycles_t cycles, const uint8_t *data, unsigned int length)
{
    unsigned long timestamp_delta = (cycles - cycles_prev) >> 14;

These thrown away bits need to be remembered and added to the cycles the next time midi_write_event() gets called. A first test sound promising, but my code still needs some cleanup.

Halfway there :)

mrehkopf commented 1 year ago

Ah, reminds me of the phase accumulator thingy I had to do for the VGM output plugin where the resolution is in imaginary 44100Hz audio samples. ^^;

mmitch commented 1 year ago

@pclalv please have a look at the midi-in-sync branch It currently contains only one extra commit d44b24a50a86233d872caedd107ee18d2ca05a78 that should fix the sync issues. I hope this solves your problems.

pclalv commented 1 year ago

my ears tell me that your fix works. well done!

mmitch commented 1 year ago

I'll merge it to master then. Preview: One of the next commits is going to be a fix to the MIDI playback speed, it currently is about 3% too fast.

mmitch commented 1 year ago

This has been fixed with commit d44b24a50a86233d872caedd107ee18d2ca05a78