Allow sub plugins for instruments aswell (LV2 requirement)#4745
Conversation
* Move m_key member of Effect into Plugin * Pass key to Instrument ctors and instantiaters * Add pluginKeys to all plugin selector widgets, and let them pass the keys when instantiating the instruments; or, if the keys must be passed over threads, pass the keys to the Engine using `Engine::setDndPluginKey()` * As instrument plugin libraries now also need to get their key passed, their second argument, which was always the same as the first, is now used to pass the sub plugin keys. This affects *all* instrument plugins. * Plugin.h: Add more virtuals to `SubPluginFeatures` in order to draw logos and images into instrument selector widgets * LadspaSubPluginFeatures: Implement the `displayName` virtual because the new behaviour to resolve displayNames is to first look at the SubPluginFeatures, which, without override, returns the superior plugin's name (Plugin.cpp) Additional: * PluginFactory.h: Allow setting up search paths without discovering plugins yet * Plugin.h: Add full documentation (should be checked)
|
Side-node: It compiles now on all CI platforms, and I could test it in a non-committed version of my SPA branch. However, without SPA/LV2, you won't see anything new, as we currently don't have instruments with sub plugins. |
The former virtuals returned `const char*`, which lead to invalid reads when `LadspaSubPluginFeatures` returned pointers to temporary `QByteArray::data`.
|
Not sure if this is necessarily what you're asking, but just wanna clarify-- By "sub plugin", would that mean nested VSTs? (meaning a vst which hosts other vsts from the same origin) |
| { | ||
| const ladspa_key_t & lkey = subPluginKeyToLadspaKey(&k); | ||
| Ladspa2LMMS * lm = Engine::getLADSPAManager(); | ||
| return lm->getName(lkey).toUtf8().data(); |
There was a problem hiding this comment.
This line is producing invalid reads. No wonder, as toUtf8() creates a temporary QByteArray, which will be freed before the pointer returned from displayName() is in use.
Probably, these functions need to return QString, unless anyone has a better idea.
Edit: Fixed in d5dcebe
No idea what nested VSTs are. E.g. LADSPA is a plugin, and CALF Mono Compressor is a LADSPA sub plugin. Or, VST is a plugin, and zynaddsubfx-vst is a sub plugin. However, I have no idea if this PR affects VST. I only did this for Lv2 and SPA. |
* Fix possible segfault in `SubPluginFeatures::displayName` * Minor fixes
JohannesLorenz
force-pushed
the
instr-sub-plugins
branch
from
f6726c4 to
3483581
Compare
|
Many thanks @Reflexe for reviewing. LMMS can now handle masses of instruments 😃 |
3 participants
For LADSPA, we only need sub plugins for effects, but for LV2 (and SPA), we will also need them for instruments. At least, assuming we want to keep using the current sub plugin structure. That's what this patch does.
Some notes for reviewers:
As this is an LV2 requirement and has influence on large parts of the core, I'd be really thankful if someone tries to review this. ❤️