hydrogen-music / hydrogen

The advanced drum machine for Linux, macOS, and Windows
http://www.hydrogen-music.org
GNU General Public License v2.0
1.03k stars 172 forks source link

Hydrogen Jack MIDI output not frame-accurate #535

Open m-grabner opened 6 years ago

m-grabner commented 6 years ago

I compared the timing accuracy of Hydrogen's internal sampler with an external one (LinuxSampler connected via Jack MIDI). I created an artificial drum sample consisting of a single pulse and recorded the generated output with Ardour such that the timing can be evaluated with frame-accuracy when maximally zooming into the wave form view. With a sampling rate of 48kHz and a tempo of 108bpm, the pulses had a distance of either 26666 or 26667 frames, i.e., the error was less than a frame, which is optimal. However, repeating the same experiment with LinuxSampler, the individual pulses were spaced either 26624 or 26880 frames from each other, which are both integer multiples of 256. I choose quite conservative settings for jack when recording (4 periods of 256 frames) since latency (21.3ms with these settings) can easily be compensated, while a single xrun can spoil the greatest take. So timing seems to be accurate only to periods, but not to single frames when using an external sampler via Jack MIDI. The error caused by this issue is in the order of the range a note can be shifted by the lead/lag feature, i.e., not immediatetly noticeable when listening casually, but too large to be totally ignored in my option.

To verify whether this is an problem with Hydrogen or LinuxSampler, I did two more experiments: using LinuxSampler as an Ardour LV2 plugin internally connected to an Ardour MIDI track, and using LinuxSampler as an external application connected to Ardour via Jack MIDI. Both gave frame-accurate results. So let me summarize:

I believe that the problem is in or near the method JackMidiDriver::JackMidiRead(), though I didn't spot any obvious mistake there.

m-grabner commented 6 years ago

I took a closer look at the JackMidiDriver::JackMidiRead() method and found something interesting. The variable "t" always counts from 0 to 2 and then starts again at 0. It is used as the "time" parameter of the jack_midi_event_reserve() function. According to the extensive documentation (see http://jackaudio.org/files/docs/html/group__MIDIAPI.html#ga150dcdc37583e1ecbe0a6f16b6ac48a9), we learn that the "time" parameter means the "Sample offset of event". This leaves some room for interpretation at least, but I believe it means the number of frames w.r.t. the start of the current period when the event should be triggered. To verify this assumption, I did another pulse experiment with Ardour+Hydrogen+LinuxSampler (see above), but modified the code such that 100 is added to the "time" parameter of the jack_midi_event_reserve() function. Indeed the distances between adjacent pulses were again integer multiples of 256, but the absolute positions in time were delayed by exactly 100 frames.

So here is probably the bug: the "time" parameter should indicate the fraction of the current period at which a note is to be played (which is a different value for each note unless the time between two notes happens to be an exact integer multiple of the period size), but instead the "time" parameter periodically counts from 0 to 2. We need to somehow compute the modulo of the absolute frame number when a note starts, and the period size, and pass this number to jack_midi_event_reserve().

m-grabner commented 6 years ago

I committed a proposal to fix this: https://github.com/m-grabner/hydrogen/commits/midi_timing

For easier review, I grouped my changes into 3 commits:

However, there are still some issues:

Comments on these modifications are highly appreciated!

Kind regards, Markus

trebmuh commented 6 years ago

@mauser : would we add a "1.0.0 milestone" to this thread/fix-proposal? I was on my way to do so, but since I don't understand all the underlying codeness thingies involved, I'm flicking it to you :)

grammoboy2 commented 4 years ago

@m-grabner You're talking about JACK MIDI, but my latest hydrogen from git, has only ALSA midi. Do I miss something?

See also: https://github.com/hydrogen-music/hydrogen/issues/799

elpescado commented 4 years ago

Do you have JACK libraries installed? Do you see JACK support enabled in cmake output, for example:

Supported audio interfaces
--------------------------
* ALSA                         : + used  ( /usr/lib/x86_64-linux-gnu/libasound.so )
* OSS                          : - not found and not desired
* JACK                         : + used  ( /usr/lib/x86_64-linux-gnu/libjack.so )
grammoboy2 commented 4 years ago

I thought Hydrogen was using ALSA MIDI, not the newer JACK MIDI. Will check later.

elpescado commented 4 years ago

Hydrogen currently supports 4 MIDI drivers: JACK, ALSA, CoreMIDI (Mac only) and PortMIDI. All are optional, Hydrogen might be built with or without any of them, so availability of different MIDI drivers depends on how Hydrogen was built.