Surge: LV2 patch corruption when re-opening GUI

[unfa]

I've been trying to get into Surge for a good while now.

My platform of choice is Linux, and until recently that's the only OS group I've been running Surge on.
However recently I had something similar happen to me under Windows 10.

If I close the Surge window (GUI) and later open it up again in my DAW - at that moment the patch gets corrupted, usually in very specific ways.

Most often the oscillators all get reset to the default Sawtooth. Often LFO settings are corrupted too. Sometimes the patch is completely reset to Init state.

If I store my patch first before closing the GUI I can just reload it immediately after it gets corrupted, but it'll break again after I reopen the Surge window for another time.

As you can expect this is a big problem and prevents me from being able to do anything more than fiddling with Surge, and it's a big loss for me, as I see a great potential for amazing sounds in it.

I've had this issue for about a year now (basically since the first time I tried Surge on Linux).
This happens to me no matter the host: Ardour, Carla or Zrythm.

However this week I had a similar problem happened on a Windows 10 system where I so far was able to use Sure with success with Ardour 5.12.

one of my patches got reset to Init when loading a project created a few days earlier. I have saved and loaded it before without problems, and other patches were fine. I don't know if this could be related.

Please let me know what can I do to help you fix this problem. I know I'm not the only Linux user suffering from it.

[baconpaul]

Hi!

OK you are the first report we have to it. Can I ask a couple of rudimentary questions?

Are you using the VST2, VST3 or LV2?
Are you using Surge 1.6.5?
Do you have the ability to build surge from source if we have a debugging build?
Do you have a DAW file for some easily available DAW which shows this?

We have recently been adding automated tests and stream/unstream invariance is one of the things we partly test. But some more details could help us loo for sure.

Thanks

[baconpaul]

Oh also: I’ve used surge for a year now (mac AU and VST3) as havre many of our users and I haven’t seen this behavior once. So step one is pinning down what code path you are on which we are not so we can look there,hence my block and tackle questions above.

Thanks!

[baconpaul]

One last thought: The activity you describe seems like some sort of race condition between the DAW restoring the state and the internal state of the synth. In the VST2/3/AU implementations we don’t have this problem because the synth is the sole master of the values, but I understand in the LV2 there is a question about whether the patch is mastered by the LV2 or by the engine.

Honestly I don’t understand that statement all that well. @jpcima and @falkTX wrote the LV2 implementation. But the behavior you describe sounds like some or all of the values are being reset to an init value, rather than the value the synth has.

So I’m going to make a wild guess that

1. You are running the LV2 (something, alas I have literally never done) and
2. The bug is some form of ordering-of-restore-vs-default in the LV2 parameter space (code, alas, I have literally never edited)

So if you are using the LV2 confirming that would be good.

I’ll note the VST3 is also free software and is way better tested and exercised, although I understand Linux DAW support for it is, charitably, spotty still.

Best!

[sub26nico]

I encounter the same bug with the lv2 version (v 1.6.4 here) in Ardour. I've already talked about it with @jpcima on IRC, it's a known bug. No problem with the vst3 version in Reaper (native linux). Presets are recalled nicely. Unfortunately, vst3's support is limited or doesn't exist in floss daws, so, a fix for lv2 would be appreciated.

[unfa]

