Midi note timing not on the grid :( Why would this be? Have seen other threads, but still confused

Hello all.

I have been exploring making midi plugins recently and I have been making a sequencer. Right now I am working with just a single 16 step pattern (sixteenth notes). Now I have seen things on the forum about rounding issues etc, so I am just trying to keep it simple at the moment.

The sample Rate is 48000.
Tempo is 120.

By my calculation this should mean that a quarter note equals 24000 samples. So divide that by 4, and we get 6000 samples for a 16th note. Is this correct? If that is indeed the case which I believe it to be. Then these sample positions should be bang on the grid, but they are not. If I record the midi events and look at what was recorded this happens:-

and it gets worse:-

Until here where something very odd happens and a gap forms:-

As I am typing this and thinking about it, the way I see it is that, perhaps because of the way how digital audio works, and sample positions that are being iterated over when processing might not perfectly line up with timing in the DAW, maybe that is introducing some kind of rounding issue? One thing is certain though, the sample positions have definitely been calculated correctly if what I said previously is correct. I am adding a note off event 1 sample behind the start of the next note on event:-

ScreenShot_20220917184717

What am missing?

Any help would be very much appreciated. Thank you in advance. :slight_smile:

EDIT: I will just add that I had no idea that this was even happening until I tested this with the Phoscyon VST. For people who are not familiar with that plugin and how it works, it is a 303 emulation. So if you have notes that run into the next note it should create a slide. In other words, in this pattern, every note should slide into each other and so it should fade away as it plays. But I could hear a new note start without sliding, and so I recorded the output and noticed this was happening.

This is even weirder… I just used a midi logger and apparently all the samples were correct to what my debug output said:-

ScreenShot_20220917194415

But then this begs the question as to why this does not line up correctly in the DAW (Reaper)?

I did try another sequencer plugin called StepicVST, and that worked correctly. So it doesn’t seem to be a Reaper issue.

Just to rule out the obvious, how are you adding the midi note events to the buffer?
Are you setting the correct sample position or just at the block beginning?
And are you starting to count at record start or are you evaluating the PositionInfo from AudioPlayHead?

Yes I am using PositionInfo from AudioPlayHead and yes I believe that I am setting the correct sample position.

What I am doing is grabbing the playhead position at the start of the block, then I am using that + the sample idx in a loop of the buffer size to calculate the currentSample. Then I am using currentSample to check against events in a list for events that have that particular sampleIdx as their start or end note values, and if so I am adding the events to the processed buffer. I am not saying this is a particularly optimal way of doing it, because it if there was lots of events it would be iterating a lot but right now I am not particularly concerned about that. Just trying to get something workable to understand things better then I will look into rewriting things better.

The playhead position is set in the plugin processor process method, but I abstracted the per sample loop into a class called Midi_Processor which has a process method of its own. This is then called from the end of the plugin processors process method. This code below is from the Midi_Processor::process method:-

if (p.hostInfo.isPlaying)
{
   // iterate through every sample in block.
   for (unsigned sample = 0; sample < p.getBlockSize(); sample++)
   {
      unsigned currentSample = p.playheadSamplePos + sample;
      std::vector<std::shared_ptr<MP_Event>> eventsList = events.get_events_at(currentSample);

      // for each event in event list.
      for (std::shared_ptr<MP_Event> e : eventsList)
      {
         if (e->initialised == true)
         {
            DBG("playing event at sample pos: " + juce::String(currentSample));
            juce::MidiMessage msg = e->msg;

            // keep track of active midi notes so that we can cut hanging notes upon playback stop...
            register_note_state(msg);
            pBuffer.addEvent(msg, currentSample);  // add event to the processed buffer.
         }
      }
   }
}
else if (!p.hostInfo.isPlaying)
{
   send_active_midi_note_offs_when_stopped();   // cut hanging notes.
}

mBuffer.swapWith(pBuffer);      // replace midi buffer from host.

When pressing play from the start of the timeline in Reaper, this is the output from the DBG print. These values look to be correct:-

ScreenShot_20220918000342

If there is anything else that people need, let me know.

Thanks again.

This is going to be a mostly useless post, but I’ll mention it in case it triggers any ideas for you;

This kind of reminds me of an issue I used to get in protools, where I would select a few bars of audio, and spam the duplicate macro. This would often result in a starting position of the looped clips not falling on the grid. Given enough duplicates, it could fall very off grid.

After looking into it, the cause was something like this: given the tempo of the session, a bar may not be perfectly divisible into samples. Protools handles this by rounding some bars to make it all work as a whole. So one bar may be a different number of samples to another bar, but on average, it works.

No idea about reaper, but is it possible you are calculating the correct sample positions, but reapers grid is doing some work to removing these rounding errors?

I think the error is, that you are feeding it the global sample position. In the addEvent call you want to set the local sample position inside the buffer instead.

pBuffer.addEvent (msg, sample);

N.B. there is probably a more efficient way to iterate through the events instead of iterating through samples…

Hope that helps

definitely in this case, for every sample it’s also iterating over the event list.

Another thing that I feel like it’s a “you should always do this” thing is when iterating over a list using for and you don’t intend to change anything in the list you should take a const reference to the objects in the list.

Not sure how your events list is set up, but the way I’ve tackled this kind of thing in the past is you iterate over the events list once, if an event falls within the range of the block being processed then you can then calculate the offset into the block and add the MIDI events accordingly.

1 Like

Yes I know that. Haha. As I say, it is rough and dirty and that isn’t really a problem at this point. I just want to solve the immediate problem. I have cannibalised my code to make it as basic as possible. There are several ways to optimise it, but it just doesn’t concern me since I am only using 16 steps at this point. Still the problem is the timing issue.

Cheers though.

EDIT: My bad I see that you were replying to Daniel. Lol! But yes… I concur… terrible solution, rough and ready.

1 Like

Ah interesting. I wonder how StepicVST got around this. Ummm.
EDIT: Never mind. Daniel spotted the issue. Problem solved. Thank you for trying to offer some food for thought though. :slight_smile:

Ah… interesting. I will try switching it now. Thanks.

EDIT: This was indeed correct. And of course, now that I have come back to it fresher and read your explanation I can see why that would be the case. In any case, much appreciated. I can now continue making progress. Excellent. Thank you very much Daniel. :grinning:

As I said, of course there are better ways to optimise it but with a simple 16 step sequence, it might as well just be that simple since it isn’t going to hit very hard into CPU. See I did some GUI stuff, and then I started writing the Midi_Processor class, but was testing as I went so I just banged that simple solution together for that since I’d been coding at the PC all of the day already and was starting to get a bit fatigued. It is a terrible way to do it to be honest. Absolutely freaking terrible. Ahahaha. It is a zero-brain solution. It will cause issues later on, but by then I will have replaced that stuff.

Disclaimer: Nobody should copy that event list iteration approach if they are reading this thread and naively think that aspect of it is a good one.

Further Edit: The fix also meant that I can now stop subtracting 1 from the end note position. As it now works as it should. :). So also if people read this, don’t do that either. It was a biproduct of the original issue that has now been solved.

