search

surge-synthesizer / surge

Synthesizer plug-in (previously released as Vember Audio Surge)
https://surge-synthesizer.github.io/
GNU General Public License v3.0
3.15k stars 400 forks source link

MIDI Learn Causing Host UI Performance Problem #3553

Open williamfields opened 3 years ago

williamfields commented 3 years ago

Bug Description: REAPER UI becomes laggy and less responsive when: (1) There are multiple instances of Surge running. (2) Every instance has at least one parameter controlled via REAPER's "MIDI Learn" functionality. (3) MIDI controller data is sent to REAPER.

Surge Version Surge Synthesizer Plugin Host: REAPER System: Intel CPU, Windows VST3, 64-bit Build Info: Built on 2020-12-29 at 00:27:24, using pipeline host 'fv-az80-319' Version: 1.8.nightly.c70d689

Reproduction Steps: Steps to reproduce the behavior:

  1. Add one instance of Surge to a track in REAPER.
  2. Move a control in Surge.
  3. Go to: Param > Learn
  4. Assign a MIDI controller and click OK.
  5. Copy/paste to duplicate this instance of Surge, so there are 6 instances (or more).
  6. Repeatedly send a bunch of MIDI controller data to REAPER, which includes the MIDI controller assigned above.
  7. Watch the Time display on the transport. It will freeze when the MIDI controller data is sent.

Expected Behavior: Host UI should not become laggy when using MIDI Learn. (This does not happen with Vital VST3.)

Screenshots:

Here you can see the MIDI data in the Chrome console on the right. You can also see the REAPER transport time freezing whenever the MIDI data is sent.

https://user-images.githubusercontent.com/9380730/103412876-4a852b00-4b45-11eb-8101-3d0c94ce46bd.mp4

Computer Information (please complete the following information):

Additional Info: Strangely, this also happens if you do the MIDI Learn on a different plugin on the same track! So, it seems that whenever MIDI is sent, if a track has a plugin with MIDI Learn enabled, REAPER is sending some kind of message to all of the plugins on that track saying "something changed on this track! update your shit!". And maybe this message is causing the problem in Surge?

williamfields commented 3 years ago

Two more points:

baconpaul commented 3 years ago

Quick question: if you run with the editor windows closed does the problem resolve itself?

That will help me figure out where to debug.

williamfields commented 3 years ago

Quick question: if you run with the editor windows closed does the problem resolve itself?

No. It is independent of whether or not the Surge UI is visible.

baconpaul commented 3 years ago

oh well goodness then it is definitely not what i thought it was

