From 39984ad442aa29e45da48533aff38a50f4bf4f65 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 12 Dec 2020 21:51:06 +0100 Subject: [PATCH] Use the new approach for creating plugin factory Directly serializing and deserializing into objects was and more boilerplate heavy (since we now need two implementations even though we only use one), and also much less flexible because we can't wrap payloads in structs or provide optional values that way. --- meson.build | 1 - src/common/logging/vst3.cpp | 29 ++-- src/common/logging/vst3.h | 4 +- src/common/serialization/vst3.h | 13 +- src/common/serialization/vst3/README.md | 11 +- .../serialization/vst3/plugin-factory.cpp | 42 ++--- .../serialization/vst3/plugin-factory.h | 151 +++++++++++------- .../bridges/vst3-impls/plugin-factory.cpp | 6 +- .../bridges/vst3-impls/plugin-factory.h | 3 +- src/plugin/bridges/vst3.cpp | 10 +- src/wine-host/bridges/vst3-impls.cpp | 35 ---- src/wine-host/bridges/vst3-impls.h | 31 ---- src/wine-host/bridges/vst3.cpp | 12 +- src/wine-host/bridges/vst3.h | 7 - 14 files changed, 151 insertions(+), 204 deletions(-) delete mode 100644 src/wine-host/bridges/vst3-impls.cpp delete mode 100644 src/wine-host/bridges/vst3-impls.h diff --git a/meson.build b/meson.build index c3b08eec..5d12b3d5 100644 --- a/meson.build +++ b/meson.build @@ -115,7 +115,6 @@ if with_vst3 'src/common/serialization/vst3/component.cpp', 'src/common/serialization/vst3/plugin-factory.cpp', 'src/wine-host/bridges/vst3.cpp', - 'src/wine-host/bridges/vst3-impls.cpp', ] endif diff --git a/src/common/logging/vst3.cpp b/src/common/logging/vst3.cpp index 0c809c80..21b50249 100644 --- a/src/common/logging/vst3.cpp +++ b/src/common/logging/vst3.cpp @@ -45,18 +45,18 @@ void Vst3Logger::log_request(bool is_host_vst, }); } +void Vst3Logger::log_request(bool is_host_vst, + const YaPluginFactory::Construct&) { + log_request_base(is_host_vst, + [](auto& message) { message << "GetPluginFactory()"; }); +} + void Vst3Logger::log_request(bool is_host_vst, const WantsConfiguration&) { log_request_base(is_host_vst, [](auto& message) { message << "Requesting "; }); } -void Vst3Logger::log_request(bool is_host_vst, const WantsPluginFactory&) { - log_request_base(is_host_vst, [](auto& message) { - message << "Requesting "; - }); -} - void Vst3Logger::log_response(bool is_host_vst, const Ack&) { log_response_base(is_host_vst, [&](auto& message) { message << "ACK"; }); } @@ -76,16 +76,15 @@ void Vst3Logger::log_response( }); } +void Vst3Logger::log_response(bool is_host_vst, + const YaPluginFactory::ConstructArgs& args) { + log_response_base(is_host_vst, [&](auto& message) { + message << " with " << args.num_classes + << " registered classes"; + }); +} + void Vst3Logger::log_response(bool is_host_vst, const Configuration&) { log_response_base(is_host_vst, [](auto& message) { message << " with " - << const_cast(factory).countClasses() - << " registered classes"; - }); -} diff --git a/src/common/logging/vst3.h b/src/common/logging/vst3.h index cac56886..f853cacb 100644 --- a/src/common/logging/vst3.h +++ b/src/common/logging/vst3.h @@ -50,15 +50,15 @@ class Vst3Logger { void log_request(bool is_host_vst, const YaComponent::Construct&); void log_request(bool is_host_vst, const YaComponent::Destruct&); void log_request(bool is_host_vst, const YaComponent::Terminate&); + void log_request(bool is_host_vst, const YaPluginFactory::Construct&); void log_request(bool is_host_vst, const WantsConfiguration&); - void log_request(bool is_host_vst, const WantsPluginFactory&); void log_response(bool is_host_vst, const Ack&); void log_response( bool is_host_vst, const std::variant&); + void log_response(bool is_host_vst, const YaPluginFactory::ConstructArgs&); void log_response(bool is_host_vst, const Configuration&); - void log_response(bool is_host_vst, const YaPluginFactory&); Logger& logger; diff --git a/src/common/serialization/vst3.h b/src/common/serialization/vst3.h index 5dde5922..06c43977 100644 --- a/src/common/serialization/vst3.h +++ b/src/common/serialization/vst3.h @@ -52,17 +52,6 @@ struct WantsConfiguration { void serialize(S&) {} }; -/** - * Marker struct to indicate the other side (the Wine plugin host) should send a - * copy of the hosted Windows VST3 plugin's `IPluginFactory{,2,3}` interface. - */ -struct WantsPluginFactory { - using Response = YaPluginFactory&; - - template - void serialize(S&) {} -}; - /** * When we send a control message from the plugin to the Wine VST host, this * encodes the information we request or the operation we want to perform. A @@ -71,7 +60,7 @@ struct WantsPluginFactory { using ControlRequest = std::variant; + YaPluginFactory::Construct>; template void serialize(S& s, ControlRequest& payload) { diff --git a/src/common/serialization/vst3/README.md b/src/common/serialization/vst3/README.md index 7056c51b..10742a63 100644 --- a/src/common/serialization/vst3/README.md +++ b/src/common/serialization/vst3/README.md @@ -3,12 +3,6 @@ TODO: Once this is more fleshed out, move this document to `docs/`, and perhaps replace this readme with a link to that document. -TODO: There are now two approaches in use: the factory takes an interface -pointer for serialization and deserializes into an object directly, and the -component uses an args struct because the alternative involving pointers is just -too unsafe (as we also have to communicate additional payload data). This should -probably be unified into only using the latter appraoch. - The VST3 SDK uses an architecture where every object inherits from an interface, and every interface inherits from `FUnknown` which offers a dynamic casting interface through `queryInterface()`. Every interface gets a unique identifier. @@ -30,6 +24,11 @@ Yabridge's serialization and communication model for VST3 is thus a lot more complicated than for VST2 since all of these objects are loosely coupled and are instantiated and managed by the host. The model works as follows: +TODO: This is now slightly out of date. Instead of serializing and deserializing +directly into interface implementations through references, we now only pass +structs with payload data around to make the receiving process much more +flexible. + 1. For an interface `IFoo`, we provide a possibly abstract implementation called `YaFoo`. 2. This class has a constructor that takes an `IPtr` interface pointer and diff --git a/src/common/serialization/vst3/plugin-factory.cpp b/src/common/serialization/vst3/plugin-factory.cpp index 1465a82b..831c1c36 100644 --- a/src/common/serialization/vst3/plugin-factory.cpp +++ b/src/common/serialization/vst3/plugin-factory.cpp @@ -21,12 +21,10 @@ #include -YaPluginFactory::YaPluginFactory(){FUNKNOWN_CTOR} +YaPluginFactory::ConstructArgs::ConstructArgs() {} -YaPluginFactory::YaPluginFactory( +YaPluginFactory::ConstructArgs::ConstructArgs( Steinberg::IPtr factory) { - FUNKNOWN_CTOR - known_iids.insert(factory->iid); // `IPluginFactory::getFactoryInfo` if (Steinberg::PFactoryInfo info; @@ -75,7 +73,11 @@ YaPluginFactory::YaPluginFactory( } } -YaPluginFactory::~YaPluginFactory() { +YaPluginFactory::YaPluginFactory(const ConstructArgs&& args) + : arguments(std::move(args)){FUNKNOWN_CTOR} + + // clang-format just doesn't understand these macros, I guess + YaPluginFactory::~YaPluginFactory() { FUNKNOWN_DTOR } @@ -88,15 +90,15 @@ tresult PLUGIN_API YaPluginFactory::queryInterface(Steinberg::FIDString _iid, void** obj) { QUERY_INTERFACE(_iid, obj, Steinberg::FUnknown::iid, Steinberg::IPluginFactory) - if (known_iids.contains(Steinberg::IPluginFactory::iid)) { + if (arguments.known_iids.contains(Steinberg::IPluginFactory::iid)) { QUERY_INTERFACE(_iid, obj, Steinberg::IPluginFactory::iid, Steinberg::IPluginFactory) } - if (known_iids.contains(Steinberg::IPluginFactory2::iid)) { + if (arguments.known_iids.contains(Steinberg::IPluginFactory2::iid)) { QUERY_INTERFACE(_iid, obj, Steinberg::IPluginFactory2::iid, Steinberg::IPluginFactory2) } - if (known_iids.contains(Steinberg::IPluginFactory3::iid)) { + if (arguments.known_iids.contains(Steinberg::IPluginFactory3::iid)) { QUERY_INTERFACE(_iid, obj, Steinberg::IPluginFactory3::iid, Steinberg::IPluginFactory3) } @@ -107,8 +109,8 @@ tresult PLUGIN_API YaPluginFactory::queryInterface(Steinberg::FIDString _iid, tresult PLUGIN_API YaPluginFactory::getFactoryInfo(Steinberg::PFactoryInfo* info) { - if (info && factory_info) { - *info = *factory_info; + if (info && arguments.factory_info) { + *info = *arguments.factory_info; return Steinberg::kResultOk; } else { return Steinberg::kNotInitialized; @@ -116,17 +118,17 @@ YaPluginFactory::getFactoryInfo(Steinberg::PFactoryInfo* info) { } int32 PLUGIN_API YaPluginFactory::countClasses() { - return num_classes; + return arguments.num_classes; } tresult PLUGIN_API YaPluginFactory::getClassInfo(Steinberg::int32 index, Steinberg::PClassInfo* info) { - if (index >= static_cast(class_infos_unicode.size())) { + if (index >= static_cast(arguments.class_infos_unicode.size())) { return Steinberg::kInvalidArgument; } - if (class_infos_1[index]) { - *info = *class_infos_1[index]; + if (arguments.class_infos_1[index]) { + *info = *arguments.class_infos_1[index]; return Steinberg::kResultOk; } else { return Steinberg::kResultFalse; @@ -135,12 +137,12 @@ tresult PLUGIN_API YaPluginFactory::getClassInfo(Steinberg::int32 index, tresult PLUGIN_API YaPluginFactory::getClassInfo2(int32 index, Steinberg::PClassInfo2* info) { - if (index >= static_cast(class_infos_1.size())) { + if (index >= static_cast(arguments.class_infos_1.size())) { return Steinberg::kInvalidArgument; } - if (class_infos_2[index]) { - *info = *class_infos_2[index]; + if (arguments.class_infos_2[index]) { + *info = *arguments.class_infos_2[index]; return Steinberg::kResultOk; } else { return Steinberg::kResultFalse; @@ -150,12 +152,12 @@ YaPluginFactory::getClassInfo2(int32 index, Steinberg::PClassInfo2* info) { tresult PLUGIN_API YaPluginFactory::getClassInfoUnicode(int32 index, Steinberg::PClassInfoW* info) { - if (index >= static_cast(class_infos_unicode.size())) { + if (index >= static_cast(arguments.class_infos_unicode.size())) { return Steinberg::kInvalidArgument; } - if (class_infos_unicode[index]) { - *info = *class_infos_unicode[index]; + if (arguments.class_infos_unicode[index]) { + *info = *arguments.class_infos_unicode[index]; return Steinberg::kResultOk; } else { return Steinberg::kResultFalse; diff --git a/src/common/serialization/vst3/plugin-factory.h b/src/common/serialization/vst3/plugin-factory.h index 5815e084..118eb305 100644 --- a/src/common/serialization/vst3/plugin-factory.h +++ b/src/common/serialization/vst3/plugin-factory.h @@ -28,8 +28,6 @@ // TODO: After implementing one or two more of these, abstract away some of the // nasty bits -// TODO: Should we have some clearer way to indicate to us which fields are -// going to return copied results directly and which make a callback? #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wnon-virtual-dtor" @@ -40,16 +38,99 @@ */ class YaPluginFactory : public Steinberg::IPluginFactory3 { public: - YaPluginFactory(); + /** + * These are the arguments for creating a `YaPluginFactoryPluginImpl`. + */ + struct ConstructArgs { + ConstructArgs(); + + /** + * Create a copy of an existing plugin factory. Depending on the + * supported interface function more or less of this struct will be left + * empty, and `iid` will be set accordingly. + */ + ConstructArgs(Steinberg::IPtr factory); + + /** + * The IIDs that the interface we serialized supports. + */ + std::set known_iids; + + /** + * For `IPluginFactory::getFactoryInfo`. + */ + std::optional factory_info; + + /** + * For `IPluginFactory::countClasses`. + */ + int num_classes; + + /** + * For `IPluginFactory::getClassInfo`. We need to store all four class + * info versions if the plugin can provide them since we don't know + * which version of the interface the host will use. Will be + * `std::nullopt` if the plugin doesn't return a class info. + */ + std::vector> class_infos_1; + + /** + * For `IPluginFactory2::getClassInfo2`, works the same way as the + * above. + */ + std::vector> class_infos_2; + + /** + * For `IPluginFactory3::getClassInfoUnicode`, works the same way as the + * above. + */ + std::vector> class_infos_unicode; + + template + void serialize(S& s) { + s.ext(known_iids, bitsery::ext::StdSet{32}, + [](S& s, Steinberg::FUID& iid) { + s.ext(iid, bitsery::ext::FUID{}); + }); + s.ext(factory_info, bitsery::ext::StdOptional{}); + s.value4b(num_classes); + s.container(class_infos_1, 2048, + [](S& s, std::optional& info) { + s.ext(info, bitsery::ext::StdOptional{}); + }); + s.container(class_infos_2, 2048, + [](S& s, std::optional& info) { + s.ext(info, bitsery::ext::StdOptional{}); + }); + s.container(class_infos_unicode, 2048, + [](S& s, std::optional& info) { + s.ext(info, bitsery::ext::StdOptional{}); + }); + } + }; /** - * Create a copy of an existing plugin factory. Depending on the supported - * interface function more or less of this struct will be left empty, and - * `iid` will be set accordingly. + * Message to request the `IPluginFactory{,2,3}`'s information from the Wine + * plugin host. */ - explicit YaPluginFactory( - Steinberg::IPtr factory); + struct Construct { + using Response = ConstructArgs; + template + void serialize(S&) {} + }; + + /** + * Instantiate this instance with arguments read from the Windows VST3 + * plugin's plugin factory. + */ + YaPluginFactory(const ConstructArgs&& args); + + /** + * We do not need to implement the destructor, since when the sockets are + * closed, RAII will clean up the Windows VST3 module we loaded along with + * its factory for us. + */ virtual ~YaPluginFactory(); DECLARE_FUNKNOWN_METHODS @@ -81,58 +162,8 @@ class YaPluginFactory : public Steinberg::IPluginFactory3 { virtual tresult PLUGIN_API setHostContext(Steinberg::FUnknown* context) override = 0; - template - void serialize(S& s) { - s.ext(known_iids, bitsery::ext::StdSet{32}, - [](S& s, Steinberg::FUID& iid) { - s.ext(iid, bitsery::ext::FUID{}); - }); - s.ext(factory_info, bitsery::ext::StdOptional{}); - s.value4b(num_classes); - s.container(class_infos_1, 2048, - [](S& s, std::optional& info) { - s.ext(info, bitsery::ext::StdOptional{}); - }); - s.container(class_infos_2, 2048, - [](S& s, std::optional& info) { - s.ext(info, bitsery::ext::StdOptional{}); - }); - s.container(class_infos_unicode, 2048, - [](S& s, std::optional& info) { - s.ext(info, bitsery::ext::StdOptional{}); - }); - } - - private: - /** - * The IIDs that the interface we serialized supports. - */ - std::set known_iids; - - /** - * For `IPluginFactory::getFactoryInfo`. - */ - std::optional factory_info; - /** - * For `IPluginFactory::countClasses`. - */ - int num_classes; - /** - * For `IPluginFactory::getClassInfo`. We need to store all four class info - * versions if the plugin can provide them since we don't know which version - * of the interface the host will use. Will be `std::nullopt` if the plugin - * doesn't return a class info. - */ - std::vector> class_infos_1; - /** - * For `IPluginFactory2::getClassInfo2`, works the same way as the above. - */ - std::vector> class_infos_2; - /** - * For `IPluginFactory3::getClassInfoUnicode`, works the same way as the - * above. - */ - std::vector> class_infos_unicode; + protected: + ConstructArgs arguments; }; #pragma GCC diagnostic pop diff --git a/src/plugin/bridges/vst3-impls/plugin-factory.cpp b/src/plugin/bridges/vst3-impls/plugin-factory.cpp index 4e5c6095..123e960b 100644 --- a/src/plugin/bridges/vst3-impls/plugin-factory.cpp +++ b/src/plugin/bridges/vst3-impls/plugin-factory.cpp @@ -20,8 +20,10 @@ #include "component.h" -YaPluginFactoryPluginImpl::YaPluginFactoryPluginImpl(Vst3PluginBridge& bridge) - : bridge(bridge) {} +YaPluginFactoryPluginImpl::YaPluginFactoryPluginImpl( + Vst3PluginBridge& bridge, + YaPluginFactory::ConstructArgs&& args) + : YaPluginFactory(std::move(args)), bridge(bridge) {} tresult PLUGIN_API YaPluginFactoryPluginImpl::createInstance(Steinberg::FIDString cid, diff --git a/src/plugin/bridges/vst3-impls/plugin-factory.h b/src/plugin/bridges/vst3-impls/plugin-factory.h index 8d877640..aee5a149 100644 --- a/src/plugin/bridges/vst3-impls/plugin-factory.h +++ b/src/plugin/bridges/vst3-impls/plugin-factory.h @@ -20,7 +20,8 @@ class YaPluginFactoryPluginImpl : public YaPluginFactory { public: - YaPluginFactoryPluginImpl(Vst3PluginBridge& bridge); + YaPluginFactoryPluginImpl(Vst3PluginBridge& bridge, + YaPluginFactory::ConstructArgs&& args); tresult PLUGIN_API createInstance(Steinberg::FIDString cid, Steinberg::FIDString _iid, diff --git a/src/plugin/bridges/vst3.cpp b/src/plugin/bridges/vst3.cpp index 9a16cdf6..7c1073a3 100644 --- a/src/plugin/bridges/vst3.cpp +++ b/src/plugin/bridges/vst3.cpp @@ -97,10 +97,12 @@ Steinberg::IPluginFactory* Vst3PluginBridge::get_plugin_factory() { // will request after loading the module. Host callback handlers should // have started before this since the Wine plugin host will request a // copy of the configuration during its initialization. - plugin_factory = new YaPluginFactoryPluginImpl(*this); - sockets.host_vst_control.receive_into( - WantsPluginFactory{}, *plugin_factory, - std::pair(logger, true)); + YaPluginFactory::ConstructArgs factory_args = + sockets.host_vst_control.send_message( + YaPluginFactory::Construct{}, + std::pair(logger, true)); + plugin_factory = + new YaPluginFactoryPluginImpl(*this, std::move(factory_args)); } return plugin_factory; diff --git a/src/wine-host/bridges/vst3-impls.cpp b/src/wine-host/bridges/vst3-impls.cpp deleted file mode 100644 index d52d8cc6..00000000 --- a/src/wine-host/bridges/vst3-impls.cpp +++ /dev/null @@ -1,35 +0,0 @@ -// yabridge: a Wine VST bridge -// Copyright (C) 2020 Robbert van der Helm -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -#include "vst3-impls.h" - -YaPluginFactoryHostImpl::YaPluginFactoryHostImpl( - Steinberg::IPtr factory) - : YaPluginFactory(factory) {} - -tresult PLUGIN_API -YaPluginFactoryHostImpl::createInstance(Steinberg::FIDString /*cid*/, - Steinberg::FIDString /*_iid*/, - void** /*obj*/) { - throw std::runtime_error( - "Unexpected call to 'IPluginFactory::createInstance()'"); -} - -tresult PLUGIN_API -YaPluginFactoryHostImpl::setHostContext(Steinberg::FUnknown* /*context*/) { - throw std::runtime_error( - "Unexpected call to 'IPluginFactory3::setHostContext()'"); -} diff --git a/src/wine-host/bridges/vst3-impls.h b/src/wine-host/bridges/vst3-impls.h deleted file mode 100644 index 59328696..00000000 --- a/src/wine-host/bridges/vst3-impls.h +++ /dev/null @@ -1,31 +0,0 @@ -// yabridge: a Wine VST bridge -// Copyright (C) 2020 Robbert van der Helm -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -#pragma once - -#include "vst3.h" - -class YaPluginFactoryHostImpl : public YaPluginFactory { - public: - YaPluginFactoryHostImpl(Steinberg::IPtr factory); - - tresult PLUGIN_API createInstance(Steinberg::FIDString cid, - Steinberg::FIDString _iid, - void** obj) override; - - tresult PLUGIN_API - setHostContext(Steinberg::FUnknown* /*context*/) override; -}; diff --git a/src/wine-host/bridges/vst3.cpp b/src/wine-host/bridges/vst3.cpp index 8fcc2318..02ed53fd 100644 --- a/src/wine-host/bridges/vst3.cpp +++ b/src/wine-host/bridges/vst3.cpp @@ -17,7 +17,6 @@ #include "vst3.h" #include "../boost-fix.h" -#include "vst3-impls.h" #include @@ -36,11 +35,6 @@ Vst3Bridge::Vst3Bridge(MainContext& main_context, sockets.connect(); - // Serialize the plugin's plugin factory. The native VST3 plugin will - // request a copy of this during its initialization. - plugin_factory = - std::make_unique(module->getFactory().get()); - // Fetch this instance's configuration from the plugin to finish the setup // process config = sockets.vst_host_callback.send_message(WantsConfiguration{}, @@ -82,8 +76,10 @@ void Vst3Bridge::run() { -> YaComponent::Terminate::Response { return component_instances[request.instance_id]->terminate(); }, - [&](const WantsPluginFactory&) -> WantsPluginFactory::Response { - return *plugin_factory; + [&](const YaPluginFactory::Construct&) + -> YaPluginFactory::Construct::Response { + return YaPluginFactory::ConstructArgs( + module->getFactory().get()); }}); } diff --git a/src/wine-host/bridges/vst3.h b/src/wine-host/bridges/vst3.h index 161e030c..7c638e88 100644 --- a/src/wine-host/bridges/vst3.h +++ b/src/wine-host/bridges/vst3.h @@ -91,13 +91,6 @@ class Vst3Bridge : public HostBridge { */ Vst3Sockets sockets; - /** - * A plugin factory copied from the Windows VST3 plugin during - * initialization. The native VST3 plugin will request a copy of this - * information during its initialization. - */ - std::unique_ptr plugin_factory; - /** * Used to assign unique identifier to instances created for * `IPluginFactory::createInstance()`.