timing accuracy of note events..
Closed this issue · 19 comments
It seems that notes don't line up quite right if you send the same MIDI messages to a Cabbage synth alongside a native synth within a DAW. There are timing issues that seem to get worse the longer the performance continues for. I've checked the setLatencySamples() calls and they all seem correct. The timing of Cabbage notes are always too soon.
@andy-fillebrown , @awelex, it turns out that Cabbage has never been sample accurate in terms of MIDI notes. No one really noticed because of the small block sizes hosts can deliver these days. But if you really zoomed into the resulting waveforms you could see something was off. I just wanted to run some changes by you. Instead of doing this in the CsoundProcessor class:
midiBuffer.addEvents(midiMessages, 0, numSamples, 0);
we should be adding MIDI events only when the time is right. So I add this to the processBlock() method:
for (int i = 0; i < numSamples; i++, ++csndIndex)
{
while(iter.getNextEvent (message, samplePos))
{
//if current sample position matches time code for MIDI event, add it to buffer...
if(samplePos == i)
midiBuffer.addEvent(message, samplePos);
}
//reset the iterator each time, so that we can step through the events again to see if they should be added
iter.setNextSamplePosition(0);
It seems to give sample accurate timing, but I'm not sure about resetting the iterator on each sample. Does this look reasonable to you guys, or a potential performance bottleneck?
I pulled commit 6fe33e8 to test the performance and it's much degraded. I don't know enough of the Juce internals to know what's going on but if the MIDI buffer it's returning on each block contains a decent number of MIDI messages then iterating over it on each sample is going to be very expensive. Have you tried using the MidiBuffer::addEvent function's 3rd argument sampleDeltaToAdd to offset the MIDI messages to the correct sample?
Also note the change to CabbagePluginProcess.cpp in that same commit causes invalid memory access crashes for me in the Cabbage IDE and Reaper on Windows.
Interesting! I'm swamped with work the next couple of days but will take a look after.
Thanks for taking a look guys. @andy-fillebrown , I was afraid the performance might be affected. Adding time values to the events makes no difference I'm afraid. The problem is that Csound's just plays whatever is in the midi buffer. It doesn't care for timestamp messages. As soon a MIDI event is added to the buffer, Csound will play it. Only by adding the events at the correct sample position can we ensure the timing is correct. Or at least it appears this way. I'm open to suggestions. Maybe we can add a sample accurate identifier for those who really need it?
Ok, I'll look into how Juce handles MIDI messages and come up with a way to insert the messages at the correct sample that's less expensive. It shouldn't be too difficult once I understand what exactly is in Juce's MIDI buffer.
I just did a simple comparison there between the old implementation and the new one, I don't see much of a performance hit. Check the following screenshots:
If one is dealing with hundreds of MIDI notes a second this will get hairy. I thought that I could remove each event as it is output, but it seems that you can't remove events from the MIDIIterator.
I'm starting to think an identifier might be the way to go. If people are using block sizes of 64 samples, the maximum drift is about a millisecond. AFAIK it's not even possible to generate notes that are shorter than this using the current MIDI specs. Of course, the best would be to have sample-accurate timing at no extra cost. I'm curious to see what you come up with!
Interesting. I rebooted my system and the performance issue is gone but the audio output is scratchy and distorted. I'm still getting memory heap corruption crashes from the latest change, too, even after I revert the change to CabbagePluginProcessor.cpp. It must be the MIDI buffer changes causing the memory issue ...which is probably what caused my system to start grinding to a halt ...which is probably what made me see performance issues in Reaper yesterday. My first guess is this change is causing some kind of threading issue in Juce on Windows, but again, I don't really know how Juce works under the hood so it's hard for me to know exactly what's happening.
I think I see the problem now. It's the change from if (csndIndex == csdKsmps) to if (csndIndex > csdKsmps) that's causing my issues. Putting it back to == fixes the problems on my machine and I don't see any performance issues or memory crashes now.
Great Andy. I'll sort that out. Thanks for taking the time to look.
I've been thinking more about this and I wonder now whether Cabbage even needs to insert the MIDI messages in a sample-accurate way. Maybe you can just bulk insert the pending MIDI messages that will be performed on the next k-pass before calling performCsoundKsmps()? That would probably be less expensive.
So one could only achieve sample accurate MIDI notes by setting ksmps to 1?
Maybe I'm misunderstanding how MIDI works in Csound, but isn't that already the case? You mentioned that Csound just plays whatever is in its MIDI buffer at the beginning of each k-cycle and doesn't have a concept of more granular sample-level timestamps.
I don't think think the recent changes actually achieve sample accurate playback. I think it just moved everything back by 1 k-pass in Csound. Maybe I'm wrong about that?
@awelex, Yes, that's what my understanding is, too.
This might be opening a can of worms (and again with the caveat that I'm not fully sure how Csound handles RT MIDI), but I think if we really want to achieve sample-accurate MIDI timing it would require changing the MIDI code in csound to allow for more precise scheduling of MIDI events. Effectively, we're just passing on MIDI data to Csound, which becomes the sequencer in this scenario (just like a DAW). So it should be responsible to make sure events are scheduled and played back at the right time.
Although I'm realizing right now that it might not make a difference since Csound schedules instruments only at k-rate boundaries anyways (unless --sample-accurate is specified).
@andy-fillebrown My tests suggest it does 'somehow' offer sample accurate events. I've tested with Live and Reaper and in both cases my synth lines up perfectly with native synths in the host, no matter how crazy my MIDI sequence is. The latest code waits till the MIDI event is perfectly lined up with the index of the current sample, and only then sends it to the MIDI buffer Csound is reading. But as you both point out, this event will only take place at a k-boundary.
So you think it would be better to accumulate all incoming MIDI messages until a k boundary, and then feed them to Csound?
Does this mean Csound's MIDI buffer handling is separate from the calls to performCsoundKsmps()?
Meh, whatever. If it aint broke don't fix it :) The performance gain I'm talking about would be minimal anyway
Does this mean Csound's MIDI buffer handling is separate from the calls to performCsoundKsmps()?
To be honest, I'm not sure. But I do expect MIDI notes to only take place on k boundaries as you and @awelex point out. I should test the latest changes with very large ksmps values and see if I can break it again