radium icon indicating copy to clipboard operation
radium copied to clipboard

vst3 plugin info is not loaded correctly

Open ghost opened this issue 2 years ago • 19 comments

i noticed that when saving and reloading a project with a vst3 plugin, not all of the information in the plugin's patch is saved, and the plugin sounds different between saving and loading. this doesn't happen with vst2 plugins

i've tested surge and vitalium so far

i'm using 6.9.96

ghost avatar Oct 07 '21 07:10 ghost

I confirm the bug, TAL Noise Maker 5.0.4 VST3 is also affected. Tested on Radium official build 6.9.96 on Linux Mint 20.2

akimaze avatar Oct 15 '21 13:10 akimaze

I don't know what could cause this, but I'll see if I can upgrade JUCE to the lastest version for the next release. There's been a few fixes for VST3 (see this discussion for instance: https://forum.juce.com/t/hosting-vst3-with-juce-getting-incorrect-parameter-reference-from-specific-vst3-plugins/46441), so maybe that'll fix it.

kmatheussen avatar Oct 16 '21 13:10 kmatheussen

@kmatheussen I tried compile Radium with JUCE 6.1.2 (with modified juce_MidiMessage.h and juce_MidiMessage.cpp) saved sound changes a little but still something is not saved.

akimaze avatar Oct 19 '21 18:10 akimaze

@kmatheussen I tried the latest official 6.9.97 version to test is this bug but Surge 1.9.0 VST3 simply crashes (always reproducible) when I try add plugin to track from plugin manager. I sent the crash report.

akimaze avatar Jan 06 '22 17:01 akimaze

I tried latest version of Surge now, 1.9.09xxxx from 2021-04-21. And it seems to work. I can't reproduce the crash. But the crash you reported happened inside JUCE, so it doesn't look like a bug in Surge. I'll look into it. Thanks for reporting.

kmatheussen avatar Jan 06 '22 21:01 kmatheussen

Yes I use the same version as you (1.9.091069f8 2021-04-21) I also tested it (Surge) in other DAWS and it works OK. Vital VST3 (VST3 seems more stable than VST2) and Odin2 is working OK. Another plugin that crashes like Surge is CHOW Kick.

akimaze avatar Jan 07 '22 13:01 akimaze

I can't remember what preset in Tal-Noise Maker was incorrectly saved in the project. But if you have a working Surge you can easily check it by selecting the preset Leads-> Bee from Surge. In the track insert a note eg C4. Then save and load the Radium project, the sound will be completely different. When you open Surge UI for example Amplitude of LFO 1 parameter was not saved, Filter EG envelope, and a lot of parameters in FX block.

akimaze avatar Jan 07 '22 13:01 akimaze

The VST3 crashes should be fixed in https://github.com/kmatheussen/radium/commit/70176fd11cb0c5c339e587921db66cd78b1eab5c It was caused by juce throwing an assertion because bus configurations was not supported. Assertions shouldn't be enabled in release mode.

kmatheussen avatar Jan 08 '22 10:01 kmatheussen

@kmatheussen Thank you Radium is not crashing now but still something is not saved from plugin state.

I had an idea how to check if the problem is JUCE (6.1.4) or some where in Radium. I compiled Juce Audio Plugin Host (from extras folder, simply run make), added Surge (preset Leads-> Bee) and saved filtergraph to file. After reading the file, the plug-in state is fully restored. From this I conclude that the problem is in saving (or reading) to the file in Radium. And JUCE is OK.

image

akimaze avatar Jan 08 '22 14:01 akimaze

I don't know exactly what's causing this, but it seems related to loading state twice, which of course isn't necessary, but shouldn't cause any problems either in my opinion.

As a quick fix, you can remove the call to 'recreate_from_state' in audio/Juce_plugins.cpp, around line 2365:

@@ -2353,7 +2361,7 @@ static void *create_plugin_data(const SoundPluginType *plugin_type, SoundPlugin
     
     run_on_message_thread([audio_instance_pointer, plugin, type_data, state2, is_loading]() {
       plugin->data = new Data(audio_instance_pointer, plugin, type_data);
-      recreate_from_state(plugin, state2, is_loading);
+      //recreate_from_state(plugin, state2, is_loading);
       });     

kmatheussen avatar Jan 09 '22 13:01 kmatheussen

I confirm that your quick fix fixes that issue. Thank you!

akimaze avatar Jan 09 '22 14:01 akimaze

Good. A word of caution though, I haven't tested, but it's possible that removing that line causes loading of .rec or .mrec files to fail, and/or that changing A/B settings will fail. In that case, this patch might work instead:

@@ -2353,7 +2361,8 @@ static void *create_plugin_data(const SoundPluginType *plugin_type, SoundPlugin
     
     run_on_message_thread([audio_instance_pointer, plugin, type_data, state2, is_loading]() {
       plugin->data = new Data(audio_instance_pointer, plugin, type_data);
-      recreate_from_state(plugin, state2, is_loading);
+      if (!is_loading)
+        recreate_from_state(plugin, state2, is_loading);
       });  

kmatheussen avatar Jan 09 '22 15:01 kmatheussen

From my tests: loading VST3 plugin with A,B,C,D - only A works OK with the first and second quick fix. But loading .rec preset works better in first solution than the second. With the first solution first (A) settings are OK but others was incorrect so something is not going right. With the second solution loading rec file (if (!is_loading)) was totally broken - all A,B,C,D plugin states was incorrect.

So second loading is definitely broken - maybe something is freed after first loading? Or maybe this assertion that you disabled should be fixed? Or maybe this is some threading issue? I don't know the Radium code so I'm just guessing ;)

akimaze avatar Jan 10 '22 18:01 akimaze

After #1367 I started to think that it could be the same with loading VST3 plugins. I did more tests with versions 96 and 97 of Radium. After testing it turns out that Vital (Chorusy Keys preset) which did not work in 96 in 97 works well. Similarly I found Tal-Noise Maker preset (ARP 303 Like FN) which did not work in 96 also works well in 97. Only this Bee preset from Surge still not working in 97. Today, however, the new Surge XT 1.0 came out and the Bee preset works in my build without your quick fix (I can't test it in official 97 because of this assertion that crashes Radium).

So it looks like the JUCE update has fixed the problem. But double loading makes Radium much more sensitive to bugs in plugins, so it's probably worth removing the second loading if it's possible :)

