View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0001291 | OpenMPT | Plugins / VST | public | 2020-01-12 14:43 | 2021-01-10 18:30 |
Reporter | Saga Musix | Assigned To | manx | ||
Priority | normal | Severity | feature | Reproducibility | have not tried |
Status | resolved | Resolution | fixed | ||
Target Version | OpenMPT 1.30.01.00 / libopenmpt 0.6.0 (upgrade first) | Fixed in Version | OpenMPT 1.30.01.00 / libopenmpt 0.6.0 (upgrade first) | ||
Summary | 0001291: Separate plugin bridge versions for legacy / buggy plugins | ||||
Description | We have observed many plugins which do not (any longer) work in OpenMPT, also not if run in the plugin bridge. Sometimes this is the result of using safer defaults for linker flags, OS compatibility declarations etc. For example:
To keep these old plugins alive, we could provide a second "unsafe" set of plugin bridge executables which disables DEP, high entropy heap and does not declare any operating system compatibility. DPI-awareness doesn't need to be handled this way as it can be configured at runtime. | ||||
Tags | No tags attached. | ||||
Has the bug occurred in previous versions? | |||||
Tested code revision (in case you know it) | |||||
related to | 0001290 | closed | Crash files from when opening Channel Matrix 2x2 plugin window | |
related to | 0001012 | closed | VST SignalAnalyzer causes crash when opened | |
related to | 0001030 | resolved | manx | VSTi Synth1 64bit crashes / not working |
related to | 0001401 | resolved | manx | Do not silently ignore crashes in VST plugins by default any more |
related to | 0001402 | resolved | manx | Use modern process settings for the main OpenMPT executable |
|
|
Committed as 1.30.00.24 (13999). |
|
Forum thread: https://forum.openmpt.org/index.php?topic=6537.0 |
|
As a suggestion, please add an option to register plugins using legacy plugin bridge. I used DeltaSP's plugins wich were broken with the newer security implementations. |
|
jmkz - have you read the "Legacy Plugin Bridge" section in https://forum.openmpt.org/index.php?topic=6537.0 ? I think that answers your question. |
|
There was no question, jmkz has a very valid point - if newly plugins are always scanned using the modern bridge variant, older plugins that crash in that variant cannot be loaded. If the modern bridge fails while scanning the plugin, OpenMPT must fall back to scanning the plugin using the legacy bridge. |
|
The last sentence says: There is a new setting in the Plugin Dialog that allows switching. (since 1.30.00.25). Means it's possible to switch between legacy & new Plugin Bridge, or? Would that not answer his point? |
|
The problem is that this is encourages the user to just always enable the legacy bridge, since it will work with more plugins. This is not the same as only falling back to the legacy bridge if the modern bridge crashed during initialization. |
|
Thanks, got it. |
|
Agreed, this must be addressed. |
|
pluginbridge-fallback-to-legacy-v2.patch (5,724 bytes)
Index: mptrack/Vstplug.cpp =================================================================== --- mptrack/Vstplug.cpp (revision 14002) +++ mptrack/Vstplug.cpp (working copy) @@ -94,7 +94,7 @@ } -AEffect *CVstPlugin::LoadPlugin(bool maskCrashes, VSTPluginLib &plugin, HMODULE &library, bool forceBridge) +AEffect *CVstPlugin::LoadPlugin(bool maskCrashes, VSTPluginLib &plugin, HMODULE &library, bool forceBridge, bool forceLegacy) { const mpt::PathString &pluginPath = plugin.dllPath; @@ -106,7 +106,7 @@ { try { - effect = BridgeWrapper::Create(plugin); + effect = BridgeWrapper::Create(plugin, forceLegacy); if(effect != nullptr) { return effect; Index: mptrack/Vstplug.h =================================================================== --- mptrack/Vstplug.h (revision 14002) +++ mptrack/Vstplug.h (working copy) @@ -77,7 +77,7 @@ CVstPlugin(bool maskCrashes, HMODULE hLibrary, VSTPluginLib &factory, SNDMIXPLUGIN &mixPlugin, Vst::AEffect &effect, CSoundFile &sndFile); ~CVstPlugin(); - static Vst::AEffect *LoadPlugin(bool maskCrashes, VSTPluginLib &plugin, HMODULE &library, bool forceBridge); + static Vst::AEffect *LoadPlugin(bool maskCrashes, VSTPluginLib &plugin, HMODULE &library, bool forceBridge, bool forceLegacy); protected: void Initialize(); Index: pluginBridge/BridgeWrapper.cpp =================================================================== --- pluginBridge/BridgeWrapper.cpp (revision 14002) +++ pluginBridge/BridgeWrapper.cpp (working copy) @@ -257,11 +257,11 @@ // Create a plugin bridge object -AEffect *BridgeWrapper::Create(const VSTPluginLib &plugin) +AEffect *BridgeWrapper::Create(const VSTPluginLib &plugin, bool forceLegacy) { BridgeWrapper *wrapper = new(std::nothrow) BridgeWrapper(); BridgeWrapper *sharedInstance = nullptr; - const Generation wantedGeneration = plugin.modernBridge ? Generation::Modern : Generation::Legacy; + const Generation wantedGeneration = (plugin.modernBridge && !forceLegacy) ? Generation::Modern : Generation::Legacy; // Should we share instances? if(plugin.shareBridgeInstance) Index: pluginBridge/BridgeWrapper.h =================================================================== --- pluginBridge/BridgeWrapper.h (revision 14002) +++ pluginBridge/BridgeWrapper.h (working copy) @@ -200,7 +200,7 @@ static bool IsPluginNative(const mpt::PathString &pluginPath) { return GetPluginBinaryType(pluginPath) == GetNativePluginBinaryType(); } static uint64 GetFileVersion(const WCHAR *exePath); - static Vst::AEffect *Create(const VSTPluginLib &plugin); + static Vst::AEffect *Create(const VSTPluginLib &plugin, bool forceLegacy); protected: BridgeWrapper(); Index: soundlib/plugins/PluginManager.cpp =================================================================== --- soundlib/plugins/PluginManager.cpp (revision 14002) +++ soundlib/plugins/PluginManager.cpp (working copy) @@ -533,6 +533,7 @@ } #ifndef NO_VST + bool requiresLegacyBridge = false; unsigned long exception = 0; // Always scan plugins in a separate process HINSTANCE hLib = NULL; @@ -542,7 +543,9 @@ ExceptionHandler::ContextSetter ectxguard{&ectx}; #endif // MODPLUG_TRACKER - Vst::AEffect *pEffect = CVstPlugin::LoadPlugin(maskCrashes, *plug, hLib, true); + Vst::AEffect *pEffect = nullptr; + + pEffect = CVstPlugin::LoadPlugin(maskCrashes, *plug, hLib, true, false); if(pEffect != nullptr && pEffect->magic == Vst::kEffectMagic && pEffect->dispatcher != nullptr) { @@ -568,6 +571,42 @@ validPlug = true; } + if(!validPlug || (exception != 0)) + { + validPlug = false; + exception = 0; + pEffect = nullptr; + + requiresLegacyBridge = true; + + pEffect = CVstPlugin::LoadPlugin(maskCrashes, *plug, hLib, true, true); + + if(pEffect != nullptr && pEffect->magic == Vst::kEffectMagic && pEffect->dispatcher != nullptr) + { + CVstPlugin::DispatchSEH(maskCrashes, pEffect, Vst::effOpen, 0, 0, 0, 0, exception); + + plug->pluginId1 = pEffect->magic; + plug->pluginId2 = pEffect->uniqueID; + + GetPluginInformation(maskCrashes, pEffect, *plug); + +#ifdef VST_LOG + intptr_t nver = CVstPlugin::DispatchSEH(maskCrashes, pEffect, Vst::effGetVstVersion, 0,0, nullptr, 0, exception); + if (!nver) nver = pEffect->version; + MPT_LOG(LogDebug, "VST", MPT_UFORMAT("{}: v{}.0, {} in, {} out, {} programs, {} params, flags=0x{} realQ={} offQ={}")( + plug->libraryName, nver, + pEffect->numInputs, pEffect->numOutputs, + mpt::ufmt::dec0<2>(pEffect->numPrograms), mpt::ufmt::dec0<2>(pEffect->numParams), + mpt::ufmt::HEX0<4>(static_cast<int32>(pEffect->flags)), pEffect->realQualities, pEffect->offQualities)); +#endif // VST_LOG + + CVstPlugin::DispatchSEH(maskCrashes, pEffect, Vst::effClose, 0, 0, 0, 0, exception); + + validPlug = true; + } + + } + } FreeLibrary(hLib); if(exception != 0) @@ -574,6 +613,13 @@ { CVstPluginManager::ReportPlugException(MPT_UFORMAT("Exception {} while trying to load plugin \"{}\"!\n")(mpt::ufmt::HEX0<8>(exception), plug->libraryName)); } + + if(requiresLegacyBridge) + { + plug->useBridge = true; + plug->modernBridge = false; + } + #endif // NO_VST // Now it should be safe to assume that this plugin loaded properly. :) @@ -720,7 +766,7 @@ HINSTANCE hLibrary = nullptr; bool validPlugin = false; - pEffect = CVstPlugin::LoadPlugin(maskCrashes, *pFound, hLibrary, TrackerSettings::Instance().bridgeAllPlugins); + pEffect = CVstPlugin::LoadPlugin(maskCrashes, *pFound, hLibrary, TrackerSettings::Instance().bridgeAllPlugins, false); if(pEffect != nullptr && pEffect->dispatcher != nullptr && pEffect->magic == Vst::kEffectMagic) { |
|
Maybe something like that? |
|
Makes sense, but I have not mention that it won't fix those plugins. They were already broken in earlier OpenMPT versions (even those which didn't use modern security features or declared compatibility with Windows 10), so it's likely that the plugins simply doesn't like running on Windows 10 at all. Still, we should of course apply this patch. However, before applying the duplicated logic in PluginManager.cpp should be de-duplicated into a function. |
|
Done. Legacy fallback when adding a plugin has been committed as r14028. |
|
I just tested, and it struggles to catch error with these DeltaSP plugins: https://drive.google.com/file/d/1RtJbkFFV23YVCM44fOxkvYJTA13kvXPw/view?usp=sharing keeps crashing OpenMPT. |
|
Yes, but they do that already in any previous OpenMPT version I have tested. Is that not the case for you? |
|
Same happens for me, more or less a year ago (something I remind is broken when switched from W 10 1909 to 2004). Module in question was last saved using 1.29.00.53. So maybe the last working version. |
|
As said, I tried version up to OpenMPT 1.22 and they all showed the same issue. It's probably a Windows update that broke it. |
|
Date Modified | Username | Field | Change |
---|---|---|---|
2020-01-12 14:43 | Saga Musix | New Issue | |
2020-01-12 14:44 | Saga Musix | Relationship added | related to 0001290 |
2020-01-12 14:44 | Saga Musix | Relationship added | related to 0001012 |
2020-01-12 14:45 | Saga Musix | Relationship added | related to 0001030 |
2020-06-11 12:37 | manx | Assigned To | => manx |
2020-06-11 12:37 | manx | Status | new => acknowledged |
2020-06-11 12:37 | manx | Target Version | => OpenMPT 1.30.01.00 / libopenmpt 0.6.0 (upgrade first) |
2020-06-11 14:34 | manx | Note Added: 0004375 | |
2020-06-11 14:34 | manx | File Added: pluginbridge-legacy-v1.patch.7z | |
2020-06-11 14:34 | manx | Status | acknowledged => feedback |
2020-06-11 17:20 | manx | Note Added: 0004379 | |
2020-06-11 17:20 | manx | File Added: pluginbridge-legacy-v2.patch.7z | |
2020-06-13 16:38 | manx | Note Added: 0004384 | |
2020-06-13 16:38 | manx | File Added: pluginbridge-legacy-v3.patch.7z | |
2020-07-29 11:00 | manx | Note Added: 0004410 | |
2020-07-29 11:00 | manx | File Added: pluginbridge-legacy-v4.7z | |
2020-12-22 17:04 | manx | Relationship added | related to 0001401 |
2020-12-23 08:23 | manx | Relationship added | related to 0001402 |
2020-12-30 11:03 | manx | Note Added: 0004586 | |
2020-12-30 11:04 | manx | Note Added: 0004589 | |
2020-12-30 21:27 | jmkz | Note Added: 0004591 | |
2020-12-30 22:27 | Gerirish | Note Added: 0004592 | |
2020-12-30 22:28 | Saga Musix | Note Added: 0004593 | |
2020-12-30 22:28 | Saga Musix | Status | feedback => assigned |
2020-12-30 22:31 | Gerirish | Note Added: 0004594 | |
2020-12-30 22:32 | Saga Musix | Note Added: 0004595 | |
2020-12-30 22:33 | Gerirish | Note Added: 0004596 | |
2020-12-31 07:01 | manx | Status | assigned => confirmed |
2020-12-31 07:01 | manx | Note Added: 0004597 | |
2020-12-31 10:02 | manx | Note Added: 0004598 | |
2020-12-31 10:02 | manx | File Added: pluginbridge-fallback-to-legacy-v2.patch | |
2020-12-31 10:03 | manx | Assigned To | manx => Saga Musix |
2020-12-31 10:03 | manx | Status | confirmed => feedback |
2020-12-31 10:03 | manx | Note Added: 0004599 | |
2021-01-02 14:39 | Saga Musix | Note Added: 0004601 | |
2021-01-03 08:03 | manx | Note Added: 0004604 | |
2021-01-03 08:04 | manx | Assigned To | Saga Musix => manx |
2021-01-04 08:05 | jmkz | Note Added: 0004608 | |
2021-01-04 11:45 | Saga Musix | Note Added: 0004609 | |
2021-01-04 11:45 | Saga Musix | Status | feedback => assigned |
2021-01-04 21:09 | jmkz | Note Added: 0004610 | |
2021-01-04 21:10 | Saga Musix | Note Added: 0004611 | |
2021-01-10 18:28 | manx | Status | assigned => resolved |
2021-01-10 18:28 | manx | Resolution | open => fixed |
2021-01-10 18:28 | manx | Fixed in Version | => OpenMPT 1.30.01.00 / libopenmpt 0.6.0 (upgrade first) |