Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SurgeXT plays notes stuttery vs. Surge #5830

Closed
Bleuzen opened this issue Jan 26, 2022 · 40 comments
Closed

SurgeXT plays notes stuttery vs. Surge #5830

Bleuzen opened this issue Jan 26, 2022 · 40 comments
Labels
Bug Report Item submitted using the Bug Report template Host Specific Issues related to specific host(s) or host features Linux Issues which only occur on Linux

Comments

@Bleuzen
Copy link

Bleuzen commented Jan 26, 2022

Bug Description:
I tried porting my patches from Surge to Surge XT and let them run with some midi notes in Ardour for testing. I noticed that while Surge plays my midi just fine and smooth, Surge XT sometimes sounds a little different and stutters.

Surge Version

  • Version: Surge XT 1.0.0.1edd725
  • Plugin Type: VST3
  • Bitness: 64-bit

Reproduction Steps:
Steps to reproduce the behavior:
1.) Load Surge and Surge XT and play the same midi notes

Expected Behavior:
With the same patch they should sound the same and Surge XT should play the notes as smooth as original Surge.

Video:
https://www.youtube.com/watch?v=BHz8VhpC7fM

Computer Information (please complete the following information):

  • OS: Arch Linux
  • Host: Ardour
  • Version: 6.9

Additional Information:
Happens with the initial patch and "Nastyfication". Haven't tested others yet, but all could be affected.

@Bleuzen Bleuzen added the Bug Report Item submitted using the Bug Report template label Jan 26, 2022
@mkruselj
Copy link
Collaborator

Audio example would be welcome.

Since this doesn't happen in any other DAW that I have over here, I would suggest this might be a VST3 issue in Ardour.

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

Audio example would be welcome.

samples.zip

Can already see when comparing them in Audacity that with Surge XT some notes trigger a little late:

image

image

Note transitions are... different. Both use the same patch and midi, but in some cases it looks like notes would have an extra attack / fade out time with Surge XT (red), where in other cases Surge and Surge XT sound the same (green).

I would suggest this might be a VST3 issue in Ardour

Can't test this as the LV2 version of Surge XT currently crashes Ardour. Meanwhile VST3 and LV2 of Surge work fine.

@baconpaul
Copy link
Collaborator

baconpaul commented Jan 27, 2022

This could occur if ardour sets the timestamps on vst3 midi incorrectly. In our old vst3 I don’t think we did a very good job of interleaving midi with processing based on that

I’m curious if you set your ardour block size to 64 or 32 if it still happens

@mkruselj mkruselj added Host Specific Issues related to specific host(s) or host features Linux Issues which only occur on Linux labels Jan 27, 2022
@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

The block size really makes a difference here. Smaller ones make the problem less noticeable while higher ones make it more problematic.
So I guess your assumption is true and reported an Ardour bug now:
https://tracker.ardour.org/view.php?id=8865

@mkruselj
Copy link
Collaborator

We've also heard this might be fixed in Ardour 7, since they're overhauling their MIDI handling.

@x42
Copy link

x42 commented Jan 27, 2022

they're overhauling their MIDI handling.

Not MIDI per se. From a Plugin's POV nothing changes. For VST3 Plugins MIDI events are still timestamped using samples as unit. The difference in in Ardour 7 is editing in the GUI. Previously rounding errors between music-time (bar|beat) and wallclock-time (samples) caused issues.

BTW, I cannot reproduce this with other VST3 plugins. e.g. u-he Diva aligns perfectly regardless of buffer-size.