Notice that we still need disabling assertion for new Surge XT also so don't remove it.

akimaze avatar Jan 18 '22 20:01 akimaze

I also found surge bug report about loading presets https://github.com/surge-synthesizer/surge/issues/5524

akimaze avatar Jan 18 '22 20:01 akimaze

Yeah, I'm pretty sure there's no threading bugs or anything like that. If there were, we would probably have noticed earlier since the same code is used for all types of plugins, not just VST3 plugins on Linux.

But I wonder which of the two state-loading calls that should be moved. Currently it works like this:

  1. Load plugin state
  2. Set plugin effects
  3. Load plugin state (again)

Should the plugin state be set after, or before, setting plugin effects?

kmatheussen avatar Jan 23 '22 16:01 kmatheussen

Oh, and my tip to fix the problem, by removing the call to 'recreate_from_state', was by removing step 1. So it's not that step 3 is the one causing the problem, which would have been a natural thing to think. It's a really strange bug, Most likely it's a bug in either the VST3 plugins themselves, or in JUCE, but it's really strange.

kmatheussen avatar Jan 23 '22 17:01 kmatheussen

For the last two months I have been still using Radium without quick fix (but with an assertion fix that you made for Surge) and I haven't noticed any problems with saving/loading projects (I use Surge XT instead of the old Surge). Since I'm using Radium as my default DAW now, it has been hundreds of loads and saves. So I would assume the problems were plug-in problems. And this quick fix is not needed. I will write a comment here if I notice a problem with loading the state of some plugins.

BTW. New JUCE version is released with some interesting fixes for VST3 and VST3Hosts: https://github.com/juce-framework/JUCE/commit/ec867690b7fb4a8dde011ec5a929e2ed22fda74b https://github.com/juce-framework/JUCE/commit/1bf9ebb4b113bf2dc3c4537974cd81f1cca67a97 https://github.com/juce-framework/JUCE/commit/a3c55a967f5d5811ef451c48dd095ebfd64f74ff https://github.com/juce-framework/JUCE/commit/bd0ca90952c95aeface9dcb4219d746f24f4601d https://github.com/juce-framework/JUCE/commit/a7811661c5d8d0bfc3ee8c1fdef5c5078640ab9c https://github.com/juce-framework/JUCE/commit/06db7f074e595f22be53a0cfc448011b88559ec4 https://github.com/juce-framework/JUCE/commit/bd52350c00ee96dc0a3175c7c7191a8fdf3f0c36

BTW2. Maybe is worth to add JUCE version to about dialog?

akimaze avatar Mar 17 '22 09:03 akimaze

Thanks for the update. I will try to update JUCE for the next release and include JUCE version in the about dialog.

On Thu, Mar 17, 2022 at 10:25 AM akimaze @.***> wrote:

For the last two months I have been still using Radium without quick fix (but with an assertion fix that you made for Surge) and I haven't noticed any problems with saving/loading projects (I use Surge XT instead of the old Surge). Since I'm using Radium as my default DAW now, it has been hundreds of loads and saves. So I would assume the problems were plug-in problems. And this quick fix is not needed. I will write a comment here if I notice a problem with loading the state of some plugins.

BTW. New JUCE version is released with some interesting fixes for VST3 and VST3Hosts: @.*** https://github.com/juce-framework/JUCE/commit/ec867690b7fb4a8dde011ec5a929e2ed22fda74b @.*** https://github.com/juce-framework/JUCE/commit/1bf9ebb4b113bf2dc3c4537974cd81f1cca67a97 @.*** https://github.com/juce-framework/JUCE/commit/a3c55a967f5d5811ef451c48dd095ebfd64f74ff @.*** https://github.com/juce-framework/JUCE/commit/bd0ca90952c95aeface9dcb4219d746f24f4601d @.*** https://github.com/juce-framework/JUCE/commit/a7811661c5d8d0bfc3ee8c1fdef5c5078640ab9c @.*** https://github.com/juce-framework/JUCE/commit/06db7f074e595f22be53a0cfc448011b88559ec4 @.*** https://github.com/juce-framework/JUCE/commit/bd52350c00ee96dc0a3175c7c7191a8fdf3f0c36

BTW2. Maybe is worth to add JUCE version to about dialog?

— Reply to this email directly, view it on GitHub https://github.com/kmatheussen/radium/issues/1350#issuecomment-1070613107, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIX3J7RDV3UXOYKM2ZAG6DVAL263ANCNFSM5FQRBN5A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

kmatheussen avatar Mar 19 '22 10:03 kmatheussen