1 Like

Glad that it’s working now!

1 Like

It sure is :smiley:, but Just to elaborate on this issue a little more:-

it seems that I misunderstood the sample number parameter of the addEvent method. I was thinking this would be a global sample position, but in actual fact, am I right in saying that a sample number passed in here cannot be larger than the current block size (or as you said the local sample number)? So for example, if I passed in a value of 96000 which should be in the second bar at this block size and tempo, it would not work that way. Which is where my problems were arising.

That is correct.

You can calculate the sample number quite easily as:

pBuffer.addEvent (event->msg, event->pos - p.playheadSamplePos);

So your loop would boil down to:

for (std::shared_ptr<MP_Event> event : eventsList)
{
    const auto localPos = event->pos - p.playheadSamplePos;
    if (juce::isPositiveAndBelow (localPos, buffer.getNumSamples())
        pBuffer.addEvent (event->msg, localPos);
}

No need for sample counting :wink:

1 Like

Haha I already did this pretty much. Slightly different because I have a method for getting only the relevant events for that block out of the larger event list, but same approach.

EDIT: I pretty much implemented an amalgamation of what you wrote there + @asimilon’s approach to checking for events that fall into the range of the block. Thereby cutting down the iterations per loop of all the events in the main event list. This could go further too if necessary for really really long sequences of events. For example, breaking it up into smaller chunks etc. But yes… It is good to know that I am on the right track now. :smiley:.

1 Like