With a short attack (and at max zoom level (1 px = 1 sample):
image

@x42
Copy link

x42 commented Jan 27, 2022

PS. It may still be Ardour's fault or a combination of Surge/JUCE/Ardour.
The fact that it's buffersize dependent however makes me think it's more likely in the plugin.

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

@x42 as shown in my screenshots above, it does not happen with all notes. Some play fine in the right time. It seems mainly between some note transitions, when one note goes directly over into another.

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

I was able to test the LV2 now. Because, while Surge XT LV2 crashes Ardour on two of my machines, it works on a third one xD
@baconpaul Found out that Surge XT as LV2 also has this problem, so it is not only VST3, letting me think the problem could be on Plugins side (I somehow trust the LV2 integration in Ardour since it is in there for a while and never saw such problems).

@baconpaul
Copy link
Collaborator

hmm lemme try here in logic and see.
i am pretty sure i got it right. but who knows!

@baconpaul
Copy link
Collaborator

So here's the logic from surge

    auto midiIt = midiMessages.findNextSamplePosition(0);
    int nextMidi = -1;
    if (midiIt != midiMessages.cend())
    {
        nextMidi = (*midiIt).samplePosition;
    }

    for (int i = 0; i < buffer.getNumSamples(); i++)
    {
        if (i == nextMidi)
        {
            applyMidi(*midiIt);
            midiIt++;
            if (midiIt == midiMessages.cend())
            {
                nextMidi = -1;
            }
            else
            {
                nextMidi = (*midiIt).samplePosition;
            }
        }

this will land midi items on exactly the right spot IF their sample position starts at 0 relative to the buffer.

If the sample has an offset, then they end up all the way at the end.

Surge 1.9 crammed them at the start of the buffer so they will be 'early' but didn't try to land them at the appropriate inter-block

So I guess my question is: does ardour label its midi from block-relative samples.

I checked in reaper and it does (with block size 512 I get the first midi location at various values between 0 and 512).

If Ardour is providing an offset, how do I get it?

@Bleuzen do you have an ability to build and test in ardour? If so I can share with you the patch I added to debug in reaper.

@x42
Copy link

x42 commented Jan 27, 2022

MIDI even-timestamps are relative to the current plugin process cycle. The Sample positions perfectly match the audio-buffers provided to the plugin's process function.

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

@Bleuzen do you have an ability to build and test in ardour?

Yes I can do that

@baconpaul
Copy link
Collaborator

x.patch.zip

Try that patch and see what stdout you get while playing with ardour

with reaper i never get the overflow and get sample accurate application

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

x.patch.zip

Try that patch and see what stdout you get while playing with ardour

/surge/src/surge-xt/SurgeSynthProcessor.cpp:258:37: Fehler: expected »;« before »for«
  258 |     midiMessages.getFirstEventTime()
      |                                     ^
      |                                     ;
  259 | 
  260 |         for (int i = 0; i < buffer.getNumSamples(); i++)
      |         ~~~   

Well xD
Whatever, got it built now with an extra semicolon.
Output from Ardour:

NM is 0 528
Applying midi event at sample 0 of  528
NM is 880 2048
Applying midi event at sample 880 of  2048
NM is 880 2048
Midi Overflow Event 880 in  2048
NM is 240 2048
Applying midi event at sample 240 of  2048
NM is 240 2048
Midi Overflow Event 240 in  2048
NM is 1648 2048
Applying midi event at sample 1648 of  2048
NM is 1648 2048
Midi Overflow Event 1648 in  2048
NM is 1008 2048
Applying midi event at sample 1008 of  2048
NM is 1008 2048
Midi Overflow Event 1008 in  2048
NM is 368 2048
Applying midi event at sample 368 of  2048
NM is 368 2048
Midi Overflow Event 368 in  2048
NM is 1776 2048
Applying midi event at sample 1776 of  2048
NM is 1776 2048
Midi Overflow Event 1776 in  2048
NM is 1136 2048
Applying midi event at sample 1136 of  2048
NM is 1136 2048
Midi Overflow Event 1136 in  2048
NM is 496 2048
Applying midi event at sample 496 of  2048
NM is 496 2048
Midi Overflow Event 496 in  2048
NM is 1904 2048
Applying midi event at sample 1904 of  2048
NM is 1904 2048
Midi Overflow Event 1904 in  2048
NM is 1264 2048
Applying midi event at sample 1264 of  2048
NM is 1264 2048
Midi Overflow Event 1264 in  2048
NM is 624 2048
Applying midi event at sample 624 of  2048
NM is 624 2048
Midi Overflow Event 624 in  2048
NM is 2032 2048
Applying midi event at sample 2032 of  2048
NM is 2032 2048
Midi Overflow Event 2032 in  2048
NM is 1392 2048
Applying midi event at sample 1392 of  2048
NM is 1392 2048
Midi Overflow Event 1392 in  2048
NM is 752 2048
Applying midi event at sample 752 of  2048
NM is 752 2048
Midi Overflow Event 752 in  2048
NM is 112 2048
Applying midi event at sample 112 of  2048
NM is 112 2048
Midi Overflow Event 112 in  2048
NM is 1520 2048
Applying midi event at sample 1520 of  2048
NM is 1520 2048
Midi Overflow Event 1520 in  2048
NM is 880 2048
Applying midi event at sample 880 of  2048
NM is 880 2048
Midi Overflow Event 880 in  2048
NM is 240 2048
Applying midi event at sample 240 of  2048
NM is 240 2048
Midi Overflow Event 240 in  2048
NM is 1648 2048
Applying midi event at sample 1648 of  2048
NM is 1648 2048
Midi Overflow Event 1648 in  2048
NM is 1008 2048
Applying midi event at sample 1008 of  2048
NM is 1008 2048
Midi Overflow Event 1008 in  2048
NM is 0 2048
Applying midi event at sample 0 of  2048
NM is 2047 2048
Applying midi event at sample 2047 of  2048

@baconpaul
Copy link
Collaborator

huh ok.so then it's not midi sample offset. hmm.

i wonder what else could be mis-aligning buffers.

@baconpaul
Copy link
Collaborator

Screen Shot 2022-01-27 at 3 02 26 PM

And just to be sure I tried the vst3 in reaper using @x42 's test on my mac and saw this exact alignment (which is within the 32 sample block size alignment).

Hmmm. @selenologist you are running a recent ardour right? Can you see anything like this if you happen to be online?

@x42
Copy link

x42 commented Jan 27, 2022

@Bleuzen Could you try https://github.com/x42/host-synth-test.lv2 as replacement synth in the same session?

That will produce a 1 sample spike on each MIDI note event:
image

Just to rule out somethin is wrong with the session itself.

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

Could you try https://github.com/x42/host-synth-test.lv2 as replacement synth in the same session?

@x42 looks fine to me:
image
image

Just to rule out somethin is wrong with the session itself.

That's not the case, can reproduce in any new session.

@baconpaul
Copy link
Collaborator

and if you just sub surge xt onto that with a sine blip (basically just take the sustain on the envelope down to zero) it misses?

why on earth would it miss in ardour for you and not in reaper for me?

@x42 have you run surge in ardour and seen this?

I guess I could give you another patch which makes surge run exactly like that blip plugin.

@x42
Copy link

x42 commented Jan 27, 2022

@x42 have you run surge in ardour and seen this?

Yes, I just tried, and yes it's offset:

image

It's an older version which I had installed: "Copy" does not work hence a screenshot:
image

@baconpaul
Copy link
Collaborator

right so that makes some sense. the old version didn't interleave into the block properly

the new version should not do that and should be much closer.

If we apply this patch to head the synth sends a blip downstream on note on and note off and in reaper it lines up perfectly. Wanna give that a try @Bleuzen ?

blip.patch.zip

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

and if you just sub surge xt onto that with a sine blip

tested with Surge and Surge XT. They both have a delay:
image

Only difference is (and that is why I never noticed with old Surge I guess):
Surge has somewhat constant and mostly smaller delays/offsets while with Surge XT they are more random in length:
image
with sometimes Surge XT even being faster than Surge:
image

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

If we apply this patch to head the synth sends a blip downstream on note on and note off and in reaper it lines up perfectly. Wanna give that a try @Bleuzen ?

@baconpaul the blip patched Surge XT does not have those delays:
image

@baconpaul
Copy link
Collaborator

wow.
ok.
let me think about that fact.

@baconpaul
Copy link
Collaborator

blipmix.patch.zip

@Bleuzen just one more if you have a chance and based on this let me think.

This one mixes the surge signal and the blip. the blip is sample accurate to the midi the surge signal should be delayed an amortized 16 samples with a min of 0 and max of 31. I am curious if that is what you see in ardour.

Thank you for these tests. I am not sure what is happening but each one is at least ruling stuff out.

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

blipmix.patch.zip

@Bleuzen just one more if you have a chance and based on this let me think.

This one mixes the surge signal and the blip.

@baconpaul
image

@baconpaul
Copy link
Collaborator

Right so that is what I would expect - our finest midi resolution is our block size which is 32 samples. Surge 19 just put everything at the top of the next block so should be more latent but will be differently renderer especially with large block sizes but I think xt is more correct

so I guess now I’m unsure what the problem is.

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

played a little with the notes because...

It seems mainly between some note transitions, when one note goes directly over into another.

with notes directly one after another:
image

but when shortening the notes before, for example note 1 and 2, look how this makes the offset for the next note smaller:
image

@baconpaul
Copy link
Collaborator

oh that's very interesting.
i see you also posted overflow events in the print but i'm not sure that patch was good
hold on. let me try one more patch.

@baconpaul
Copy link
Collaborator

oh i see one potential bug - that's very useful.
hold on

@baconpaul
Copy link
Collaborator

wonder.patch.zip

Does this patch improve the situation?

I think surge xt has a bug when two midi events occur with exactly the same sample which will push the items to the end of the block.

@baconpaul
Copy link
Collaborator

by the way if that does improve the situation, along with a few other bugs we found, we may include it in 1.0.1 release. But I'd make a nightly first.

@x42
Copy link

x42 commented Jan 27, 2022

two midi events occur with exactly the same sample

Aha! Yes that would happen with the MIDI data shown the the screenshot above after the first note.
Note off (x) | Note on (y) will have the same timestamp.

@Bleuzen
Copy link
Author

Bleuzen commented Jan 27, 2022

wonder.patch.zip

Does this patch improve the situation?

makes it much better.
Before:

image

After / with the latest patch:

image

Great, thanks :)

That said there are still little delays for each note:

image

Seems small, probably not audible in most cases.

@baconpaul
Copy link
Collaborator

Yeah that's the amortized 32 delay from the block size. No getting rid of that.

OK so cool this is a real honest to goodness bug

Let me patch this and then if you can confirm the code at head we can put together a 1.0.1

Thanks for your patience and sorry for blaming ardour :)