last time you attached a reaper file which was setup to show the problem. That was super duper useful. Do't suppose you could do the same again? I don't really us reaper (I have it and use it for vst3 testing but haven't used it to make music - use LPX for that)

Thanks

williamfields commented 3 years ago

See attached. Cutoff of filter 2 is mapped to MIDI channel 15 CC 100.

ui glitch test3.zip

The tricky part is that, to reproduce the problem, you somehow need to generate a bunch of MIDI controller data to send to REAPER repeatedly.

Another interesting fact: The problem still occurs when all instances of Surge are set to "bypass". The problem does NOT occur when all instances of Surge are set to "offline". (Some background on the difference: https://forums.cockos.com/showpost.php?p=552642&postcount=2)

It's not a big deal from my perspective, but I wanted to report this because it may be symptomatic of some underlying inefficiency.

baconpaul commented 3 years ago

Not to be naive but would wiggling the knob on my controller mapped to 15/100 work? And is there an easy way to map to 0/40 or some such?

I wonder what it is - will be interesting to find out

baconpaul commented 3 years ago

Anyway I’ll peek and if it is something obvious will push a fix and if it is non obvious may wait until after we ship 18 in 10 days or so

williamfields commented 3 years ago

To reproduce the problem, it seems to require more controller data being sent all at once.

I'm not sure why the rest of the controller data is not ignored. Maybe by doing one "MIDI Learn" mapping, it flips a flag somewhere that means "pay attention to incoming MIDI controllers", and after that, all incoming controllers are processed/evaluated?

Thanks for your support as always!

mkruselj commented 3 years ago

Maybe by doing one "MIDI Learn" mapping, it flips a flag somewhere that means "pay attention to incoming MIDI controllers", and after that, all incoming controllers are processed/evaluated?

Nope, it's not like that, last time I checked out that part of the code.

baconpaul commented 3 years ago

You are doing a learn in surge or a learn in reaper?

mkruselj commented 3 years ago

Reaper's MIDI learn was mentioned. To do that, you use the mighty VST3 right-click context menu that has the option to MIDI learn the current parameter 🙂

baconpaul commented 3 years ago

I don't think this will be a 18 fix but I'm tagging it 18 so I can be sure it's not a one-liner before I push it into 19

mkruselj commented 3 years ago

Moving this one to 1.9 milestone.

williamfields commented 3 years ago

I discovered that the problem only occurs if there is a large number of MIDI controller messages (like 250) coming in at the same instant. (Note: Each instance of Surge is only mapped to respond to 3 of them.)

If I smear the timing of the controller messages by randomly delaying each controller message between 0 and 1000 milliseconds, then the UI halting problem mostly goes away.

I am trying to figure out a way to make this reproduceable for you. Maybe I could code up a little MIDI-generator utility?

By the way, I know this is a very unusual use case for Surge, and I totally understand if you decide to ignore this entirely.

Good luck with the launch of 1.8!

baconpaul commented 3 years ago

Definitely not ignoring it entirely but that also explains why we couldn’t see it. If they all come in at once I wonder what we are doing wrong! I can simulate that But we are thinking of maybe for 19 changing quite a bit about how our plugin layer works so I’m going to pause on this issue for the next tiny bit. Lets keep it open though.

williamfields commented 3 years ago

This was fixed by something in 1.9. So, this can be closed.

baconpaul commented 3 years ago

Great! I wonder what did it. Thanks for checking!

williamfields commented 10 months ago

@baconpaul: I just noticed that this same issue is affecting the Clap version of Surge XT 1.3. (The VST3 version is OK.)

Too bad we never figured out what caused it the first time.

Not sure if it is worth opening another issue. I'll probably just switch back to the VST3 version. (Without program change support, sadly. 😢)

baconpaul commented 10 months ago

Lemme peek with the other 1.3.1 issues but maybe after Christmas for those

baconpaul commented 9 months ago

OK looking at the code, i can definitely see why sending many many identical ccs at the same sample would back us up. I just need to figure out how to test it. I'm also not sure why the VST3 would act any differently than the CLAP since the code path is very similar.

Any way you happen to have a file kicking around which has the deathly-midi in it already? Something I can open in reaper and press play and see the problem would be super duper useful.

The thing is: When we get a midi cc we have to iterate over all params to find the mapping location. But we do that on the audio thread. However doing that forces refreshes which are pretty substantial.

I see some places I could optimize but really having something I can run to show the problem would help the most.

mkruselj commented 9 months ago

Are we not doing any sort of same value filtering? I reckon that should help some, too...

baconpaul commented 9 months ago

we do in one spot yes. basically if in a single block you update a param n times it only redraws it once.

but a stress case in reaper would be great. something is making the ui thread hang up and i can't see what it would be.

williamfields commented 9 months ago

I could probably hack together some JavaScript that generates a bunch of controller data. I will report back later when I have some time to work on this.

On Mon, Jan 22, 2024 at 8:47 AM Paul @.***> wrote:

we do in one spot yes. basically if in a single block you update a param n times it only redraws it once.

but a stress case in reaper would be great. something is making the ui thread hang up and i can't see what it would be.

— Reply to this email directly, view it on GitHub https://github.com/surge-synthesizer/surge/issues/3553#issuecomment-1904039962, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHSG6R4BYVFHINFDRIIQ73YPZUXPAVCNFSM4VPODQV2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJQGQYDGOJZGYZA . You are receiving this because you authored the thread.Message ID: @.***>

baconpaul commented 9 months ago

Can you give me an idea of order of magnitude of data you are sending? I have some midi generators kicking around as well.

williamfields commented 9 months ago

Maybe a couple hundred? I mentioned 250 in this comment: https://github.com/surge-synthesizer/surge/issues/3553#issuecomment-761180646

Andreya-Autumn commented 9 months ago

We'll release 1.3.1 today and this has not been addressed. So I'm bouncing this out of the milestone.