I have definitely experienced this using LV2 in Ardour, Carla and Zrythm. I am usually building the latest git version using AUR (I'm working on Manjaro Linux).

I think I had the same issue with VST2, but I need to check it again before I can be sure. If you have a debug build I could use that to provide more information.

@baconpaul, thank you for your help!

[sub26nico]

FYI, lv2 works fine in Qtractor, at least, patch's recalls works fine.

[baconpaul]

OK a quick poll over in the facebook group seems to show that folks in VST2/3/AU land dont’ see this so I am going to go with my hunch, which apparently @jpcima has confirmed, that this is an order of operations in the LV2 default restoring world.

@sub26nico did @jpcima say if it was a known bug in LV2, SurgeLV2, or the DAW?

I’ve never run Ardour or Zrythm and have only used Carla as the standalone linux vst2 launcher not as a daw, and I didn’t write any of the LV2 code, so it would be super hard for me to debug this. If you are chatting with @jpcima in IRC pointing him at this issue would be appreciated and maybe we can get a plan of attack?

[sub26nico]

@sub26nico did @jpcima say if it was a known bug in LV2, SurgeLV2, or the DAW?

an Surge lv2 version iirc

If you are chatting with @jpcima in IRC pointing him at this issue would be appreciated and maybe we can get a plan of attack?

Already done, I'm waiting his answer

[baconpaul]

Alright thanks both @unfa and @sub26nico !!

[unfa]

Thank you!
I'd be perfectly happy with LV2 (I actually prefer that plugin format as a fully open and rooted in FOSS culture).
So if this can be fixed in LV2 I'm going to be very happy, and I will most likely start making videos about Surge.

[baconpaul]

Yeah the behavior you describe is exactly consistent with surge getting someone sending it default values onto the engine from the outside

Assume I have never used any of these daws. How do you do even basic stuff? Do you have a video on that somewhere in case I do end up looking

But full disclosure I am heads down on a skinning engine for the ui now so would be way way better if the original authors could look.

[unfa]

Ok, turns out I didn't probably ever use VST3 on Linux, because the AUR PKBUILD I used to install installs LV2 and VST3, adn Ardour probably didn't ever load VST3.

[jpcima]

Hello,
The state restoring trouble is LV2-only, I think.

It arises from some particular restrictions, which are part of LV2 specifications, precisely:

1. the DSP part cannot write parameters to the host ; they are read-only from DSP, however it's UI who can set them
2. the DSP has to load and save the parameters, and then the rest of state, as separate data

This did not fit too well with the core Surge, which I think modeled approximately after VST.
As I remember, for instance, LV2 Surge was disallowed to report a parameter to host when it is manipulated by MIDI, because of point 1. above.

(I recall this from memory, forgive if I'm wrong.)

About the plan on how to fix it:

1. when a patch in selected from the editor, all the setParameters which are part of the patch must occur from GUI side (even if redundant).

This will let the host be noticed of the parameter changes, which lets them save and restore correctly. Otherwise, the parameters known to the host will remain these before the patch change, which means it would save the values which are out of date.

1. when DSP side reloads the state blob, do not set the parameters from it.
   The LV2 host will send recorded parameters one-by-one.

On this, I don't think there is a defined order in which the parameters and the blob will arrive to plugin, which makes it perhaps more complicated.

I can dedicate some time to fix this, also I'll take some suggestions if you have some. Thanks :)

[baconpaul]

Ok I merged your change but let me look at your comment more carefully and respond

[baconpaul]

OK so I read your note. I am surprised any VI can meet those constraints! So let me step back for one second.

I think you all have a version of dexed running as an LV2 yeah? How did you deal with the engine loading sysex? The cartridge manager seems to do exactly the same thing as the surge patch manager, and from being in the code, seems to do it in a very similar way. How did that work?

As to surge's architecture it works like this

• each synth has an instance of a storage which has an instance of a patch
• The patch is the definitive store of values for parameters, modulations, and so on
• Some of the patch is exposed by the various hosts as VST parameters, but not all
• The patch stores itself with a load and save method
• The UI is a stateless window on to that patch where UI actions modify the patch
• The DAW is, similarly, an editor of the patch through the DAW automation APIs
• DAW save and restore saves the patch then saves a few bits of non-patch state (like MPE Pitch - - Bend settings and alternate tuning settings, which either can be in the patch or out of the patch)

So that means the job of the DAW is as follows

1. When constructing a surge anew, scan the surge instance you created for the value of all parameters using the get_f01 mechanism. You can see this in the VST2/3/AU code pretty clearly.

2. When constructing a surge from a saved state, do #1 then restore the saved blob which the prior surge gave you using the loadRaw and loadStateFromDAW methods on the synth

3. At a later date, if you open the UI, the UI will build itself from the surge patch. Opening the UI should do nothing with the parameters

4. Persisting any state other than the blob sent from saveRaw will give you an inconsistency. Initializing the parameters by any mechanism other than either initialize / scan synth or initialize / load raw / scan synth will give you an inconsistency

I'm not sure what that means for LV2 but that's how surge works in VST2, VST3, AU, Headless.

If you want to use a subset of the synth there are other things you can do to manipulate the internal data structures directly (that's how rack and the FX works) but I strongly recommend against that for a full-synth build, since rack and FX don't have things like the modulation matrix.

Hope that helps!

[jpcima]

Thanks for writing this explanation.

Maybe I can quote LV2 State, which can explain some things better than I do.

From https://lv2plug.in/ns/ext/state/state.html :

This extension defines a simple mechanism which allows hosts to save and restore a plugin instance's state. The goal is for an instance's state to be completely described by port values (as with all LV2 plugins) and a simple dictionary.

With "port values" being synonym of parameters, and state "dictionary" being the raw file.
LV2 enforces by specification the separation of these as 2 distinct elements.

I think you all have a version of dexed running as an LV2 yeah? How did you deal with the engine loading sysex? The cartridge manager seems to do exactly the same thing as the surge patch manager, and from being in the code, seems to do it in a very similar way. How did that work?

In typical LV2, I think preset storage would be entirely managed in UI, it will only dispatch to DSP the parameters and state items which compose a chosen preset.
As I understand, LV2 favors program management on host side, and on plugin side is quite severely restricted.

On the PR which has been merged, it's a way to force Surge somewhat into operating in similar logic.

When constructing a surge from a saved state, do #1 then restore the saved blob which the prior surge gave you using the loadRaw and loadStateFromDAW methods on the synth

The steps are pretty much that which are followed, with this exception being mandated in LV2: parameters will be always persisted separate from the Raw (duplicated), and resent on next load.

I remember falktx has once mentioned me an edge case, where the two will not be necessarily identical, but the memory of this escapes me at the moment.

For now, I have not certainty whether this peculiar of way state loading may lead or not to inconsistency, but I have not experienced it in testing. (it could be Parameters and then Raw, or the reverse)

[baconpaul]

Gotcha. That idea of storing parameters as distinct from the streaming provided by the DAW is kind of unique to LV2. (It’s sort of what JUCE encourages you to do also but doesn’t require and juce then consistently maps it into the underlying single-store DAW state). It is also surely where the problem is coming from. It’s also a bold design choice to be a requirement (as opposed to an option). I suppose an LV2 could also advertise “ignore my params; I’ve got this” and then just hand you a blob as a mode. Then it would look just like the other flavors. but that’s not a surge issue.

But what @unfa described would happen if, say, some default values (which I know you dump into the ttl) get sent to the synth after you restore the DAW state. I’m imagining something (naively) like

Thread 1
A restore parameters to values
B send parameters to synth
C restore daw state

Thread 2
A make parameters with defaults
B send parameters to synth

So clearly if the order is 2A 2B 1A 2B 1C we are fine. If the order is 2A 1A 1B 1C 2B we will get the state (but not the modulation) reset to the init default. If the order is 2A 1A 1C (1B/2B mixed) then we would get exactly what @unfa reports.

Can anyone tell me or point me at a youtube video or anything about how to even run one of these LV2 daws on linux and save a state? You say Carla is breaking - I’ve not really used it. How is it breaking?

Finally, if the LV2 is doing what you say it should be doing properly, then the assertion should be true that restore params / restore state will leave the params unchanged, since the streaming of the state contains the value of the params. That’s something we could assert in the LV2 code to catch the bug. Basically snap the .val.f of every ptr before you do a loadState and after in a throwaway array. If any change then you have a problem. (This constraint is NOT true in the VST2/3/AU since the only persistence of state in those environments is the DAW XML).

[jpcima]

Can anyone tell me or point me at a youtube video or anything about how to even run one of these LV2 daws on linux and save a state?

It's more easy for me to explain in text, how to get a working dev workflow.

Prerequisite:

• get JACK started in Cadence, there are various resources to explain this
  (eg. https://kx.studio/Documentation:Manual:cadence_introduction)

Installing:

• Install Carla
• Make a git build of Surge
• Copy data files to /usr/share/Surge

Host:

• Go to Carla settings, plugin paths, LV2
• Add the path where Surge is built
  
• Reopen Carla to reload LV2; which should get Surge listed.
  
• Add Surge
• Go to patchbay tab, hook up Surge outputs to system
  
• "Save As" to store a session file, with a state, and with these connections persisted (example ~/MySurgeSession.carxp)

From this point on, you can write following in the Surge source dir and get one-line build+restore.
./build-linux.sh build -p lv2 && carla ~/MySurgeSession.carxp

(except restore not working at the moment, but you get the idea..)

[baconpaul]

Very useful thanks. I was able to follow these steps and see the bug. Not fix it, but see it.

[baconpaul]

Hey next question (sorry for all of these)

When I run carla and put a std::cout in the code, I don't see stdout in the terminal. Clearly that is being routed somewhere else. Do you or @falkTX know where?

[falkTX]

The logs tab in the GUI.
The first page in the settings allows you to turn that off as needed.

[baconpaul]

Thanks!!!!

[baconpaul]

Alright I have a fix to this.

Basically carla calls connectPort before restoreState and then you check if a value has changed vs a cache, but you build that cache after connectPort and before restoreState so it gets the default values, not the streamed values.

If, in restoreState, you write onto the float * which are each of the port values, Carla restores properly.

Let me put my diff up here in a branch for you guys to review and test, OK?

[unfa]

I have tested Surge again in Ardour 5.12 yesterday and I had patch corruption happening without closing ad opening the UI.

I would rewind the transport to play a part in my timeline again and LFO and insert effect settings would change on their own. I have video proof of this:
https://youtu.be/Rvf0Ihe8_zg

[baconpaul]

Yeah the state management is still not right. I’m working on it with talk and jp

[baconpaul]

Ok @falkTX here is what is happening in Carla

I save a carlxp file following directions above. I save it with parameter 118 (pitch) set to .8 and the default is .5

When I restore I get the following in order

1. I get a connect port. In this method the port value is .5 - the default
   2 I get a restorestate - in this I get the xml which has the .8
   3 I get a second call to restore state
   4 I get an activate
   5 I get a run - in that run the internal state of surge is .8 but the port is .5
   6 so the surge takes the value of the port and overwrites the xml restore

So: what is the mechanism by which Carla restores values of the ports before run is called and what is surge doing to not get the ports restored properly even though the xml is?

[baconpaul]

Like do I need to cal super::save state in save state or some such?

[falkTX]

Do you see 0.8 as the pitch value in the carla state file?
If not, what carla is doing seems correct to me.

Note that the first restore is to startup the plugin in a good default state.
Could be ignored when loading a project I guess, but should not affect things here.

The connection_port call needs to be irrelevant.
Only state restore and run matter.

[baconpaul]

Yeah right so the carla state file contains the 0.8 and I think that I have found the problem

On the first restore - when presumably I get the settings - @jpcima is returning LV2_BAD_CHUNK if there's no custom so we drop the values

[baconpaul]

So: what should restoreState do with that first chunk? That's clearly the problem. Even if I return success the first chunk which contains the params is not being applied.

[baconpaul]

OK I pushed to baconpaul/lv2-1577 a version which shows the error.

Build with that, load surge, open a preset, modify OSC1 pitch slider up some, save. In the carlxp you will see that param 118 (which is that OSC1 pitch slider) has a value.

Then restore that carlxp. You will se that port 118 never gets the value set onto it, and is 0.5, and that 0.5 restores over the synth state.

So the problem is: The port never gets restored from the carlxp file.

I think it is because of the first call to restoreState and something the code is missing in that if block

[falkTX]

something is odd regarding carla and handling state, from what I see..

[baconpaul]

I agree with that! But is it Carla surge specific or just Carla and anyone who implements the lv2 State?

[falkTX]

ok I see the issue, it is surge after all.
some of the port symbols are duplicated, so carla ends up setting the port "Osc1_Pitch" twice.

the generated ttl has this:

        a lv2:InputPort, lv2:ControlPort ;
        lv2:index 118 ;
        lv2:symbol "Osc1_Pitch" ;
        lv2:name "Osc1 Pitch" ;
        lv2:default 0.5 ;
        lv2:minimum 0 ;
        lv2:maximum 1 ;

and then a bit later:

        a lv2:InputPort, lv2:ControlPort ;
        lv2:index 389 ;
        lv2:symbol "Osc1_Pitch" ;
        lv2:name "Osc1 Pitch" ;
        lv2:default 0.5 ;
        lv2:minimum 0 ;
        lv2:maximum 1 ;

that makes it impossible to save the state in a proper way.
so all session files made with surge-lv2 until now are all broken :(

[baconpaul]

Oh that’s super easy to fi! I just need to chang @jpcima ttl generator to use the streaming name rather than the display name.

Presumably name must be unique and symbol need not be?

[baconpaul]

(I am out seeing a show tonight but I can do this tomorrow)

[baconpaul]

Also is there an lv2 linter or something we should run? I use the au and vst3 validation to check those versions

[falkTX]

symbol must be unique, name does not have to be.
what is this streaming name?

there is an lv2lint tool yes. it would have caught this issue I guess..

[baconpaul]

Surge has a parameter name which is stable and unique for streaming and a display name. Seems we use display for both symbol and name. I will modify symbol to use the unique name

[falkTX]

The lv2 code is using getParameterName though..
In any case, I can get a fix soon

[falkTX]

Here is a quick&dirty patch that works here:

diff --git a/src/lv2/SurgeLv2Export.cpp b/src/lv2/SurgeLv2Export.cpp
index f5dc871..0a4fa12 100644
--- a/src/lv2/SurgeLv2Export.cpp
+++ b/src/lv2/SurgeLv2Export.cpp
@@ -1,5 +1,7 @@
 #include "SurgeLv2Wrapper.h"
+#include <algorithm>
 #include <fstream>
+#include <vector>

 #if defined _WIN32
 #define LV2_DLL_SUFFIX ".dll"
@@ -81,7 +83,9 @@ void lv2_generate_ttl(const char* baseName)
                       (c >= '0' && c <= '9');
             };
             if (name.empty())
+            {
                name = '_';
+            }
             else
             {
                if (!isLeadChar(name[0]))
@@ -92,6 +96,21 @@ void lv2_generate_ttl(const char* baseName)
                      name[i] = '_';
                }
             }
+
+            // Do not allow identical symbols
+            static std::vector<std::string> usedSymbols;
+
+            if (std::find(usedSymbols.begin(), usedSymbols.end(), name) != usedSymbols.end())
+            {
+                name += "_2";
+
+                if (std::find(usedSymbols.begin(), usedSymbols.end(), name) != usedSymbols.end())
+                {
+                    throw "parameter names are not unique enough";
+                }
+            }
+            usedSymbols.push_back(name);
+
             return name;
          })(pName);

[falkTX]

cleaned the patch just a bit.

btw, off-topic, but c++ std is pretty awful. I was trying to do the same I had with juce code, but damn there is no string "replace", array/vector "contains" errrr

[falkTX]

btw, I am combining that with your branch changes. those are needed too

[tank-trax]

not sure if this helps but the output of ./lv2lint https://surge-synthesizer.github.io/lv2/surge

```
./lv2lint 0.5.7
Copyright (c) 2016-2020 Hanspeter Portner ([email protected])
Released under Artistic License 2.0 by Open Music Kontrollers
https://surge-synthesizer.github.io/lv2/surge
[FAIL] Symbols
binary exports superfluous globally visible symbols:
* _ZGVcN4v___log_finite
* _ZNKSt12experimental10filesystem2v17__cxx114path17_M_find_extensionEv
* _ZNSt10filesystem10equivalentERKNS_7__cxx114pathES3_
* _ZNSt10filesystem7__cxx114path15_M_add_root_dirEm
* _ZNSt12experimental10filesystem2v17__cxx114path14_S_convert_locEPKcS5_RKSt6locale
* _ZNSt12experimental10filesystem2v16statusERKNS1_7__cxx114pathE
* _ZNSt10filesystem9proximateERKNS_7__cxx114pathES3_RSt10error_code
* _ZNSt10filesystem7__cxx1116filesystem_errorD0Ev
* _ZNKSt12experimental10filesystem2v17__cxx114path18has_root_directoryEv
* _ZNSt10filesystem5spaceERKNS_7__cxx114pathE
* ... there is more, but the rest is being truncated
seeAlso: http://lv2plug.in/ns/lv2core#binary
[WARN] Author Email
foaf:email not found
seeAlso: http://lv2plug.in/ns/lv2core#project
https://surge-synthesizer.github.io/lv2/surge#UI
[FAIL] Symbols
binary exports superfluous globally visible symbols:
* _ZGVcN4v___log_finite
* _ZNKSt12experimental10filesystem2v17__cxx114path17_M_find_extensionEv
* _ZNSt10filesystem10equivalentERKNS_7__cxx114pathES3_
* _ZNSt10filesystem7__cxx114path15_M_add_root_dirEm
* _ZNSt12experimental10filesystem2v17__cxx114path14_S_convert_locEPKcS5_RKSt6locale
* _ZNSt12experimental10filesystem2v16statusERKNS1_7__cxx114pathE
* _ZNSt10filesystem9proximateERKNS_7__cxx114pathES3_RSt10error_code
* _ZNSt10filesystem7__cxx1116filesystem_errorD0Ev
* _ZNKSt12experimental10filesystem2v17__cxx114path18has_root_directoryEv
* _ZNSt10filesystem5spaceERKNS_7__cxx114pathE
* ... there is more, but the rest is being truncated
seeAlso: http://lv2plug.in/ns/lv2core#binary
[WARN] Instance Access
usage of instance-access is highly discouraged
seeAlso: http://lv2plug.in/ns/ext/instance-access
[WARN] Mixed DSP/UI
mixing DSP and UI code in same binary is discouraged
seeAlso: http://lv2plug.in/ns/extensions/ui#

[falkTX]

Please note that LV2 state relates to port symbols. (LV2 does not use parameter indexes for state)
So once we release, we cannot change them. not without breaking compatibility with everyone's saved presets/projects.

[baconpaul]

Cool and glad that works but I’ll merge a version which uses our built in unique name to match the xml. Will make patch tomorrow - thanks folks!!!

[baconpaul]

Oh and the name I will use is the same name we use in our xml blob so if it isn’t stable way more than lv2 will break! Thanks for the help here

[baconpaul]

Ha OK when I replace the symbol with the streaming name everything works. Will take me a wee bit to get a PR together this morning but should be a fix by lunchtime.

[baconpaul]

Alright PR #1596 is the answer. If you can look @falkTX that would be awesome! It now works perfectly for me in carla and uses proper unique names for every parameter (the same names we use to save the .fxp)

[baconpaul]

OK @unfa I have pushed a fix based on the conversation @falkTX and I had. In Carla it now streams and unstraps properly and works with automation. We're both confident this is correct.

The problem was that LV2 ports were mis-labeled. So our fix means everything works. Bit it also means that saved sessions from the buggy state won't reload properly. Sorry - but there's no other way to fix the problem. Since those sessions were buggy it's not that bad.

I want to mark the release as 1.6.6 tomorrow morning. Is it possible for you to build from master and test today? The git commit for the corrected version is 8dc0cf88a1537. I'm not sure how you are acquiring surge but that git commit checkout and a build or the pre-built .deb nightly which will be on our website in about 30 minutes should both contain the fix.

Thanks!

[tank-trax]

It appears to be working but this weird thing happens in Qtractor using LV2

created a patch and saved session
reopen and Surge has the INIT playing when the MIDI device is played, however upon opening Surge or displaying its GUI and it plays the correct streamed/saved preset in the DAW

tried with a 2nd preset and it loads the correct sound when qtractor opens whether Surge's GUI is displayed or not,

in both cases the preset browser window displays INIT as it was not a preset that was saved but the session

it does appear to be holding the streamed sounds that are created without needing to save them as a Surge FXP preset

Update: it's not holding in Ardour (on first save occasionally, revisiting the sessions seems to work)

[baconpaul]

so "opening the editor restores the state"?
Sigh. This is hard. How do I run QTractor and Ardour?

[falkTX]

Note that qtractor might send a MIDI program message at the beginning of the timeline.
Not sure if related or not..

[baconpaul]

So I just started qtractor and loaded a preset and saved and when i restored and opened surge I had the preset name. When I look at the .qtr file it has everything saved properly also it seems.

Lemme see if I can make ardour work in my vm.

@tank-trax you are sure you are using latest, and also using fresh qtr files? Files you made with the old lv2 will not work reliably. (Although I will notice that qtractor streams the id number and name, not the symbol, differently than Carla, to the qtr)

[baconpaul]

So I can't actually test my sound or midi since my ubuntu vm doesn't route to my mac speakers and I don't have any usb midi devices which are set up for linux. And if I restore my std::cout prints in qtractor, it dumps core when I load the LV2 (Ha!).

I tried to make Ardour load the LV2 but couldn't find the LV2 path in any of the dialogs

So I'm not going to be much help. We are at least better off - we work in Carla it seems now - so I still plan to sweep this diff into 1.6.6 tomorrow.

But if I'm going to debug this I will need a lot of help with these tools and also things like "making ubuntu in a VM play out my mac speakers" which I've not needed to do up until now.

[tank-trax]

I will post some examples

[baconpaul]

As I mentioned on slack I can delay 1.6.6 a day or two while we try to pin this down

If you can tell me how to find and load an lv2 in ardour i Can look this weekend

[tank-trax]

At boot up Ardour (and Mixbus) scan for plugins, generally scanning whatever is in LV2_PATH, LADSPA_PATH and VST_PATH

sometime though certain plugins get blacklisted or need rescanning
that would be in "Preferences -> Plugins -> Scan for Plugins"

to find paths in Linux for plugins...

echo $VST_PATH
~/.vst:/usr/lib/vst:/usr/local/lib/vst:/usr/lib/lxvst:~/.vst3 (I will have to add /usr/lib/vst3)

echo $LV2_PATH
~/.lv2:/usr/lib/lv2:/usr/local/lib/lv2

echo $LADSPA_PATH
~/.ladspa:/usr/lib/ladspa:/usr/local/lib/ladspa

echo $DSSI_PATH
~/.dssi:/usr/lib/dssi:/usr/local/lib/dssi

How to load LV2.... and an example of created preset saved by DAW, looks to be good, the qtractor thing could be an anomaly

I think it's OK, apart from qtractor needing to display the GUI on one stream to load it JALV.SELECT. Carla, Ardour are loading the saved/streamed preset according to the DAW,

[baconpaul]

OK so it sounds like we are feeling pretty good then with the change? Ardour seems to work?

[unfa]

I've just tested in Ardour and I don't know about re-opening the GUI, but I get patch changing itself when I stop the transport:

https://youtu.be/RTS5PQG70vQ

I've built today from source:

I'm not sure if this should be put on a separate issue.

[baconpaul]

It's all the same thing - we can leave it here.

Can you share that ardour file (just zip it and attach it to this issue) so I don't have to figure out how to set up and make midi flow into ardour?

I'm not sure which API point is getting called on transport stop but if I can just run it I will get there.

I really wonder why your build has a version number which says 1.6.2 + git by the way. That git hash is the latest so it is fine but whatever packing mechanism you are using should fix that version number! Totally separate issue though.

Thanks!

[baconpaul]

Actually I got it all working and it seems totally fine to me. Can stop transport, can do automation, can send notes.

I wonder: We had to change the lv2 description to fix this bug. Is there some ardour cache hanging around which you need to blow? I am using basically a fresh vm and a fresh install of everything so I have no lingering stuff.

https://www.youtube.com/watch?v=HlUHcsVI4F0&feature=youtu.be

there's a video of it running fine for me in a simple 'play 3 notes, have a patch saved'. And it all gets retained if I save and restore.

Thoughts on what's different between my experience and yours? The lingering cache is the only one I can really imagine.

[baconpaul]

And specifically from looking at how ardour saves files, if you are using a session that you created with surge.lv2 before 8dc0cf8 it will definitely have incorrect internal state that will lead to undefined behaviors.

[unfa]

That particular session was created long ago and I used it for testing. I should create a new session and test there too I guess.

[tank-trax]

git clone https://aur.archlinux.org/surge-synthesizer.git

will pull the PKGBUILD file

the version in the file says 1.6.5

and they are at Package Details: surge-synthesizer 1.6.5-1

https://aur.archlinux.org/packages/surge-synthesizer/

[unfa]

Here's the old test session I recorded the video with:
Surge Test.zip

[baconpaul]

That particular session was created long ago and I used it for testing. I should create a new session and test there too I guess.

Yes the nature of our bug means that we had to change port symbols. Looking at the ardour implementation, they keep their port mappings based on symbols (which is a correct use of the spec) but when we change them it means the old session will be unpredictable.

The double fact that our bug was we had dup symbols means old sessions are double unpredictable

A fresh session is, alas, what's required yes. Sorry!

[unfa]

Ok, I've tested with a fresh session. The transport start/stop doesn't cause the corruption.
But re-opening the GUI does!

[baconpaul]

OK great. I should have tested that I'm sorry. Lemme fire up my VM.

[baconpaul]

Alright so I just opened and closed the UI 20 times and didn't have vales change in the UI.

Can you give just a touch more detail?

I'm sorry - I'm so unfamiliar with these tools that debugging is really slow for me. I appreciate your help.

[unfa]

I've closed and re-opened Ardour and created yet another new session to be sure, but I still get this issue. Here's the session:
Surge Test 3.zip

[unfa]

My oscillators are reset to Sawtooth and my LFO modes get reset too.
I think Send effect returns are also reset to 100%, but haven't tested it right here.

Not all parameters are being reset. I think the filters are intact.
I've been testing a bit more and I still get the transport corruption now.

[unfa]

What's interesting - transport corruption occurs eve if the GUI is closed.

That its: when I stop playback in Ardour the oscillator types get reset to Sawtooth (but not other parameters or LFO assignements).

Also my LFO's get switched back to Sine, from Envelope type I've set.
Would a debug build be helpful?

[baconpaul]

Fascinating. I am putting in a bunch of log stuff. Seems ardour is calling activate/deacivate when you press stop. Taking a look....

A debug build might help. What I really need to do though is see what ardour is calling when with what values so I'm just using printfs right now tbh.

Stay tuned...

[baconpaul]

Oh boy that LFO comment is super useful. I'm actually able to reproduce that.

Basically open UI, set LFO type to square, close UI, open UI - voila sine again.

That's something I can debug for sure!

[baconpaul]

OK here is exactly the problem

Some parameters (and oscillator type and LFO shape) are not sending automation events to the LV2

Since LV2 (and only LV2) maintains this idea that there is an independently managed version of the state which is authoritative, though, since the LFO shape switch updates the internal state and the patch, but not the port, when you deactivate/reactivate you end up getting reset to the stored value in the port (in this case, sine and sawtooth) even though that value is nonsense.

I don't want to editorialize about the LV2 design decision that much other than to say it is radically different from the VSTs and the AU. But now I know this I think I can make a fix which for 1.6.6 I can conditionalize just to the LV2 to those two control points, and can test the rest.

[baconpaul]

Sigh. Very very difficult but I think I will have the fix soon enough. I'll just wrap it all in an #ifdef LV2 so we don' have to pollute the other hosts this close to a release. My guess is tomorrow or monday before i have something testable.

The three areas are the LFO type, the effects, and the oscillator type

And again the problem is that LV2 wants to pretend that the ports are primary, whereas the other host types know the synth is primary. And so we have to do careful state management. If we miss one message from synth -> host in LV2 we get corrupted, whereas in every other plugin style we have no problem because the host asks the plugin for values and doesn't maintain independent state that needs coordinating. :(

[baconpaul]

OK that LFO tip was exactly what I needed

Three things - FX, LFO and OSC - could get out of sync between the synth and the ports because Surge chose to not notify automation of those in bulk. In the other flavors this wasn't a problem. In the LV2 it is fatal.

So I have forced surge to scream parameter changes to the LV2 when you change those things and now it all works.

Let me let our CI run and then I'll merge the change and you can try it again

Thanks for your patience.

[baconpaul]

Alright @unfa @tank-trax if you can try as of fc6af5b970 that would be really appreciated. I think I got it all

If you do see a corruption (which is more accurate called a 'non-synchronization' now I understand it) I'll need to know which field I missed so I can apply the same fix to it. I tried them to make sure messages get sent and think i got them all but some exhaustive test (which is basally: Change something, close the ui, press plan and stop, open the UI, is it the same) would be helpful.

the always-on sliders are all OK. It's just the switches and dynamic elements which could be broken.

Would still like to do a release on Wednesday so feedback tomorrow or Monday would be super useful if you can.

Thank you for your help and patience.

[falkTX]

Did I understood things wrong, or was the act of opening the UI triggering changes in the DSP?
When it Ui opens, it needs to read values from DSP side and display them. It should never ever change values when opened, parameters and state must only change when dealing with an action from the user.
So even if the LFO type in the UI got wrong, it still should not do anything regarding the DSP side... right?

[baconpaul]

So @falkTX here’s what was happening exactly

There were UI gestures which changed the internal state of the UI but didn’t broadcast parameter changes to the LV2.

This mean the LV2 ports kept the old copy

When you start and stop transport or in some cases open and close the UI in Ardour it activates and reactivates; or does some other form of reapplying the ports to the synth. There’s a couple of paths but basically if the synth and ports are inconsistent you see it when changing transport or when opening and closing.

Thus the old copy was applied

My fix was to make sure the UI gestures sent the full set of parameter changes to the LV2

This isn’t a problem in the other hosts since they don’t maintain and apply independent copies of the state

I think it is probably wrong that we don’t broadcast the parameter changes from surge but for our upcoming release, I conditioned it to only LV2 to minimize risk. I will revisit in 1.7 if we get confirmation that the latest build fixes.

If I were to make one LV2 feature request it would be to have a “purely virtual” port whose getter doesn’t store a value but rather calls a function every time, and has a notification of change. I don’t know how hard that is but such a port means we could keep the state resident in surge and not have this double state management to deal with.

Appreciate your help. The double copy of the state is a tricky feature of LV2 for sure but I think we have it now.

[baconpaul]

Actually now I think of it we can probably remove all the state caching stuff if we just make the patch reload send an automation message for every parameter in the lv2. I don’t want to make that big a change now especially if we have it right with my tactical change but clearly there’s an opportunity to use what I learned last 12 hours and fix some other bits of state management in a way which makes surge more lv2-y. Separate issue though - what I need now is confirmation or bug report on current branch. Thanks!

[baconpaul]

Actually: I'm going to take a look at doing just this. Since the LV2 is broken now it can't make it any worse and I can conditionalize it all. Really would appreciate tests at head but may be more coming.

Ignore that. I'm sticking with the patch we have. I see why it is complicated - there are situations where we change a patch in surge which are not triggered by a UI action, like midi program changes - and I now see how @jpcima dealt with that.

I think we can, indeed, deal with that better by doing a big collection of UI-like gestures from the synth but it's more restructuring than I want to do

The version at master i the version I want to go with for now. Sorry for the git noise.

[unfa]

I've just built from latest master and I can't reproduce this problem any more!
Horray!

I don't want to bless the day already, because it might be too early, but I think you've got it sorted out guys.

Thank you so much for your work!

[baconpaul]

Awesome. Thank you very much!!

I plan to ship this version as 1.6.6 on probably Wednesday.

[unfa]

I'd like to perform some thorough testing to make sure this is done and dusted before the release, but I am afraid I won't manage to do as much. I will try to test as much as I can and provide feedback here if I find any problems st ill.

I am very happy, because I can finally start using Surge in my music and make videos about it! I've been waiting fro so long :D

Thank you!

[baconpaul]

Yeah great! And sorry you had been waiting so long - that's a bummer bug glad we got it fixed once you flagged it here.

Please do flag issue here on the GitHub. It is the only way I'll reliably see them.

[unfa]

@baconpaul Thank you so much for your work on fixing this. I hope to test Surge more thoroughly soon and start using it in my work. I'll report any residual related issues here if they creep up.

[baconpaul]

Wonderful! As you do please remember we’ve worked to keep the manual up to date; so if you find the manual doesn’t match your experience that’s also a bug we would fix. And enjoy making music with surge!

[unfa]

I've found some more places where this is still present.

When I used 4 LFOs (1 - 4) and switched them to envelope mode, they got switched back to sine and also the sustain level of the envelopes was set to 1.

You can see that on the snare patch here:
Hardbass Test.zip

(I was unable to get proper amount of pitch modulation fro ma single LFO/envelope so I used 4 together to get the desired effect - BTW am I doing it wrong?)

[unfa]

I will update now and see if I can reproduce.

[baconpaul]

OK I was going to do the 1.6.6. Release tomorrow morning. I’ll peek at this case before I do and perhaps sneak a fix in under the wire. Thanks!

As to extreme pitch modulation, right click on the pitch slider and choose “extend” and both your setting and modulation range go from +/- 12 to +/- 96 notes.

[baconpaul]

I can't load your session; but I was able to have the envelope mode and envelope sustain work all no problem for me with the code at head in ardour

[baconpaul]

Since I can't repro this I'll proceed with the 1.6.6 release this morning - we can always fix it later if there is a problem we find a solution to and can even branch back into the 1.6.6 family if we are well down the 1.7 path.

[unfa]

Hmm, I've done a livestream using Surge last Sunday and it not only worked slowly, but also caused Ardour to crash multiple times, and then kept crashing the session upon load making it impossible to progress: https://youtu.be/hgsQu8xHU2k

I've just updated from Git and I've tried to use it again, but I have the transport top/start resetting my Insert FX dry/wet to 100% and also it crashed Ardour once.