Java - Audible Lag with Custom MIDI Sequencer using PPQ Ticks for Timing

Question

I have been trying to implement my own asynchronous MIDI sequencer in Java that sends ShortMessage's to VST's by processing lists of MidiEvent's. I need performance to be optimal so that there will be no audible delay when listening to the audio output.

The problem is that there definitely is audible lag as the ticks increment inaccurately (increments either too fast or too slow at times which messes up all MidiEvent's timing).

Here is the code of the sequencer below:

package com.dranithix.spectrum.vst;

import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import javax.sound.midi.MidiEvent;
import javax.sound.midi.ShortMessage;

import com.synthbot.audioplugin.vst.vst2.JVstHost2;

/**
 * 
 * @author Kenta Iwasaki
 * 
 */
public class VstSequencer implements Runnable {
    public static long BPM = 128L, PPQ = 4L;
    private long oneTick = (60000L / (BPM * PPQ)) * 1000000;

    private Map<MidiEvent, Long> currentEvents = new ConcurrentHashMap<MidiEvent, Long>();
    private long startTime = System.nanoTime(), elapsedTicks = 0;
    private JVstHost2 vst;

    public VstSequencer(JVstHost2 vst) {
        this.vst = vst;
    }

    @Override
    public void run() {
        while (true) {
            if (System.nanoTime() - startTime >= oneTick) {
                elapsedTicks++;
                startTime = System.nanoTime();
            }

            Iterator<MidiEvent> it = currentEvents.keySet().iterator();
            while (it.hasNext()) {
                MidiEvent currentEvent = it.next();
                long eventTime = currentEvent.getTick() - elapsedTicks;
                if (eventTime <= 0) {
                    vst.queueMidiMessage((ShortMessage) currentEvent
                            .getMessage());
                    it.remove();
                }
            }
        }
    }

    public void queueEvents(List<MidiEvent> events) {
        Map<MidiEvent, Long> add = new HashMap<MidiEvent, Long>();
        for (MidiEvent event : events) {
            event.setTick(event.getTick() + elapsedTicks);
            add.put(event, event.getTick());
        }
        currentEvents.putAll(add);
    }

    public void queueEvent(MidiEvent event) {
        event.setTick(event.getTick() + elapsedTicks);
        currentEvents.put(event, event.getTick());
    }
}

How can we improve the performance of this system? Can we ensure that there will be no audible delay for this sort of system (for example: a fixed timestep)?

Thanks in advance.

EDIT: Just to isolate causes of audible lag, I can confirm that there is no lag from the VST itself or from the framework sending the MIDI Messages to the VST. It has to do with the tick-based timing system presently used in the sequencer.

SOLVED: I fixed the issue by having the VST events become processed parallel to the event sequencer itself by including the VST event processing code in the same thread (they were originally on separate threads). To whomever out there reads this and has been scavenging around for sequencing MIDI events to JVstHost2 or any similar Java VST Host library, feel free to use parts of fixed code for your own projects as it has been very difficult for me to find proper VST sequencing online due to VST being a commercial format that is rarely ever touched with Java.

Solved Code:

package com.dranithix.spectrum.vst;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

import javax.sound.midi.MidiEvent;
import javax.sound.midi.ShortMessage;
import javax.sound.sampled.AudioFormat;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.DataLine;
import javax.sound.sampled.LineUnavailableException;
import javax.sound.sampled.SourceDataLine;

import com.synthbot.audioplugin.vst.vst2.JVstHost2;

/**
 * 
 * @author Kenta Iwasaki
 * 
 */
public class VstSequencer implements Runnable {
    private static final float ShortMaxValueAsFloat = (float) Short.MAX_VALUE;
    public static float BPM = 120f, PPQ = 2f;
    private static float oneTick = 60000f / (BPM * PPQ);

    private List<MidiEvent> currentEvents = new ArrayList<MidiEvent>();
    private long startTime = System.currentTimeMillis(), elapsedTicks = 0;
    private JVstHost2 vst;

    private final float[][] fInputs;
    private final float[][] fOutputs;
    private final byte[] bOutput;
    private int blockSize;
    private int numOutputs;
    private int numAudioOutputs;
    private AudioFormat audioFormat;
    private SourceDataLine sourceDataLine;

    public VstSequencer(JVstHost2 vst) {
        this.vst = vst;

        numOutputs = vst.numOutputs();
        numAudioOutputs = Math.min(2, numOutputs);

        blockSize = vst.getBlockSize();
        fInputs = new float[vst.numInputs()][blockSize];
        fOutputs = new float[numOutputs][blockSize];
        bOutput = new byte[numAudioOutputs * blockSize * 2];

        audioFormat = new AudioFormat((int) vst.getSampleRate(), 16,
                numAudioOutputs, true, false);
        DataLine.Info dataLineInfo = new DataLine.Info(SourceDataLine.class,
                audioFormat);

        sourceDataLine = null;
        try {
            sourceDataLine = (SourceDataLine) AudioSystem.getLine(dataLineInfo);
            sourceDataLine.open(audioFormat, bOutput.length);
            sourceDataLine.start();
        } catch (LineUnavailableException lue) {
            lue.printStackTrace(System.err);
            System.exit(1);
        }
    }

    @Override
    protected void finalize() throws Throwable {
        try {
            sourceDataLine.drain();
            sourceDataLine.close();
        } finally {
            super.finalize();
        }
    }

    private byte[] floatsToBytes(float[][] fData, byte[] bData) {
        int index = 0;
        for (int i = 0; i < blockSize; i++) {
            for (int j = 0; j < numAudioOutputs; j++) {
                short sval = (short) (fData[j][i] * ShortMaxValueAsFloat);
                bData[index++] = (byte) (sval & 0x00FF);
                bData[index++] = (byte) ((sval & 0xFF00) >> 8);
            }
        }
        return bData;
    }

    @Override
    public void run() {
        while (true) {
            if (Thread.interrupted()) {
                break;
            }
            if (System.currentTimeMillis() - startTime >= oneTick) {
                elapsedTicks++;
                startTime = System.currentTimeMillis();
            }
            vst.processReplacing(fInputs, fOutputs, blockSize);
            sourceDataLine.write(floatsToBytes(fOutputs, bOutput), 0,
                    bOutput.length);

            Iterator<MidiEvent> it = currentEvents.iterator();
            while (it.hasNext()) {
                MidiEvent currentEvent = it.next();
                long eventTime = currentEvent.getTick() - elapsedTicks;
                if (eventTime <= 0) {
                    vst.queueMidiMessage((ShortMessage) currentEvent
                            .getMessage());
                    it.remove();
                }
            }
        }

    }

    public void queueEvents(List<MidiEvent> events) {
        for (MidiEvent event : events) {
            event.setTick(event.getTick() + elapsedTicks);
        }
        currentEvents.addAll(events);
    }

    public void queueEvent(MidiEvent event) {
        event.setTick(event.getTick() + elapsedTicks);
        currentEvents.add(event);
    }
}

Answer 1

I suspect the problem is:

event.setTick(event.getTick() + elapsedTicks);
add.put(event, event.getTick());

The stream of events presumably already have time-stamps, so there is no need add elapsedTicks to them. This simply means they get later and later as time progresses.

There are several very obvious ways to improve the performance of the code above. Whether they are the cause of your problems is hard to say:

1: Don't busy-wait: The code above has no means of blocking until there's something to do (a ConcurrentHashMap doesn't provide blocking behaviour). Instead it loops continuously burning CPU cycles, even when there's nothing to do. This kind of behaviour is often penalised by operating system schedulers. Your thread can't schedule events when it's not running, and its current design encourages this.

2: The use of of a HashMap keyed with MIDIEvent for currentEvents is a poor choice and inefficient. You need to iterate the entire container to find events that need delivering to the VST. Furthermore, since there is no ordering guarantee, you potentially deliver events falling in the current tick out of order. Consider using a SortedMap where the key is the delivery time. Events are now ordered, with the soonest at the beginning of the structure. Delivering events is cheap.

A further potential problem lies with this line - it won't cause irregular timing, but I may mean that oneTick is wrong:

private long oneTick = (60000L / (BPM * PPQ)) * 1000000;

The division by BPM * PPQ causes a truncation. Perform the multiplication first.