baconpaul added a commit to baconpaul/surge that referenced this issue Jan 27, 2022
If two midi events arrived with the exact sample sample
timestep, they would push to the end of the block along
with the rest of the midi in that block. Fix it with
an if/while switch on processing the midi list.

Addresses surge-synthesizer#5830
@x42
Copy link

x42 commented Jan 27, 2022

Thanks for your patience and sorry for blaming ardour :)

No worries.

Wow, from bug-report to solution in under 12 hours. Great work everyone!

baconpaul added a commit that referenced this issue Jan 27, 2022
If two midi events arrived with the exact sample sample
timestep, they would push to the end of the block along
with the rest of the midi in that block. Fix it with
an if/while switch on processing the midi list.

Addresses #5830
@baconpaul
Copy link
Collaborator

Ok @Bleuzen i just merged the fix to head

if you can pull main 6e7f2e0 and confirm it fixes for you will will roll that up with a few other post release fixes into a 1.0.1

thanks for your patience here

@Bleuzen
Copy link
Author

Bleuzen commented Jan 28, 2022

@baconpaul works from latest git main :)

@Bleuzen Bleuzen closed this as completed Jan 28, 2022
@baconpaul
Copy link
Collaborator

Awesome. Thank you.

We will ship that with a few there bug fixes as a 1.0.1 release this weekend sometime (maybe Monday at the outside)

Appreciate the help here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Report Item submitted using the Bug Report template Host Specific Issues related to specific host(s) or host features Linux Issues which only occur on Linux
Projects
None yet
Development

No branches or pull requests

4 participants