AUv3 & ExternalPlugin on iOS
Closed this issue · 12 comments
Hi!
AUv3 plugins, on iOS, need to be instantiated asynchronically.
Right now, ExternalPlugin::doFullInitialisation() will end up calling JUCE's PluginFormatManager::createPluginInstance(), which will immediately return with the following error: "This plug-in cannot be instantiated synchronously".
It is possible to assign a specific lambda for custom instantiation on the global PluginManager (via the createPluginInstance variable), however, since this call is initially encapsulated in a callBlocking lambda, which internally relies on a juce::WaitableEvent, and since PluginFormatManager::createPluginInstanceAsync also uses a juce::WaitableEvent, the code is never triggered because of these competing blocking mechanisms.
After some initial research, it seems that the best approach would be to patch ExternalPlugin instantiation/initialization code directly so it can create instances asynchronically and resume once it's ready.
Before diving in, I'm just checking in with the team to see if it's maybe already in the backlog or, alternatively, to gather implementation ideas before submitting a pull request.
Thanks in advance,
Mathieu.
Update: well, that was quick. I am delaying the initialization after the instance is returned, and this works perfectly. Still happy to hear your ideas so I can improve the code before submitting it.
Can you show me what changes you need to make? Presumably if a juce::AudioPluginFormatManager::createPluginInstance returns a nullptr instance, you just asynchronously call te::ExternalPlugin::doFullInitialisation?
Tbh I've not had time to think about this in much detail.
There's a bit more involved as doFullInitialisation() is initially called from the ctor (via initialiseFully()), so I added a simple mechanism to postpone and resume the initialisation once the instance is returned, in the lambda passed by juce::AudioPluginFormatManager::createPluginInstanceAsync. I also manually call ExternalPlugin::initialise() as the instance could be returned after the engine initially calls this method.
Here's the patch as of now - more testing is needed but so far it works nicely:
diff --git a/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.cpp b/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.cpp
index 4df1c907..44fb7bee 100644
--- a/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.cpp
+++ b/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.cpp
@@ -819,19 +819,40 @@ void ExternalPlugin::doFullInitialisation()
identiferString = createIdentifierString (desc);
updateDebugName();
- if (processing && pluginInstance == nullptr && edit.shouldLoadPlugins())
+ if ((processing && pluginInstance == nullptr && edit.shouldLoadPlugins()) ||
+ waitingForInstance)
{
if (isDisabled())
return;
CRASH_TRACER_PLUGIN (getDebugName());
juce::String error;
-
+
+ #if JUCE_IOS
+ // AUv3 requires async instantiation on iOS
+ if (isAU())
+ {
+ if (!waitingForInstance)
+ {
+ waitingForInstance = true;
+ createPluginInstanceAsync (*foundDesc);
+ return;
+ }
+ if (!asyncError.isEmpty())
+ {
+ error = asyncError;
+ asyncError = {};
+ }
+ jassert (pluginInstance);
+ waitingForInstance = false;
+ }
+ #else
callBlocking ([this, &error, &foundDesc]
{
CRASH_TRACER_PLUGIN (getDebugName());
error = createPluginInstance (*foundDesc);
});
+ #endif
if (pluginInstance != nullptr)
{
@@ -1280,7 +1301,7 @@ void ExternalPlugin::applyToBuffer (const PluginRenderContext& fc)
{
const bool processedBypass = fc.allowBypassedProcessing && ! isEnabled();
- if (pluginInstance != nullptr && (processedBypass || isEnabled()))
+ if (pluginInstance != nullptr && (processedBypass || isEnabled()) && !waitingForInstance)
{
CRASH_TRACER_PLUGIN (getDebugName());
const juce::ScopedLock sl (lock);
@@ -1693,6 +1714,31 @@ juce::String ExternalPlugin::createPluginInstance (const juce::PluginDescription
return error;
}
+
+void ExternalPlugin::createPluginInstanceAsync (const juce::PluginDescription& description)
+{
+ jassert (! pluginInstance); // This should have already been deleted!
+
+ auto& dm = engine.getDeviceManager();
+
+ fullyInitialised = false;
+
+ juce::String error;
+ engine.getPluginManager().pluginFormatManager
+ .createPluginInstanceAsync (description, dm.getSampleRate(), dm.getBlockSize(),
+ [&](auto _instance, const auto& err) -> void {
+ pluginInstance = std::move(_instance);
+ asyncError = err;
+
+ if (pluginInstance != nullptr)
+ {
+ pluginInstance->enableAllBuses();
+ processorChangedManager = std::make_unique<ProcessorChangedManager> (*this);
+ initialiseFully();
+ initialise({ 0, sampleRate, blockSizeSamples });
+ }
+ });
+}
void ExternalPlugin::deletePluginInstance()
{
diff --git a/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.h b/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.h
index f53b16ba..7bdfddd2 100644
--- a/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.h
+++ b/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.h
@@ -147,7 +147,9 @@ private:
std::unique_ptr<PluginPlayHead> playhead;
bool fullyInitialised = false, supportsMPE = false, isFlushingLayoutToState = false;
-
+ bool waitingForInstance = false;
+ juce::String asyncError;
+
struct MPEChannelRemapper;
std::unique_ptr<MPEChannelRemapper> mpeRemapper;
@@ -157,6 +159,7 @@ private:
//==============================================================================
juce::String createPluginInstance (const juce::PluginDescription&);
+ void createPluginInstanceAsync (const juce::PluginDescription&);
void deletePluginInstance();
//==============================================================================
Just wanted to bump this PR - any chance it could be merged? Seems quite relevant to us current users ..
@seclorum I probably need to double check everything again, especially de-initialization of plugins. That said, I agree, this would be a great addition in the current develop branch when everything is throughly tested.
Have you already tried the diff ?
Here's the patch merged for the latest develop branch (f83c850)
Gist : https://gist.github.com/mathieugarcia/b3d4cd3aa71db340b052728281c143e8
diff --git a/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.cpp b/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.cpp
index 46d380c2b3..b490fabdb1 100644
--- a/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.cpp
+++ b/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.cpp
@@ -823,19 +823,40 @@ void ExternalPlugin::doFullInitialisation()
identiferString = createIdentifierString (desc);
updateDebugName();
- if (processing && pluginInstance == nullptr && engine.getEngineBehaviour().shouldLoadPlugin (*this))
+ if ((processing && pluginInstance == nullptr && engine.getEngineBehaviour().shouldLoadPlugin (*this)) ||
+ waitingForInstance)
{
if (isDisabled())
return;
CRASH_TRACER_PLUGIN (getDebugName());
loadError = {};
-
+
+ #if JUCE_IOS
+ // AUv3 requires async instantiation on iOS
+ if (isAU())
+ {
+ if (!waitingForInstance)
+ {
+ waitingForInstance = true;
+ createPluginInstanceAsync (*foundDesc);
+ return;
+ }
+ if (!asyncError.isEmpty())
+ {
+ loadError = asyncError;
+ asyncError = {};
+ }
+ jassert (pluginInstance);
+ waitingForInstance = false;
+ }
+ #else
callBlocking ([this, &foundDesc]
{
CRASH_TRACER_PLUGIN (getDebugName());
loadError = createPluginInstance (*foundDesc);
});
+ #endif
if (pluginInstance != nullptr)
{
@@ -1301,7 +1322,7 @@ void ExternalPlugin::applyToBuffer (const PluginRenderContext& fc)
{
const bool processedBypass = fc.allowBypassedProcessing && ! isEnabled();
- if (pluginInstance != nullptr && (processedBypass || isEnabled()))
+ if (pluginInstance != nullptr && (processedBypass || isEnabled()) && !waitingForInstance)
{
CRASH_TRACER_PLUGIN (getDebugName());
const juce::ScopedLock sl (lock);
@@ -1715,6 +1736,31 @@ juce::String ExternalPlugin::createPluginInstance (const juce::PluginDescription
return error;
}
+void ExternalPlugin::createPluginInstanceAsync (const juce::PluginDescription& description)
+{
+ jassert (! pluginInstance); // This should have already been deleted!
+
+ auto& dm = engine.getDeviceManager();
+
+ fullyInitialised = false;
+
+ juce::String error;
+ engine.getPluginManager().pluginFormatManager
+ .createPluginInstanceAsync (description, dm.getSampleRate(), dm.getBlockSize(),
+ [&](auto _instance, const auto& err) -> void {
+ pluginInstance = std::move(_instance);
+ asyncError = err;
+
+ if (pluginInstance != nullptr)
+ {
+ pluginInstance->enableAllBuses();
+ processorChangedManager = std::make_unique<ProcessorChangedManager> (*this);
+ initialiseFully();
+ initialise({ 0_tp, sampleRate, blockSizeSamples });
+ }
+ });
+}
+
void ExternalPlugin::deletePluginInstance()
{
processorChangedManager.reset();
diff --git a/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.h b/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.h
index 450473f5d0..5b050a16d8 100644
--- a/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.h
+++ b/modules/tracktion_engine/plugins/external/tracktion_ExternalPlugin.h
@@ -150,6 +150,8 @@ private:
std::unique_ptr<PluginPlayHead> playhead;
bool fullyInitialised = false, supportsMPE = false, isFlushingLayoutToState = false;
+ bool waitingForInstance = false;
+ juce::String asyncError;
struct MPEChannelRemapper;
std::unique_ptr<MPEChannelRemapper> mpeRemapper;
@@ -160,6 +162,7 @@ private:
//==============================================================================
juce::String createPluginInstance (const juce::PluginDescription&);
+ void createPluginInstanceAsync (const juce::PluginDescription&);
void deletePluginInstance();
//==============================================================================
I am finishing up the AUv3 implementation in my app so I will perform more testing in the upcoming days.
With this patch, AUv3 plugins works on iOS.
Cheers!
Thanks for the work on this. There's a couple of things that would probably need to change in order for me to be able to merge it though.
Firstly, it's iOS specific, it would need to be able to correctly identify any plugin that need async initialisation, probably by querying requiresUnblockedMessageThreadDuringCreation.
Secondly, I'm not sure having a flag that holds the state if its waiting for a completion callback is the best idea. It seems like it could be a bit error prone. I'd probably think of it more like an async/await function but in current C++ split the init function in to two, startPluginInitialisation and completePluginInitialisation and have the non-async version complete both steps sequentially. That way there's less difference between the two.
Hey @drowaudio, thanks for chiming in.
Let me try these changes so it complies with your standards -- I agree about the flag being error prone.
Cheers !
Hi!
Some news.
Unfortunately, after some research, it's not possible to wait for the asynchronous instantiation of the plugin before resuming initialization. Mainly because the main thread shouldn't block while PluginFormatManager::createPluginInstanceAsync() runs (also, this functions uses a WaitableEvent internally already).
Furthermore, after reading back and forth the ExternalPlugin code, it seems it would require a more complex change. Right now, the instance is created right from the constructor. Further initialization happens within PluginNode as well, which calls ExternalPlugin::initialise, but in our case, when this function is called, the plugin instance isn't ready yet.
Another possibility would be to pass the plugin instance directly to the ExternalPlugin ctor; this could maybe happen via an async version of PluginManager::createNewPlugin() which would take care of the instantiation of the plugin and the ExternalPlugin, and would require a completion lambda to retrieve the Plugin::Ptr. Just an idea!
Cheers!
More news.
I was able to work around the PluginNode initialisation issue by manually calling baseClassInitialise() and initialise(). This seems a little bit hacky, but after testing it thoroughly it seems to work nicely.
I have the pull request ready for review, if you want, @drowaudio !
Cheers!
Can we close this issue can carry it on in the PR? It will be easier to discuss it relating to the code.
I think with the PluginNode issue you mention, rather than manually calling baseClassInitialise, and initialise, the plugin should call edit.restartPlayback() when the instance is created which will rebuild the graph, create a new PluginNode and do the initialisation. Would that work?
Closing this issue and bringing the edit.restartPlayback() conversation on the PR.
Thanks!