Pin connector refactor #2
Conversation
Will want to replace with C++20's std::span as soon as possible
The new implementation does not require a working buffer and requires much less copying, so it should be more efficient in terms of both space and time
|
Some developers had already proposed that using potentially different threads for different note play handles is overkill. This might even be changed when we make LMMS realtime safe. I assume in that case, thee instruments would be able to use |
Pin connector support can be added in later. I think it's best to hold off for now since Carla and Lv2 do not implement support for multiple plugin channels yet, which could complicate upgrading project files created between now and when that support is added. Also cleaned up the plugin headers, removing some unecessary changes
|
@JohannesLorenz Yes, I'm not aware of anything that would prevent |
| @@ -195,7 +189,7 @@ class CARLABASE_EXPORT CarlaInstrument : public Instrument | |||
| QString nodeName() const override; | |||
| void saveSettings(QDomDocument& doc, QDomElement& parent) override; | |||
| void loadSettings(const QDomElement& elem) override; | |||
| void play(SampleFrame* workingBuffer) override; | |||
| void playImpl(CoreAudioDataMut out) override; | |||
There was a problem hiding this comment.
Is there even a play function that would cause conflicts? If not, play would be shorter and possibly easier to understand for many (The name change also requires changing comments that referenced play(), e.g. the one above the fMutex variable.).
Even if this does conflict with play of a parent class, it might make more sense to rename that upper function and keep play in the final Plugin implementations consistent to the past.
It also looks strange because of asymmetry: saveSettings, loadSettings etc are also implementations, but not named ...Impl.
Major refactor of audio plugin implementations to add support for the pin connector to MIDI-based instrument plugins and all effect plugins. Also refactors parts of the pin connector itself.
The idea was to add a new class template called
AudioPluginInterfacewhich audio plugins can inherit from rather than inheriting directly fromInstrumentorEffect. This class template uses template parameters to allow compile-time customization for plugins (i.e. how many inputs, how many outputs, whether in-place processing is used, etc.) and is responsible for owning and using the pin connector and its routing capabilities.AudioPluginInterfaceis entirely optional - a plugin implementation could choose to continue inheriting fromInstrumentorEffectdirectly and all it would mean is no support for the pin connector.One of the challenges along the way was how to deal with different working buffers used by plugins. The size, data type, and data layout differ depending on plugin characteristics such as how many in/out channels there are, whether in-place processing is used, etc. But also, VSTs and ZynAddSubFX cannot just use any buffer - they use a shared memory buffer.
The solution I came up with is to introduce
AudioPluginBufferInterfacewhich usesAudioPluginInterface's compile-time customization options to customize its itself based on the needs of the plugin implementation.AudioPluginBufferDefaultImplis provided which implements the interface in a way that works for most plugins. For VST and ZynAddSubFX, they specifically request (via thecustomBufferoption) to implement the buffer themselves and provide the audio plugin interface access to it throughAudioPluginBufferInterfaceProvider. LV2, CLAP, Carla, and LADSPA could also take advantage of the custom buffers feature to reuse the same buffers they provide to their plugin APIs which would avoid extra memory and extra copying, though this work has not been done yet.While this system is as flexible, simple, and performant as I could make it, it is not thread-safe which leads to a consequence I didn't foresee -
NotePlayHandle-based instruments (which are all instruments except for Carla, CLAP, LV2, OpulenZ, Vestige, and ZynAddSubFX) are unable to useAudioPluginInterfacebecause all the note play handles will use the same working buffer concurrently which messes up the audio. However, this isn't much of an issue since allNotePlayHandle-based instruments have exactly two fixed audio output channels, and the main pin connector use case for instruments is to provide support for a variable number of output channels or sidechain inputs. Pin connector support forNotePlayHandle-based instruments could potentially be added in the future, though it probably wouldn't be worth it.This PR also adds unit tests for the pin connector.