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

@@ -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
 

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 <Configuration>";
     });
 }
 
-void Vst3Logger::log_request(bool is_host_vst, const WantsPluginFactory&) {
-    log_request_base(is_host_vst, [](auto& message) {
-        message << "Requesting <IPluginFactory*>";
-    });
-}
-
 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 << "<IPluginFactory*> 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 << "<Configuration"; });
 }
-
-void Vst3Logger::log_response(bool is_host_vst,
-                              const YaPluginFactory& factory) {
-    log_response_base(is_host_vst, [&](auto& message) {
-        message << "<IPluginFactory*> with "
-                << const_cast<YaPluginFactory&>(factory).countClasses()
-                << " registered classes";
-    });
-}

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<YaComponent::ConstructArgs, UniversalTResult>&);
+    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;
 

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 <typename S>
-    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<YaComponent::Construct,
                                     YaComponent::Destruct,
                                     YaComponent::Terminate,
-                                    WantsPluginFactory>;
+                                    YaPluginFactory::Construct>;
 
 template <typename S>
 void serialize(S& s, ControlRequest& payload) {

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<IFoo>` interface pointer and

src/common/serialization/vst3/plugin-factory.cpp

@@ -21,12 +21,10 @@
 
 #include <public.sdk/source/vst/utility/stringconvert.h>
 
-YaPluginFactory::YaPluginFactory(){FUNKNOWN_CTOR}
+YaPluginFactory::ConstructArgs::ConstructArgs() {}
 
-YaPluginFactory::YaPluginFactory(
+YaPluginFactory::ConstructArgs::ConstructArgs(
     Steinberg::IPtr<Steinberg::IPluginFactory> 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<int32>(class_infos_unicode.size())) {
+    if (index >= static_cast<int32>(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<int32>(class_infos_1.size())) {
+    if (index >= static_cast<int32>(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<int32>(class_infos_unicode.size())) {
+    if (index >= static_cast<int32>(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;

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<Steinberg::IPluginFactory> factory);
+
+        /**
+         * The IIDs that the interface we serialized supports.
+         */
+        std::set<Steinberg::FUID> known_iids;
+
+        /**
+         * For `IPluginFactory::getFactoryInfo`.
+         */
+        std::optional<Steinberg::PFactoryInfo> 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<std::optional<Steinberg::PClassInfo>> class_infos_1;
+
+        /**
+         * For `IPluginFactory2::getClassInfo2`, works the same way as the
+         * above.
+         */
+        std::vector<std::optional<Steinberg::PClassInfo2>> class_infos_2;
+
+        /**
+         * For `IPluginFactory3::getClassInfoUnicode`, works the same way as the
+         * above.
+         */
+        std::vector<std::optional<Steinberg::PClassInfoW>> class_infos_unicode;
+
+        template <typename S>
+        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<Steinberg::PClassInfo>& info) {
+                            s.ext(info, bitsery::ext::StdOptional{});
+                        });
+            s.container(class_infos_2, 2048,
+                        [](S& s, std::optional<Steinberg::PClassInfo2>& info) {
+                            s.ext(info, bitsery::ext::StdOptional{});
+                        });
+            s.container(class_infos_unicode, 2048,
+                        [](S& s, std::optional<Steinberg::PClassInfoW>& 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<Steinberg::IPluginFactory> factory);
+    struct Construct {
+        using Response = ConstructArgs;
 
+        template <typename S>
+        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 <typename S>
-    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<Steinberg::PClassInfo>& info) {
-                        s.ext(info, bitsery::ext::StdOptional{});
-                    });
-        s.container(class_infos_2, 2048,
-                    [](S& s, std::optional<Steinberg::PClassInfo2>& info) {
-                        s.ext(info, bitsery::ext::StdOptional{});
-                    });
-        s.container(class_infos_unicode, 2048,
-                    [](S& s, std::optional<Steinberg::PClassInfoW>& info) {
-                        s.ext(info, bitsery::ext::StdOptional{});
-                    });
-    }
-
-   private:
-    /**
-     * The IIDs that the interface we serialized supports.
-     */
-    std::set<Steinberg::FUID> known_iids;
-
-    /**
-     * For `IPluginFactory::getFactoryInfo`.
-     */
-    std::optional<Steinberg::PFactoryInfo> 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<std::optional<Steinberg::PClassInfo>> class_infos_1;
-    /**
-     * For `IPluginFactory2::getClassInfo2`, works the same way as the above.
-     */
-    std::vector<std::optional<Steinberg::PClassInfo2>> class_infos_2;
-    /**
-     * For `IPluginFactory3::getClassInfoUnicode`, works the same way as the
-     * above.
-     */
-    std::vector<std::optional<Steinberg::PClassInfoW>> class_infos_unicode;
+   protected:
+    ConstructArgs arguments;
 };
 
 #pragma GCC diagnostic pop

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,

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,

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<Vst3Logger&, bool>(logger, true));
+        YaPluginFactory::ConstructArgs factory_args =
+            sockets.host_vst_control.send_message(
+                YaPluginFactory::Construct{},
+                std::pair<Vst3Logger&, bool>(logger, true));
+        plugin_factory =
+            new YaPluginFactoryPluginImpl(*this, std::move(factory_args));
     }
 
     return plugin_factory;

src/wine-host/bridges/vst3-impls.cpp

@@ -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 <https://www.gnu.org/licenses/>.
-
-#include "vst3-impls.h"
-
-YaPluginFactoryHostImpl::YaPluginFactoryHostImpl(
-    Steinberg::IPtr<Steinberg::IPluginFactory> 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()'");
-}

src/wine-host/bridges/vst3-impls.h

@@ -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 <https://www.gnu.org/licenses/>.
-
-#pragma once
-
-#include "vst3.h"
-
-class YaPluginFactoryHostImpl : public YaPluginFactory {
-   public:
-    YaPluginFactoryHostImpl(Steinberg::IPtr<Steinberg::IPluginFactory> factory);
-
-    tresult PLUGIN_API createInstance(Steinberg::FIDString cid,
-                                      Steinberg::FIDString _iid,
-                                      void** obj) override;
-
-    tresult PLUGIN_API
-    setHostContext(Steinberg::FUnknown* /*context*/) override;
-};

src/wine-host/bridges/vst3.cpp

@@ -17,7 +17,6 @@
 #include "vst3.h"
 
 #include "../boost-fix.h"
-#include "vst3-impls.h"
 
 #include <public.sdk/source/vst/hosting/module_win32.cpp>
 
@@ -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<YaPluginFactoryHostImpl>(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());
             }});
 }
 

src/wine-host/bridges/vst3.h

@@ -91,13 +91,6 @@ class Vst3Bridge : public HostBridge {
      */
     Vst3Sockets<Win32Thread> 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<YaPluginFactory> plugin_factory;
-
     /**
      * Used to assign unique identifier to instances created for
      * `IPluginFactory::createInstance()`.