From 9a82e82c87808076a34eb76c7f6b03922e056f7b Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 14 May 2020 19:11:17 +0200 Subject: [PATCH 01/73] Factor out directory finding from prefix detection This will also be used to locate the `yabridge.tmol` configuration file. --- src/common/configuration.h | 52 ++++++++++++++++++++++++++++++++++++++ src/plugin/utils.cpp | 19 +++++++------- src/plugin/utils.h | 3 ++- 3 files changed, 63 insertions(+), 11 deletions(-) create mode 100644 src/common/configuration.h diff --git a/src/common/configuration.h b/src/common/configuration.h new file mode 100644 index 00000000..0f31fd01 --- /dev/null +++ b/src/common/configuration.h @@ -0,0 +1,52 @@ +// 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 +#include + +/** + * Starting from the starting file or directory, go up in the directory + * hierarchy until we find a file named `filename`. + * + * @param filename The name of the file we're looking for. This can also be a + * directory name since directories are also files. + * @param starting_from The directory to start searching in. If this is a file, + * then start searching in the directory the file is located in. + * @param predicate The predicate to use to check if the path matches a file. + * Needed as an easy way to limit the search to directories only since C++17 + * does not have any built in coroutines or generators. + * + * @return The path to the *file* found, or `std::nullopt` if the file could not + * be found. + */ +template +std::optional find_dominating_file( + const std::string& filename, + std::filesystem::path starting_dir, + F predicate = std::filesystem::exists) { + while (starting_dir != "") { + const std::filesystem::path candidate = starting_dir / filename; + if (predicate(candidate)) { + return candidate; + } + + starting_dir = starting_dir.parent_path(); + } + + return std::nullopt; +} diff --git a/src/plugin/utils.cpp b/src/plugin/utils.cpp index e464ebfb..92363912 100644 --- a/src/plugin/utils.cpp +++ b/src/plugin/utils.cpp @@ -28,6 +28,8 @@ // Generated inside of build directory #include +#include <../common/configuration.h> + namespace bp = boost::process; namespace fs = boost::filesystem; @@ -53,16 +55,13 @@ std::string create_logger_prefix(const fs::path& socket_path) { } std::optional find_wineprefix() { - // Try to locate the Wine prefix the plugin's .dll file is located in by - // finding the first parent directory that contains a directory named - // `dosdevices` - fs::path wineprefix_path = find_vst_plugin(); - while (wineprefix_path != "") { - if (fs::is_directory(wineprefix_path / "dosdevices")) { - return wineprefix_path; - } - - wineprefix_path = wineprefix_path.parent_path(); + // We need these string conversions because Boost still doesn't use + // std::filesystem paths + std::optional dosdevices_dir = + find_dominating_file("dosdevices", find_vst_plugin().string(), + std::filesystem::is_directory); + if (dosdevices_dir.has_value()) { + return dosdevices_dir->parent_path().string(); } return std::nullopt; diff --git a/src/plugin/utils.h b/src/plugin/utils.h index c2991080..9978511e 100644 --- a/src/plugin/utils.h +++ b/src/plugin/utils.h @@ -101,7 +101,8 @@ boost::filesystem::path find_vst_plugin(); /** * Locate the Wine prefix this file is located in, if it is inside of a wine - * prefix. + * prefix. This is done by locating the first parent directory that contains a + * directory named `dosdevices`. * * @return Either the path to the Wine prefix (containing the `drive_c?` * directory), or `std::nullopt` if it is not inside of a wine prefix. From 6f148b97a4a5343184b6551a25b5e9506d36456d Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 15 May 2020 14:26:18 +0200 Subject: [PATCH 02/73] Add a TOML parser dependency --- meson.build | 25 ++++++++++++++++++++++--- subprojects/.gitignore | 3 +++ subprojects/tomlplusplus.wrap | 4 ++++ 3 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 subprojects/tomlplusplus.wrap diff --git a/meson.build b/meson.build index 1a6c8c5b..9bcde7d8 100644 --- a/meson.build +++ b/meson.build @@ -46,6 +46,7 @@ boost_dep = dependency('boost', version : '>=1.66', static : true) boost_filesystem_dep = dependency('boost', version : '>=1.66', modules : ['filesystem'], static : true) bitsery_dep = subproject('bitsery').get_variable('bitsery_dep') threads_dep = dependency('threads') +tomlplusplus_dep = subproject('tomlplusplus').get_variable('tomlplusplus_dep') # The built in threads dependency does not know how to handle winegcc wine_threads_dep = declare_dependency(link_args : '-lpthread') xcb_dep = dependency('xcb') @@ -69,7 +70,13 @@ shared_library( ], native : true, include_directories : include_dir, - dependencies : [boost_dep, boost_filesystem_dep, bitsery_dep, threads_dep], + dependencies : [ + boost_dep, + boost_filesystem_dep, + bitsery_dep, + threads_dep, + tomlplusplus_dep + ], cpp_args : compiler_options, link_args : ['-ldl'] ) @@ -90,7 +97,13 @@ executable( host_sources, native : false, include_directories : include_dir, - dependencies : [boost_dep, bitsery_dep, wine_threads_dep, xcb_dep], + dependencies : [ + boost_dep, + bitsery_dep, + tomlplusplus_dep, + wine_threads_dep, + xcb_dep + ], cpp_args : compiler_options + ['-m64'], link_args : ['-m64'] ) @@ -108,7 +121,13 @@ if get_option('use-bitbridge') host_sources, native : false, include_directories : include_dir, - dependencies : [boost_dep, bitsery_dep, wine_threads_dep, xcb_dep], + dependencies : [ + boost_dep, + bitsery_dep, + tomlplusplus_dep, + wine_threads_dep, + xcb_dep + ], # FIXME: 32-bit winegcc defines `__stdcall` differently than the 64-bit # version, and one of the changes is the inclusion of # `__atribute__((__force_align_arg_pointer__))`. For whetever reason diff --git a/subprojects/.gitignore b/subprojects/.gitignore index ea053e16..21ced76b 100644 --- a/subprojects/.gitignore +++ b/subprojects/.gitignore @@ -1 +1,4 @@ /*/* + +# The above pattern doesn't match submodules +/tomlplusplus diff --git a/subprojects/tomlplusplus.wrap b/subprojects/tomlplusplus.wrap new file mode 100644 index 00000000..15bb40d5 --- /dev/null +++ b/subprojects/tomlplusplus.wrap @@ -0,0 +1,4 @@ +[wrap-git] +url = https://github.com/marzer/tomlplusplus.git +revision = v1.2.5 +depth = 1 From d9ff98de8410c0c8c3aca2050daac106666f5b2a Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 15 May 2020 14:51:08 +0200 Subject: [PATCH 03/73] Move everything configuration related to plugin If it's tied to the .so file rather than the .dll file it wouldn't make any sense to use it directly from the Wine host. --- meson.build | 1 + src/{common => plugin}/configuration.h | 0 src/plugin/utils.cpp | 2 +- 3 files changed, 2 insertions(+), 1 deletion(-) rename src/{common => plugin}/configuration.h (100%) diff --git a/meson.build b/meson.build index 9bcde7d8..e5ea2f3c 100644 --- a/meson.build +++ b/meson.build @@ -63,6 +63,7 @@ shared_library( [ 'src/common/logging.cpp', 'src/common/serialization.cpp', + 'src/plugin/configuration.cpp', 'src/plugin/plugin.cpp', 'src/plugin/plugin-bridge.cpp', 'src/plugin/utils.cpp', diff --git a/src/common/configuration.h b/src/plugin/configuration.h similarity index 100% rename from src/common/configuration.h rename to src/plugin/configuration.h diff --git a/src/plugin/utils.cpp b/src/plugin/utils.cpp index 92363912..61885c94 100644 --- a/src/plugin/utils.cpp +++ b/src/plugin/utils.cpp @@ -28,7 +28,7 @@ // Generated inside of build directory #include -#include <../common/configuration.h> +#include "configuration.h" namespace bp = boost::process; namespace fs = boost::filesystem; From f96c08775ab13479228f382b39375723c8f491d1 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 15 May 2020 14:54:42 +0200 Subject: [PATCH 04/73] Use Boost.Filesystem for the configuration I'd much rather just use std::filesystem, but since all of Boost.Process, Boost.DLL Boost.Asio uses its own filesystem library we need to use it anyways. --- src/plugin/configuration.h | 12 ++++++------ src/plugin/utils.cpp | 13 +++++-------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/plugin/configuration.h b/src/plugin/configuration.h index 0f31fd01..d9e96ccf 100644 --- a/src/plugin/configuration.h +++ b/src/plugin/configuration.h @@ -16,7 +16,7 @@ #pragma once -#include +#include #include /** @@ -34,13 +34,13 @@ * @return The path to the *file* found, or `std::nullopt` if the file could not * be found. */ -template -std::optional find_dominating_file( +template +std::optional find_dominating_file( const std::string& filename, - std::filesystem::path starting_dir, - F predicate = std::filesystem::exists) { + boost::filesystem::path starting_dir, + F predicate = boost::filesystem::exists) { while (starting_dir != "") { - const std::filesystem::path candidate = starting_dir / filename; + const boost::filesystem::path candidate = starting_dir / filename; if (predicate(candidate)) { return candidate; } diff --git a/src/plugin/utils.cpp b/src/plugin/utils.cpp index 61885c94..061be195 100644 --- a/src/plugin/utils.cpp +++ b/src/plugin/utils.cpp @@ -55,16 +55,13 @@ std::string create_logger_prefix(const fs::path& socket_path) { } std::optional find_wineprefix() { - // We need these string conversions because Boost still doesn't use - // std::filesystem paths - std::optional dosdevices_dir = - find_dominating_file("dosdevices", find_vst_plugin().string(), - std::filesystem::is_directory); - if (dosdevices_dir.has_value()) { - return dosdevices_dir->parent_path().string(); + std::optional dosdevices_dir = + find_dominating_file("dosdevices", find_vst_plugin(), fs::is_directory); + if (!dosdevices_dir.has_value()) { + return std::nullopt; } - return std::nullopt; + return dosdevices_dir->parent_path(); } PluginArchitecture find_vst_architecture(fs::path plugin_path) { From a615a66cc507266cffbaf0da34cc009758cc18b2 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 15 May 2020 16:27:52 +0200 Subject: [PATCH 05/73] Add the group configuration parser As described in #15. --- src/plugin/configuration.cpp | 71 ++++++++++++++++++++++++++ src/plugin/configuration.h | 98 +++++++++++++++++++++++++++--------- src/plugin/utils.h | 32 ++++++++++++ 3 files changed, 176 insertions(+), 25 deletions(-) create mode 100644 src/plugin/configuration.cpp diff --git a/src/plugin/configuration.cpp b/src/plugin/configuration.cpp new file mode 100644 index 00000000..a7fd2984 --- /dev/null +++ b/src/plugin/configuration.cpp @@ -0,0 +1,71 @@ +// 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 "configuration.h" + +#include +#include +#include + +#include "utils.h" + +namespace fs = boost::filesystem; + +Configuration::Configuration() {} + +Configuration::Configuration(const fs::path& config_path, + const fs::path& yabridge_path) + : Configuration() { + // Will throw a `toml::parsing_error` if the file cannot be parsed. Better + // to throw here rather than failing silently since syntax errors would + // otherwise be impossible to spot. + toml::table table = toml::parse_file(config_path.string()); + + const fs::path relative_path = + yabridge_path.lexically_relative(config_path.parent_path()); + for (const auto& [pattern, value] : table) { + // First try to match the glob pattern, allow matching an entire + // directory for ease of use. If none of the patterns in the file match + // the plugin path then everything will be left at the defaults. + if (fnmatch(pattern.c_str(), relative_path.c_str(), + FNM_PATHNAME | FNM_LEADING_DIR) != 0) { + continue; + } + + matched_file = config_path; + matched_pattern = pattern; + + // If the table is missing some fields then they will simply be left at + // their defaults + if (toml::table* config = value.as_table(); config != nullptr) { + group = (*config)["group"].value(); + } + + break; + } +} + +Configuration Configuration::load_for(const fs::path& yabridge_path) { + // First find the closest `yabridge.tmol` file for the plugin, falling back + // to default configuration settings if it doesn't exist + const std::optional config_file = + find_dominating_file("yabridge.toml", yabridge_path); + if (!config_file.has_value()) { + return Configuration(); + } + + return Configuration(config_file.value(), yabridge_path); +} diff --git a/src/plugin/configuration.h b/src/plugin/configuration.h index d9e96ccf..ca88a8f6 100644 --- a/src/plugin/configuration.h +++ b/src/plugin/configuration.h @@ -20,33 +20,81 @@ #include /** - * Starting from the starting file or directory, go up in the directory - * hierarchy until we find a file named `filename`. + * An object that's used to provide plugin-specific configuration. Right now + * this is only used to declare plugin groups. A plugin group is a set of + * plugins that will be hosted in the same process rather than individually so + * they can share resources. Configuration file loading works as follows: * - * @param filename The name of the file we're looking for. This can also be a - * directory name since directories are also files. - * @param starting_from The directory to start searching in. If this is a file, - * then start searching in the directory the file is located in. - * @param predicate The predicate to use to check if the path matches a file. - * Needed as an easy way to limit the search to directories only since C++17 - * does not have any built in coroutines or generators. + * 1. `Configuration::load_for(path)` gets called with a path to the copy of or + * symlink to `libyabridge.so` that the plugin host has tried to load. + * 2. We start looking for a file named `yabridge.toml` in the same directory as + * that `.so` file, iteratively continuing to search one directory higher + * until we either find the file or we reach the filesystem root. + * 3. If the file is found, then parse it as a TOML file and look for the first + * table whose key is a glob pattern that (partially) matches the relative + * path between the found `yabridge.toml` and the `.so` file. As a rule of + * thumb, if the `find -type f` command executed in Bash would list + * the `.so` file, then the following table in `yabridge.tmol` would also + * match the same `.so` file: * - * @return The path to the *file* found, or `std::nullopt` if the file could not - * be found. + * ```toml + * [""] + * group = "..." + * ``` + * 4. If one of these glob patterns could be matched with the relative path of + * the `.so` file then we'll use the settings specified in that section. + * Otherwise the default settings will be used. */ -template -std::optional find_dominating_file( - const std::string& filename, - boost::filesystem::path starting_dir, - F predicate = boost::filesystem::exists) { - while (starting_dir != "") { - const boost::filesystem::path candidate = starting_dir / filename; - if (predicate(candidate)) { - return candidate; - } +class Configuration { + /** + * Create an empty configuration object with default settings. + */ + Configuration(); - starting_dir = starting_dir.parent_path(); - } + /** + * Load the configuration for an instance of yabridge from a configuration + * file by matching the plugin's relative path to the glob patterns in that + * configuration file. Will leave the object empty if the plugin cannot be + * matched to any of the patterns. Not meant to be used directly. + * + * @throw toml::parsing_error If the file could not be parsed. + * + * @see Configuration::load_for + */ + Configuration(const boost::filesystem::path& config_path, + const boost::filesystem::path& yabridge_path); - return std::nullopt; -} + /** + * Load the configuration that belongs to a copy of or symlink to + * `libyabridge.so`. If no configuration file could be found then this will + * return an empty configuration object with default settings. + * + * @param yabridge_path The path to the .so file that's being loaded.by the + * VST host. This will be used both for the starting location of the + * search and to determine which section in the config file to use. + * + * @return Either a configuration object populated with values from matched + * glob pattern within the found configuration file, or an empty + * configuration object if no configuration file could be found or if the + * plugin could not be matched to any of the glob patterns in the + * configuration file. + */ + static Configuration load_for(const boost::filesystem::path& yabridge_path); + + /** + * The name of the plugin group that should be used for the plugin this + * configuration object was created for. If not set, then the plugin should + * be hosted individually instead. + */ + std::optional group; + + /** + * The path to the configuration file that was parsed. + */ + std::optional matched_file; + + /** + * The matched glob pattern in the above configuration file. + */ + std::optional matched_pattern; +}; diff --git a/src/plugin/utils.h b/src/plugin/utils.h index 9978511e..ad481852 100644 --- a/src/plugin/utils.h +++ b/src/plugin/utils.h @@ -141,3 +141,35 @@ std::string get_wine_version(); * using the user's default prefix. */ boost::process::environment set_wineprefix(); + +/** + * Starting from the starting file or directory, go up in the directory + * hierarchy until we find a file named `filename`. + * + * @param filename The name of the file we're looking for. This can also be a + * directory name since directories are also files. + * @param starting_from The directory to start searching in. If this is a file, + * then start searching in the directory the file is located in. + * @param predicate The predicate to use to check if the path matches a file. + * Needed as an easy way to limit the search to directories only since C++17 + * does not have any built in coroutines or generators. + * + * @return The path to the *file* found, or `std::nullopt` if the file could not + * be found. + */ +template +std::optional find_dominating_file( + const std::string& filename, + boost::filesystem::path starting_dir, + F predicate = boost::filesystem::exists) { + while (starting_dir != "") { + const boost::filesystem::path candidate = starting_dir / filename; + if (predicate(candidate)) { + return candidate; + } + + starting_dir = starting_dir.parent_path(); + } + + return std::nullopt; +} From d0fe1c930a5edb2df982518949a1e66cabcaf10f Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 15 May 2020 16:51:50 +0200 Subject: [PATCH 06/73] Mention tomlplusplus in the readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index e1bbe9cf..cd4ab42e 100644 --- a/README.md +++ b/README.md @@ -222,6 +222,7 @@ the following dependencies: The following dependencies are included in the repository as a Meson wrap: - bitsery +- tomlplusplus The project can then be compiled as follows: From e76d4b474c0e33dd9bdcae86372b26fb98f10919 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 16 May 2020 14:46:48 +0200 Subject: [PATCH 07/73] Rearrange fields in PluginBridge --- src/plugin/plugin-bridge.h | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/plugin/plugin-bridge.h b/src/plugin/plugin-bridge.h index b180c695..fc979a18 100644 --- a/src/plugin/plugin-bridge.h +++ b/src/plugin/plugin-bridge.h @@ -95,19 +95,6 @@ class PluginBridge { */ AEffect plugin; - /** - * The VST host can query a plugin for arbitrary binary data such as - * presets. It will expect the plugin to write back a pointer that points to - * that data. This vector is where we store the chunk data for the last - * `effGetChunk` event. - */ - std::vector chunk_data; - /** - * The VST host will expect to be returned a pointer to a struct that stores - * the dimensions of the editor window. - */ - VstRect editor_rectangle; - private: /** * Write output from an async pipe to the log on a line by line basis. @@ -219,6 +206,19 @@ class PluginBridge { */ std::vector process_buffer; + /** + * The VST host can query a plugin for arbitrary binary data such as + * presets. It will expect the plugin to write back a pointer that points to + * that data. This vector is where we store the chunk data for the last + * `effGetChunk` event. + */ + std::vector chunk_data; + /** + * The VST host will expect to be returned a pointer to a struct that stores + * the dimensions of the editor window. + */ + VstRect editor_rectangle; + /** * Sending MIDI events sent to the host by the plugin using * `audioMasterProcessEvents` function has to be done during the processing From 312200f10008901b2e0a7109f0dbb6f7960bc37c Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 16 May 2020 16:08:01 +0200 Subject: [PATCH 08/73] Make the 'this_line_location' hack more reliable It shouldn't be done if it's not needed. --- CHANGELOG.md | 4 ++++ src/plugin/utils.cpp | 13 ++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5ed748f..b0f5ab69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ Versioning](https://semver.org/spec/v2.0.0.html). - Changed architecture to use one less socket. - Removed dependency on 32-bit Boost.Filesystem. +### Fixed + +- Made the plugin and host detection slightly more robust. + ## [1.1.4] - 2020-05-12 ### Fixed diff --git a/src/plugin/utils.cpp b/src/plugin/utils.cpp index 061be195..896b0313 100644 --- a/src/plugin/utils.cpp +++ b/src/plugin/utils.cpp @@ -200,9 +200,16 @@ fs::path get_this_file_location() { // on both Ubuntu 18.04 and 20.04, but not on Arch based distros. // Under Linux a path starting with two slashes is treated the same as // a path starting with only a single slash, but Wine will refuse to - // load any files when the path starts with two slashes. Prepending - // `/` to a pad coerces theses two slashes into a single slash. - return "/" / boost::dll::this_line_location(); + // load any files when the path starts with two slashes. The easiest + // way to work around this if this happens is to just add another + // leading slash and then normalize the path, since three or more + // slashes will be coerced into a single slash. + fs::path this_file = boost::dll::this_line_location(); + if (this_file.string().find("//") == 0) { + this_file = ("/" / this_file).lexically_normal(); + } + + return this_file; } std::string get_wine_version() { From d2cd608abb79e829e93a4ed8223e1bee43526bb1 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 16 May 2020 16:18:16 +0200 Subject: [PATCH 09/73] Print the configuration on startup --- src/plugin/configuration.cpp | 1 + src/plugin/configuration.h | 4 +++ src/plugin/plugin-bridge.cpp | 58 +++++++++++++++++++++++++++--------- src/plugin/plugin-bridge.h | 15 ++++++++++ 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/src/plugin/configuration.cpp b/src/plugin/configuration.cpp index a7fd2984..a45944af 100644 --- a/src/plugin/configuration.cpp +++ b/src/plugin/configuration.cpp @@ -67,5 +67,6 @@ Configuration Configuration::load_for(const fs::path& yabridge_path) { return Configuration(); } + // TODO: Disable plugin groups when not compiling with plugin group support return Configuration(config_file.value(), yabridge_path); } diff --git a/src/plugin/configuration.h b/src/plugin/configuration.h index ca88a8f6..f3e51bb1 100644 --- a/src/plugin/configuration.h +++ b/src/plugin/configuration.h @@ -46,6 +46,7 @@ * Otherwise the default settings will be used. */ class Configuration { + public: /** * Create an empty configuration object with default settings. */ @@ -69,6 +70,9 @@ class Configuration { * `libyabridge.so`. If no configuration file could be found then this will * return an empty configuration object with default settings. * + * This function will take any optional compile-time features that have not + * been enabled into account. + * * @param yabridge_path The path to the .so file that's being loaded.by the * VST host. This will be used both for the starting location of the * search and to determine which section in the config file to use. diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index ea90240b..be6eef53 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -71,6 +71,7 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) host_callback_function(host_callback), logger(Logger::create_from_environment( create_logger_prefix(socket_endpoint.path()))), + config(Configuration::load_for(get_this_file_location())), wine_version(get_wine_version()), wine_stdout(io_context), wine_stderr(io_context), @@ -102,29 +103,58 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) bp::start_dir = vst_plugin_path.parent_path()) #endif { - logger.log("Initializing yabridge version " + - std::string(yabridge_git_version)); - logger.log("host: '" + vst_host_path.string() + "'"); - logger.log("plugin: '" + vst_plugin_path.string() + "'"); - logger.log("socket: '" + socket_endpoint.path() + "'"); - logger.log("wine prefix: '" + - find_wineprefix().value_or("").string() + "'"); - logger.log("wine version: '" + wine_version + "'"); + std::stringstream init_msg; + init_msg << "Initializing yabridge version " << yabridge_git_version + << std::endl; + init_msg << "host: '" << vst_host_path.string() << "'" << std::endl; + init_msg << "plugin: '" << vst_plugin_path.string() << "'" + << std::endl; + init_msg << "socket: '" << socket_endpoint.path() << "'" << std::endl; + init_msg << "wine prefix: '" + << find_wineprefix().value_or("").string() << "'" + << std::endl; + init_msg << "wine version: '" << wine_version << "'" << std::endl; + init_msg << std::endl; + + // Print the path to the currently loaded configuration file and all + // settings in use. Printing the matched glob pattern could also be useful + // but it'll be very noisy and it's likely going to be clear from the shown + // values anyways. + init_msg << "config path: '" + << config.matched_file.value_or("").string() << "'" + << std::endl; + init_msg << "hosting mode: '"; + if (config.group.has_value()) { + init_msg << "group \"" << config.group.value() << "\""; + } else { + init_msg << "individual"; + } + if (vst_plugin_arch == PluginArchitecture::vst_32) { + init_msg << ", 32-bit"; + } else { + init_msg << ", 64-bit"; + } + init_msg << "'" << std::endl; + init_msg << std::endl; // Include a list of enabled compile-tiem features, mostly to make debug // logs more useful - logger.log(""); - logger.log("Enabled features:"); + // TODO: Add a feature flag for the plugin group support + init_msg << "Enabled features:" << std::endl; #ifdef USE_BITBRIDGE - logger.log("- bitbridge support"); + init_msg << "- bitbridge support" << std::endl; #endif #ifdef USE_WINEDBG - logger.log("- winedbg"); + init_msg << "- winedbg" << std::endl; #endif #if !(defined(USE_BITBRIDGE) || defined(USE_WINEDBG)) - logger.log(" "); + init_msg << " " << std::endl; #endif - logger.log(""); + init_msg << std::endl; + + for (std::string line = ""; std::getline(init_msg, line);) { + logger.log(line); + } // Print the Wine host's STDOUT and STDERR streams to the log file. This // should be done before trying to accept the sockets as otherwise we will diff --git a/src/plugin/plugin-bridge.h b/src/plugin/plugin-bridge.h index fc979a18..ca20823c 100644 --- a/src/plugin/plugin-bridge.h +++ b/src/plugin/plugin-bridge.h @@ -26,6 +26,7 @@ #include #include "../common/logging.h" +#include "configuration.h" #include "utils.h" /** @@ -171,8 +172,22 @@ class PluginBridge { */ audioMasterCallback host_callback_function; + /** + * The logging facility used for this instance of yabridge. See + * `Logger::create_from_env()` for how this is configured. + * + * @see Logger::create_from_env + */ Logger logger; + /** + * The configuration for this instance of yabridge. Set based on the values + * from a `yabridge.toml`, if it exists. + * + * @see Configuration::load_for + */ + Configuration config; + /** * The version of Wine currently in use. Used in the debug output on plugin * startup. From a849927a08f25088dc616fb3417a7f9c4f4f468e Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 17 May 2020 14:33:52 +0200 Subject: [PATCH 10/73] Move initialization message to a function It was starting to get a bit unwieldy. --- src/plugin/plugin-bridge.cpp | 113 +++++++++++++++++++---------------- src/plugin/plugin-bridge.h | 5 ++ 2 files changed, 66 insertions(+), 52 deletions(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index be6eef53..d9e08ac3 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -103,58 +103,7 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) bp::start_dir = vst_plugin_path.parent_path()) #endif { - std::stringstream init_msg; - init_msg << "Initializing yabridge version " << yabridge_git_version - << std::endl; - init_msg << "host: '" << vst_host_path.string() << "'" << std::endl; - init_msg << "plugin: '" << vst_plugin_path.string() << "'" - << std::endl; - init_msg << "socket: '" << socket_endpoint.path() << "'" << std::endl; - init_msg << "wine prefix: '" - << find_wineprefix().value_or("").string() << "'" - << std::endl; - init_msg << "wine version: '" << wine_version << "'" << std::endl; - init_msg << std::endl; - - // Print the path to the currently loaded configuration file and all - // settings in use. Printing the matched glob pattern could also be useful - // but it'll be very noisy and it's likely going to be clear from the shown - // values anyways. - init_msg << "config path: '" - << config.matched_file.value_or("").string() << "'" - << std::endl; - init_msg << "hosting mode: '"; - if (config.group.has_value()) { - init_msg << "group \"" << config.group.value() << "\""; - } else { - init_msg << "individual"; - } - if (vst_plugin_arch == PluginArchitecture::vst_32) { - init_msg << ", 32-bit"; - } else { - init_msg << ", 64-bit"; - } - init_msg << "'" << std::endl; - init_msg << std::endl; - - // Include a list of enabled compile-tiem features, mostly to make debug - // logs more useful - // TODO: Add a feature flag for the plugin group support - init_msg << "Enabled features:" << std::endl; -#ifdef USE_BITBRIDGE - init_msg << "- bitbridge support" << std::endl; -#endif -#ifdef USE_WINEDBG - init_msg << "- winedbg" << std::endl; -#endif -#if !(defined(USE_BITBRIDGE) || defined(USE_WINEDBG)) - init_msg << " " << std::endl; -#endif - init_msg << std::endl; - - for (std::string line = ""; std::getline(init_msg, line);) { - logger.log(line); - } + log_init_message(); // Print the Wine host's STDOUT and STDERR streams to the log file. This // should be done before trying to accept the sockets as otherwise we will @@ -676,6 +625,66 @@ void PluginBridge::async_log_pipe_lines(patched_async_pipe& pipe, }); } +void PluginBridge::log_init_message() { + std::stringstream init_msg; + + init_msg << "Initializing yabridge version " << yabridge_git_version + << std::endl; + init_msg << "host: '" << vst_host_path.string() << "'" << std::endl; + init_msg << "plugin: '" << vst_plugin_path.string() << "'" + << std::endl; + init_msg << "socket: '" << socket_endpoint.path() << "'" << std::endl; + init_msg << "wine prefix: '" + << find_wineprefix().value_or("").string() << "'" + << std::endl; + init_msg << "wine version: '" << wine_version << "'" << std::endl; + init_msg << std::endl; + + // Print the path to the currently loaded configuration file and all + // settings in use. Printing the matched glob pattern could also be useful + // but it'll be very noisy and it's likely going to be clear from the shown + // values anyways. + // TODO: The group socket should take into account the: + // - wine prefix + // - group name + // - architecture + init_msg << "config path: '" + << config.matched_file.value_or("").string() << "'" + << std::endl; + init_msg << "hosting mode: '"; + if (config.group.has_value()) { + init_msg << "group \"" << config.group.value() << "\""; + } else { + init_msg << "individual"; + } + if (vst_plugin_arch == PluginArchitecture::vst_32) { + init_msg << ", 32-bit"; + } else { + init_msg << ", 64-bit"; + } + init_msg << "'" << std::endl; + init_msg << std::endl; + + // Include a list of enabled compile-tiem features, mostly to make debug + // logs more useful + // TODO: Add a feature flag for the plugin group support + init_msg << "Enabled features:" << std::endl; +#ifdef USE_BITBRIDGE + init_msg << "- bitbridge support" << std::endl; +#endif +#ifdef USE_WINEDBG + init_msg << "- winedbg" << std::endl; +#endif +#if !(defined(USE_BITBRIDGE) || defined(USE_WINEDBG)) + init_msg << " " << std::endl; +#endif + init_msg << std::endl; + + for (std::string line = ""; std::getline(init_msg, line);) { + logger.log(line); + } +} + // The below functions are proxy functions for the methods defined in // `Bridge.cpp` diff --git a/src/plugin/plugin-bridge.h b/src/plugin/plugin-bridge.h index ca20823c..8b7672fb 100644 --- a/src/plugin/plugin-bridge.h +++ b/src/plugin/plugin-bridge.h @@ -109,6 +109,11 @@ class PluginBridge { boost::asio::streambuf& buffer, std::string prefix = ""); + /** + * Format and log all relevant debug information during initialization. + */ + void log_init_message(); + boost::asio::io_context io_context; boost::asio::local::stream_protocol::endpoint socket_endpoint; boost::asio::local::stream_protocol::acceptor socket_acceptor; From 95e716d22950d17b44302f366c2053e059e9abc1 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 17 May 2020 14:37:59 +0200 Subject: [PATCH 11/73] Rename vst-host.cpp -> individual-host.cpp --- meson.build | 4 ++-- src/wine-host/{vst-host.cpp => individual-host.cpp} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename src/wine-host/{vst-host.cpp => individual-host.cpp} (100%) diff --git a/meson.build b/meson.build index e5ea2f3c..fd511f5a 100644 --- a/meson.build +++ b/meson.build @@ -87,9 +87,9 @@ host_sources = [ 'src/common/serialization.cpp', 'src/wine-host/editor.cpp', 'src/wine-host/editor.cpp', - 'src/wine-host/vst-host.cpp', - 'src/wine-host/wine-bridge.cpp', + 'src/wine-host/individual-host.cpp', 'src/wine-host/utils.cpp', + 'src/wine-host/wine-bridge.cpp', version_header, ] diff --git a/src/wine-host/vst-host.cpp b/src/wine-host/individual-host.cpp similarity index 100% rename from src/wine-host/vst-host.cpp rename to src/wine-host/individual-host.cpp From 994f3c9e3853684cfbe356d83c2c1cc909c9905a Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 17 May 2020 15:07:20 +0200 Subject: [PATCH 12/73] Add a plugin group host application --- README.md | 6 +++ meson.build | 54 +++++++++++++++++---- src/common/config/config.h.in | 29 ++++++++--- src/common/config/meson.build | 6 ++- src/plugin/utils.cpp | 5 +- src/wine-host/group-host.cpp | 81 +++++++++++++++++++++++++++++++ src/wine-host/individual-host.cpp | 15 ++++-- 7 files changed, 171 insertions(+), 25 deletions(-) create mode 100644 src/wine-host/group-host.cpp diff --git a/README.md b/README.md index cd4ab42e..68e03ae2 100644 --- a/README.md +++ b/README.md @@ -125,6 +125,10 @@ yabridge is also able to load 32-bit VST plugins. The installation procedure for automatically detect whether a plugin is 32-bit or 64-bit on startup and it will handle it accordingly. +### Plugin groups + +TODO: Document + ### Wine prefixes It is also possible to use yabridge with multiple Wine prefixes. Yabridge will @@ -431,3 +435,5 @@ as the _Windows VST plugin_. The whole process works as follows: from the plugin's `AEffect` struct to the Linux native VST plugin over the `dispatcher()` socket. This is only done once at startup. After this point the plugin will stop blocking and has finished loading. + +TODO: Document plugin groups diff --git a/meson.build b/meson.build index fd511f5a..81b87057 100644 --- a/meson.build +++ b/meson.build @@ -15,15 +15,17 @@ if not meson.get_compiler('cpp').compiles(winelib_check) error('You need to set up a cross compiler, check the README for compilation instructions.') endif -# Depending on the `use-bitbridge` flag we'll enable building a second 32-bit -# host application that can act as a bit bridge for using 32-bit Windows plugins +# Depending on the `use-bitbridge` flag we'll enable building secondary 32-bit +# host applications that can act as a bit bridge for using 32-bit Windows plugins # in 64-bit Linux VST hosts. The plugin will determine which host application to # use based on the `.dll` file it's trying to load. # This setup is necessary until Meson provides a way to have multiple # cross-builds for a single build directory: # https://github.com/mesonbuild/meson/issues/5125 -host_name_64bit = 'yabridge-host' -host_name_32bit = 'yabridge-host-32' +individual_host_name_64bit = 'yabridge-host' +individual_host_name_32bit = 'yabridge-host-32' +group_host_name_64bit = 'yabridge-group' +group_host_name_32bit = 'yabridge-group-32' # This provides an easy way to start the Wine VST host using winedbg since it # can be quite a pain to set up @@ -87,15 +89,33 @@ host_sources = [ 'src/common/serialization.cpp', 'src/wine-host/editor.cpp', 'src/wine-host/editor.cpp', - 'src/wine-host/individual-host.cpp', 'src/wine-host/utils.cpp', 'src/wine-host/wine-bridge.cpp', version_header, ] +individual_host_sources = host_sources + ['src/wine-host/individual-host.cpp'] +group_host_sources = host_sources + ['src/wine-host/group-host.cpp'] + executable( - host_name_64bit, - host_sources, + individual_host_name_64bit, + individual_host_sources, + native : false, + include_directories : include_dir, + dependencies : [ + boost_dep, + bitsery_dep, + tomlplusplus_dep, + wine_threads_dep, + xcb_dep + ], + cpp_args : compiler_options + ['-m64'], + link_args : ['-m64'] +) + +executable( + group_host_name_64bit, + group_host_sources, native : false, include_directories : include_dir, dependencies : [ @@ -118,8 +138,8 @@ if get_option('use-bitbridge') xcb_dep = winegcc.find_library('xcb') executable( - host_name_32bit, - host_sources, + individual_host_name_32bit, + individual_host_sources, native : false, include_directories : include_dir, dependencies : [ @@ -139,4 +159,20 @@ if get_option('use-bitbridge') cpp_args : compiler_options + ['-m32', '-Wno-ignored-attributes'], link_args : ['-m32'] ) + + executable( + group_host_name_32bit, + group_host_sources, + native : false, + include_directories : include_dir, + dependencies : [ + boost_dep, + bitsery_dep, + tomlplusplus_dep, + wine_threads_dep, + xcb_dep + ], + cpp_args : compiler_options + ['-m32', '-Wno-ignored-attributes'], + link_args : ['-m32'] + ) endif diff --git a/src/common/config/config.h.in b/src/common/config/config.h.in index 8f225016..358e9073 100644 --- a/src/common/config/config.h.in +++ b/src/common/config/config.h.in @@ -17,14 +17,29 @@ #pragma once /** - * The name of the wine host name, e.g. `yabridge-host.exe` for the regular 64 - * bit build. + * The name of the Wine VST host application, e.g. `yabridge-host.exe` for the + * regular 64-bit build. */ -constexpr char yabridge_wine_host_name[] = "@host_binary_64bit@"; +constexpr char yabridge_individual_host_name[] = + "@individual_host_binary_64bit@"; /** - * The name of the 32-bit wine host name, e.g. `yabridge-host-32.exe`.` This is - * used as a bitbridge to be able to load legacy 32-bit only Windows plugins - * from a 64-bit Linux host. + * The name of the group host application, e.g. `yabridge-group.exe` for the + * regular 64-bit build. */ -constexpr char yabridge_wine_host_name_32bit[] = "@host_binary_32bit@"; +constexpr char yabridge_group_host_name[] = "@group_host_binary_64bit@"; + +/** + * The name of the 32-bit Wine VST host application, e.g. + * `yabridge-host-32.exe`.` This is used as a bitbridge to be able to load + * legacy 32-bit only Windows plugins from a 64-bit Linux host. + */ +constexpr char yabridge_individual_host_name_32bit[] = + "@individual_host_binary_32bit@"; + +/** + * The name of the 32-bit group host application, e.g. `yabridge-group-32.exe`.` + * This is used as a bitbridge to be able to load legacy 32-bit only Windows + * plugins from a 64-bit Linux host. + */ +constexpr char yabridge_group_host_name_32bit[] = "@group_host_binary_32bit@"; diff --git a/src/common/config/meson.build b/src/common/config/meson.build index eb2cd4f6..6c88ee17 100644 --- a/src/common/config/meson.build +++ b/src/common/config/meson.build @@ -5,8 +5,10 @@ config_header = configure_file( output : 'config.h', configuration : configuration_data( { - 'host_binary_32bit': host_name_32bit + '.exe', - 'host_binary_64bit': host_name_64bit + '.exe', + 'individual_host_binary_32bit': individual_host_name_32bit + '.exe', + 'individual_host_binary_64bit': individual_host_name_64bit + '.exe', + 'group_host_binary_32bit': group_host_name_32bit + '.exe', + 'group_host_binary_64bit': group_host_name_64bit + '.exe', } ) ) diff --git a/src/plugin/utils.cpp b/src/plugin/utils.cpp index 896b0313..2fa192fa 100644 --- a/src/plugin/utils.cpp +++ b/src/plugin/utils.cpp @@ -112,9 +112,10 @@ PluginArchitecture find_vst_architecture(fs::path plugin_path) { } fs::path find_vst_host(PluginArchitecture plugin_arch) { - auto host_name = yabridge_wine_host_name; + // TODO: Take plugin group settings into account + auto host_name = yabridge_individual_host_name; if (plugin_arch == PluginArchitecture::vst_32) { - host_name = yabridge_wine_host_name_32bit; + host_name = yabridge_individual_host_name_32bit; } fs::path host_path = diff --git a/src/wine-host/group-host.cpp b/src/wine-host/group-host.cpp new file mode 100644 index 00000000..afd3f03c --- /dev/null +++ b/src/wine-host/group-host.cpp @@ -0,0 +1,81 @@ +// 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 + +// Generated inside of build directory +#include +#include + +#include "wine-bridge.h" + +/** + * This works very similar to the host application defined in + * `individual-host.cpp`, but instead of just loading a single plugin this will + * act as a daemon that can host multiple 'grouped' plugins. This works by + * allowing the `libyabridge.so` instance to connect this this process over a + * socket to ask this process to host a VST `.dll` file using a provided socket. + * After that initialization step both the regular individual plugin host and + * this group plugin host will function identically on both the plugin and the + * Wine VST host side. + * + * The explicit calling convention is needed to work around a bug introduced in + * Wine 5.7: https://bugs.winehq.org/show_bug.cgi?id=49138 + */ +int __cdecl main(int argc, char* argv[]) { + // Instead of directly hosting a plugin, this process will receive a UNIX + // domain socket endpoint path that it should listen on to allow yabridge + // instances to spawn plugins in this process. + if (argc < 3) { + std::cerr << "Usage: " +#ifdef __i386__ + << yabridge_group_host_name_32bit +#else + << yabridge_group_host_name +#endif + << " " << std::endl; + + return 1; + } + + const std::string group_name(argv[1]); + const std::string group_socket_endpoint_path(argv[2]); + + // TODO: Before doing anything, try listening on the socket and fail + // silently (or log a message?) if another application is already + // listening on the socket. This way we don't need any complicated + // inter-process synchronization to ensure that there is a single + // active group host listening for this group. + // TODO: We should somehow try and redirect this process's STDOUT and STDERR + // streams to the logger so we can forward Wine's debug messages to + // the log even the yabridge plugin instance that initially spawned + // this group host process has exited. The only way I can think of + // doing this would be using some kind of in memory file and the + // `dup2()` system call. + + Logger logger = Logger::create_from_environment(group_name); + logger.log("Initializing yabridge group host version " + + std::string(yabridge_git_version) +#ifdef __i386__ + + " (32-bit compatibility mode)" +#endif + ); + + // TODO: After initializing, listen for connections and spawn plugins + // the exact same way as what happens in `individual-host.cpp` + // TODO: Allow this process to exit when the last plugin exits. Make sure + // that that doesn't cause any race conditions. +} diff --git a/src/wine-host/individual-host.cpp b/src/wine-host/individual-host.cpp index 2d6e059c..1d8149a3 100644 --- a/src/wine-host/individual-host.cpp +++ b/src/wine-host/individual-host.cpp @@ -22,9 +22,14 @@ #include "wine-bridge.h" -// This explicit calling convention is needed to work around a bug introduced in -// Wine 5.7 -// https://bugs.winehq.org/show_bug.cgi?id=49138 +/** + * This is the default VST host application. It will load the specified VST2 + * plugin, and then connect back to the `libyabridge.so` instace that spawned + * this over the socket. + * + * The explicit calling convention is needed to work around a bug introduced in + * Wine 5.7: https://bugs.winehq.org/show_bug.cgi?id=49138 + */ int __cdecl main(int argc, char* argv[]) { // We pass the name of the VST plugin .dll file to load and the Unix domain // socket to connect to in plugin/bridge.cpp as the first two arguments of @@ -32,9 +37,9 @@ int __cdecl main(int argc, char* argv[]) { if (argc < 3) { std::cerr << "Usage: " #ifdef __i386__ - << yabridge_wine_host_name_32bit + << yabridge_individual_host_name_32bit #else - << yabridge_wine_host_name + << yabridge_individual_host_name #endif << " " << std::endl; From 2f3965032293e9305f6d6a4cc2afea1539b11df9 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 17 May 2020 16:18:08 +0200 Subject: [PATCH 13/73] Remove last traces of Boost.Filesystem from host Missed this in e728dbe5a2262d9df931c93c651b053d7a80b5e5. --- src/wine-host/wine-bridge.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/wine-host/wine-bridge.cpp b/src/wine-host/wine-bridge.cpp index 800339b4..2adc9f39 100644 --- a/src/wine-host/wine-bridge.cpp +++ b/src/wine-host/wine-bridge.cpp @@ -18,13 +18,9 @@ #include -#include - #include "../common/communication.h" #include "../common/events.h" -namespace fs = boost::filesystem; - /** * A function pointer to what should be the entry point of a VST plugin. */ From 4e80e23cc09afe9296c913e8ab9569788139eaf0 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 17 May 2020 16:36:09 +0200 Subject: [PATCH 14/73] Revert "Don't link the winelibs with libboost_filesystem" This reverts commit e728dbe5a2262d9df931c93c651b053d7a80b5e5. `std::filesystem` is broken on wineg++, at least with Wine 5.8. Any path operations will throw a `std::filesystem::__cxx11::filesystem_error`: what(): filesystem error: Cannot convert character sequence: Invalid or incomplete multibyte or wide character --- CHANGELOG.md | 1 - README.md | 2 +- meson.build | 14 +++++++++++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0f5ab69..e0c72541 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,6 @@ Versioning](https://semver.org/spec/v2.0.0.html). ### Changed - Changed architecture to use one less socket. -- Removed dependency on 32-bit Boost.Filesystem. ### Fixed diff --git a/README.md b/README.md index 68e03ae2..0580ff21 100644 --- a/README.md +++ b/README.md @@ -244,7 +244,7 @@ It is also possible to compile a host application for yabridge that's compatible with 32-bit plugins such as old SynthEdit plugins. This will allow yabridge to act as a bitbirdge, allowing you to run old 32-bit only Windows VST2 plugins in a modern 64-bit Linux VST host. For this you'll need to have installed the 32 -bit version of the XCB library. This can then be set up as follows: +bit versions of the Boost and XCB libraries. This can then be set up as follows: ```shell # Enable the bitbridge on an existing build diff --git a/meson.build b/meson.build index 81b87057..d69a9925 100644 --- a/meson.build +++ b/meson.build @@ -41,9 +41,8 @@ endif # and the name of the host binary subdir('src/common/config') -# Statically link against Boost.Filesystem, otherwise it becomes impossible to -# distribute a prebuilt version of yabridge. For the Wine host applications we -# only use the headers only libraries. +# Statically link against Boost.Filesystem, otherwise it would become impossible +# to distribute a prebuilt version of yabridge boost_dep = dependency('boost', version : '>=1.66', static : true) boost_filesystem_dep = dependency('boost', version : '>=1.66', modules : ['filesystem'], static : true) bitsery_dep = subproject('bitsery').get_variable('bitsery_dep') @@ -104,6 +103,7 @@ executable( include_directories : include_dir, dependencies : [ boost_dep, + boost_filesystem_dep, bitsery_dep, tomlplusplus_dep, wine_threads_dep, @@ -120,6 +120,7 @@ executable( include_directories : include_dir, dependencies : [ boost_dep, + boost_filesystem_dep, bitsery_dep, tomlplusplus_dep, wine_threads_dep, @@ -135,6 +136,11 @@ if get_option('use-bitbridge') # I honestly have no idea what the correct way to have `find_dependency()` use # `/usr/lib32` instead of `/usr/lib` is. If anyone does know, please tell me! winegcc = meson.get_compiler('cpp', native : false) + boost_filesystem_dep = winegcc.find_library( + 'boost_filesystem', + static : true, + dirs : ['/usr/lib', '/usr/local/lib'] + ) xcb_dep = winegcc.find_library('xcb') executable( @@ -144,6 +150,7 @@ if get_option('use-bitbridge') include_directories : include_dir, dependencies : [ boost_dep, + boost_filesystem_dep, bitsery_dep, tomlplusplus_dep, wine_threads_dep, @@ -167,6 +174,7 @@ if get_option('use-bitbridge') include_directories : include_dir, dependencies : [ boost_dep, + boost_filesystem_dep, bitsery_dep, tomlplusplus_dep, wine_threads_dep, From df2c4c29a9442e59d7271dabf22ef905371e0e99 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 17 May 2020 16:46:25 +0200 Subject: [PATCH 15/73] Fix search path for 32-bit Boost --- meson.build | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/meson.build b/meson.build index d69a9925..fa04a7a2 100644 --- a/meson.build +++ b/meson.build @@ -139,7 +139,12 @@ if get_option('use-bitbridge') boost_filesystem_dep = winegcc.find_library( 'boost_filesystem', static : true, - dirs : ['/usr/lib', '/usr/local/lib'] + dirs : [ + '/usr/lib32', + '/usr/lib/i386-linux-gnu', + '/usr/local/lib32', + '/usr/local/lib/i386-linux-gnu' + ] ) xcb_dep = winegcc.find_library('xcb') From 0ad849a42fed3a810d05c577047faf762155ce8f Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 17 May 2020 18:35:40 +0200 Subject: [PATCH 16/73] Fall back to /dev/stderr instead of std::cerr This allows us to remap STDIO internally in the group process. --- src/common/logging.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/common/logging.cpp b/src/common/logging.cpp index 6950dd26..d8e9b892 100644 --- a/src/common/logging.cpp +++ b/src/common/logging.cpp @@ -72,7 +72,12 @@ Logger Logger::create_from_environment(std::string prefix) { if (log_file->is_open()) { return Logger(log_file, verbosity_level, prefix); } else { - return Logger(std::shared_ptr(&std::cerr, [](auto) {}), + // For STDERR we sadly can't just use `std::cerr`. In the group process + // we need to capture all output generated by the process itself, and + // the only way to do this is by reopening the STDERR and STDOUT streams + // to a pipe. Luckily `/dev/stderr` stays unaffected, so we can still + // write there without causing infinite loops. + return Logger(std::make_shared("/dev/stderr"), verbosity_level, prefix); } } From b8028b8e13d8ce834fd83232f673fc41bd2f9694 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 17 May 2020 18:59:10 +0200 Subject: [PATCH 17/73] Remap STDOUT and STDERR in group process to log This is not very pretty, but there's not really another way and I"m surprised that it actually works. --- src/wine-host/group-host.cpp | 118 +++++++++++++++++++++++++++++++---- 1 file changed, 107 insertions(+), 11 deletions(-) diff --git a/src/wine-host/group-host.cpp b/src/wine-host/group-host.cpp index afd3f03c..76a7450d 100644 --- a/src/wine-host/group-host.cpp +++ b/src/wine-host/group-host.cpp @@ -14,7 +14,16 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . +#include "boost-fix.h" + +#include +#include +#include +#include +#include #include +#include +#include // Generated inside of build directory #include @@ -22,6 +31,27 @@ #include "wine-bridge.h" +// FIXME: `std::filesystem` is broken in wineg++, at least under Wine 5.8. Any +// path operation will thrown an encoding related error. +namespace fs = boost::filesystem; + +// TODO: Move most plumbing to another file + +/** + * Create a logger prefix containing the group name based on the socket path. + */ +std::string create_logger_prefix(const fs::path& socket_path); + +/** + * Continuously Read from a stream and write the lines to a logger instance. + * + * TODO: Merge this with the other similar function in `PluginBridge` + */ +void log_lines(Logger& logger, + boost::asio::posix::stream_descriptor& pipe, + boost::asio::streambuf& buffer, + std::string prefix); + /** * This works very similar to the host application defined in * `individual-host.cpp`, but instead of just loading a single plugin this will @@ -39,34 +69,55 @@ int __cdecl main(int argc, char* argv[]) { // Instead of directly hosting a plugin, this process will receive a UNIX // domain socket endpoint path that it should listen on to allow yabridge // instances to spawn plugins in this process. - if (argc < 3) { + if (argc < 2) { std::cerr << "Usage: " #ifdef __i386__ << yabridge_group_host_name_32bit #else << yabridge_group_host_name #endif - << " " << std::endl; + << " " << std::endl; return 1; } - const std::string group_name(argv[1]); - const std::string group_socket_endpoint_path(argv[2]); + const std::string group_socket_endpoint_path(argv[1]); // TODO: Before doing anything, try listening on the socket and fail // silently (or log a message?) if another application is already // listening on the socket. This way we don't need any complicated // inter-process synchronization to ensure that there is a single // active group host listening for this group. - // TODO: We should somehow try and redirect this process's STDOUT and STDERR - // streams to the logger so we can forward Wine's debug messages to - // the log even the yabridge plugin instance that initially spawned - // this group host process has exited. The only way I can think of - // doing this would be using some kind of in memory file and the - // `dup2()` system call. - Logger logger = Logger::create_from_environment(group_name); + // Has to be initialized before redirecting the STDIO streams + Logger logger = Logger::create_from_environment( + create_logger_prefix(group_socket_endpoint_path)); + + // Redirect this process's STDOUT and STDERR streams to a pipe so we can + // process the output and redirect it to a logger. Needed to capture Wine + // debug output, since this process will likely outlive the yabridge + // instance that originally spawned it. + int stdout_pipe[2]; + pipe(stdout_pipe); + dup2(stdout_pipe[1], STDOUT_FILENO); + close(stdout_pipe[1]); + + int stderr_pipe[2]; + pipe(stderr_pipe); + dup2(stderr_pipe[1], STDERR_FILENO); + close(stderr_pipe[1]); + + boost::asio::io_context io_context; + boost::asio::streambuf stdout_buffer; + boost::asio::streambuf stderr_buffer; + boost::asio::posix::stream_descriptor stdout_redirect(io_context, + stdout_pipe[0]); + boost::asio::posix::stream_descriptor stderr_redirect(io_context, + stderr_pipe[0]); + log_lines(logger, stdout_redirect, stdout_buffer, "[STDOUT] "); + log_lines(logger, stderr_redirect, stderr_buffer, "[STDERR] "); + std::thread io_handler([&]() { io_context.run(); }); + logger.log("Initializing yabridge group host version " + std::string(yabridge_git_version) #ifdef __i386__ @@ -74,8 +125,53 @@ int __cdecl main(int argc, char* argv[]) { #endif ); + // TODO: Remove debug prints + printf("This should be caught now!\n"); + std::cerr << "This too!" << std::endl; + // TODO: After initializing, listen for connections and spawn plugins // the exact same way as what happens in `individual-host.cpp` // TODO: Allow this process to exit when the last plugin exits. Make sure // that that doesn't cause any race conditions. + + // TODO: This usleep() is just to ensure that the second print to stderr + // also gets processed before stopping the IO context since we're + // immediately stopping it after starting. This would not needed in + // normal use. + usleep(1000); + io_context.stop(); + io_handler.join(); +} + +void log_lines(Logger& logger, + boost::asio::posix::stream_descriptor& pipe, + boost::asio::streambuf& buffer, + std::string prefix) { + boost::asio::async_read_until(pipe, buffer, '\n', + [&, prefix](const auto&, size_t) { + std::string line; + std::getline(std::istream(&buffer), line); + logger.log(prefix + line); + + log_lines(logger, pipe, buffer, prefix); + }); +} + +std::string create_logger_prefix(const fs::path& socket_path) { + // The group socket filename will be in the format + // '/tmp/yabridge-group---.sock', + // where Wine prefix ID is just Wine prefix ran through `std::hash` to + // prevent collisions without needing complicated filenames. We want to + // extract the group name. + std::string socket_name = + socket_path.filename().replace_extension().string(); + + std::smatch group_match; + std::regex group_regexp("^yabridge-group-(.*)-[^-]+-[^-]+$", + std::regex::ECMAScript); + if (std::regex_match(socket_name, group_match, group_regexp)) { + socket_name = group_match[1].str(); + } + + return "[" + socket_name + "] "; } From 53acb1f78a0a12cf06760df7a49c931af0c73d2d Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 18 May 2020 15:13:13 +0200 Subject: [PATCH 18/73] Move wine-bridge.h -> bridges/vst2.h This way we can structure the group handling and a potential future VST3 bridge in the same way. --- README.md | 2 +- meson.build | 2 +- src/plugin/plugin-bridge.cpp | 2 +- src/plugin/plugin-bridge.h | 2 +- .../{wine-bridge.cpp => bridges/vst2.cpp} | 42 +++++++++---------- .../{wine-bridge.h => bridges/vst2.h} | 18 ++++---- src/wine-host/group-host.cpp | 2 +- src/wine-host/individual-host.cpp | 4 +- 8 files changed, 37 insertions(+), 37 deletions(-) rename src/wine-host/{wine-bridge.cpp => bridges/vst2.cpp} (94%) rename src/wine-host/{wine-bridge.h => bridges/vst2.h} (95%) diff --git a/README.md b/README.md index 0580ff21..b3c4cf78 100644 --- a/README.md +++ b/README.md @@ -419,7 +419,7 @@ as the _Windows VST plugin_. The whole process works as follows: in the `DispatchDataConverter` and `HostCallbackDataCovnerter` classes for the `dispatcher()` and `audioMaster()` functions respectively. For operations involving the plugin editor there is also some extra glue in - `WineBridge::dispatch_wrapper`. On the receiving end of the function calls, + `Vst2Bridge::dispatch_wrapper`. On the receiving end of the function calls, the `passthrough_event()` function which calls the callback functions and handles the marshalling between our data types created by the `*DataConverter` classes and the VST API's different pointer types. This diff --git a/meson.build b/meson.build index fa04a7a2..ed88f111 100644 --- a/meson.build +++ b/meson.build @@ -86,10 +86,10 @@ shared_library( host_sources = [ 'src/common/logging.cpp', 'src/common/serialization.cpp', + 'src/wine-host/bridges/vst2.cpp', 'src/wine-host/editor.cpp', 'src/wine-host/editor.cpp', 'src/wine-host/utils.cpp', - 'src/wine-host/wine-bridge.cpp', version_header, ] diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index d9e08ac3..5f463605 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -165,7 +165,7 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) try { while (true) { // TODO: Think of a nicer way to structure this and the similar - // handler in `WineBridge::handle_dispatch_midi_events` + // handler in `Vst2Bridge::handle_dispatch_midi_events` receive_event( vst_host_callback, std::pair(logger, false), [&](Event& event) { diff --git a/src/plugin/plugin-bridge.h b/src/plugin/plugin-bridge.h index 8b7672fb..d26aff36 100644 --- a/src/plugin/plugin-bridge.h +++ b/src/plugin/plugin-bridge.h @@ -65,7 +65,7 @@ class PluginBridge { * Ask the VST plugin to process audio for us. If the plugin somehow does * not support `processReplacing()` and only supports the old `process()` * function, then this will be handled implicitely in - * `WineBridge::handle_process_replacing()`. + * `Vst2Bridge::handle_process_replacing()`. */ void process_replacing(AEffect* plugin, float** inputs, diff --git a/src/wine-host/wine-bridge.cpp b/src/wine-host/bridges/vst2.cpp similarity index 94% rename from src/wine-host/wine-bridge.cpp rename to src/wine-host/bridges/vst2.cpp index 2adc9f39..22a6c861 100644 --- a/src/wine-host/wine-bridge.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -14,12 +14,12 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -#include "wine-bridge.h" +#include "vst2.h" #include -#include "../common/communication.h" -#include "../common/events.h" +#include "../../common/communication.h" +#include "../../common/events.h" /** * A function pointer to what should be the entry point of a VST plugin. @@ -30,7 +30,7 @@ using VstEntryPoint = AEffect*(VST_CALL_CONV*)(audioMasterCallback); * This ugly global is needed so we can get the instance of a `Brdige` class * from an `AEffect` when it performs a host callback during its initialization. */ -WineBridge* current_bridge_instance = nullptr; +Vst2Bridge* current_bridge_instance = nullptr; intptr_t VST_CALL_CONV host_callback_proxy(AEffect*, int, int, intptr_t, void*, float); @@ -43,12 +43,12 @@ uint32_t WINAPI handle_parameters_proxy(void*); uint32_t WINAPI handle_process_replacing_proxy(void*); /** - * Fetch the WineBridge instance stored in one of the two pointers reserved + * Fetch the Vst2Bridge instance stored in one of the two pointers reserved * for the host of the hosted VST plugin. This is sadly needed as a workaround * to avoid using globals since we need free function pointers to interface with * the VST C API. */ -WineBridge& get_bridge_instance(const AEffect* plugin) { +Vst2Bridge& get_bridge_instance(const AEffect* plugin) { // This is needed during the initialization of the plugin since we can only // add our own pointer after it's done initializing if (current_bridge_instance != nullptr) { @@ -57,10 +57,10 @@ WineBridge& get_bridge_instance(const AEffect* plugin) { return *current_bridge_instance; } - return *static_cast(plugin->ptr1); + return *static_cast(plugin->ptr1); } -WineBridge::WineBridge(std::string plugin_dll_path, +Vst2Bridge::Vst2Bridge(std::string plugin_dll_path, std::string socket_endpoint_path) : plugin_handle(LoadLibrary(plugin_dll_path.c_str()), FreeLibrary), io_context(), @@ -135,7 +135,7 @@ WineBridge::WineBridge(std::string plugin_dll_path, Win32Thread(handle_process_replacing_proxy, this); } -void WineBridge::handle_dispatch() { +void Vst2Bridge::handle_dispatch() { using namespace std::placeholders; // For our communication we use simple threads and blocking operations @@ -145,7 +145,7 @@ void WineBridge::handle_dispatch() { while (true) { receive_event(host_vst_dispatch, std::nullopt, passthrough_event( - plugin, std::bind(&WineBridge::dispatch_wrapper, + plugin, std::bind(&Vst2Bridge::dispatch_wrapper, this, _1, _2, _3, _4, _5, _6))); // Because of the way the Win32 API works we have to process events @@ -156,7 +156,7 @@ void WineBridge::handle_dispatch() { // run this loop. The only exception is a in specific situation that // can cause a race condition in some plugins because of incorrect // assumptions made by the plugin. See the dostring for - // `WineBridge::editor` for more information. + // `Vst2Bridge::editor` for more information. std::visit(overload{[](Editor& editor) { editor.handle_events(); }, [](std::monostate&) { MSG msg; @@ -170,7 +170,7 @@ void WineBridge::handle_dispatch() { [](EditorOpening&) { // Don't handle any events in this // particular case as explained in - // `WineBridge::editor` + // `Vst2Bridge::editor` }}, editor); } @@ -180,7 +180,7 @@ void WineBridge::handle_dispatch() { } } -[[noreturn]] void WineBridge::handle_dispatch_midi_events() { +[[noreturn]] void Vst2Bridge::handle_dispatch_midi_events() { while (true) { receive_event( host_vst_dispatch_midi_events, std::nullopt, [&](Event& event) { @@ -221,14 +221,14 @@ void WineBridge::handle_dispatch() { // Maybe this should just be a hard error instead, since it // should never happen return passthrough_event( - plugin, std::bind(&WineBridge::dispatch_wrapper, this, + plugin, std::bind(&Vst2Bridge::dispatch_wrapper, this, _1, _2, _3, _4, _5, _6))(event); } }); } } -[[noreturn]] void WineBridge::handle_parameters() { +[[noreturn]] void Vst2Bridge::handle_parameters() { while (true) { // Both `getParameter` and `setParameter` functions are passed // through on this socket since they have a lot of overlap. The @@ -251,7 +251,7 @@ void WineBridge::handle_dispatch() { } } -[[noreturn]] void WineBridge::handle_process_replacing() { +[[noreturn]] void Vst2Bridge::handle_process_replacing() { std::vector> output_buffers(plugin->numOutputs); while (true) { @@ -307,7 +307,7 @@ void WineBridge::handle_dispatch() { } } -intptr_t WineBridge::dispatch_wrapper(AEffect* plugin, +intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, int opcode, int index, intptr_t value, @@ -448,7 +448,7 @@ class HostCallbackDataConverter : DefaultDataConverter { std::optional& time_info; }; -intptr_t WineBridge::host_callback(AEffect* effect, +intptr_t Vst2Bridge::host_callback(AEffect* effect, int opcode, int index, intptr_t value, @@ -470,13 +470,13 @@ intptr_t VST_CALL_CONV host_callback_proxy(AEffect* effect, } uint32_t WINAPI handle_dispatch_midi_events_proxy(void* instance) { - static_cast(instance)->handle_dispatch_midi_events(); + static_cast(instance)->handle_dispatch_midi_events(); } uint32_t WINAPI handle_parameters_proxy(void* instance) { - static_cast(instance)->handle_parameters(); + static_cast(instance)->handle_parameters(); } uint32_t WINAPI handle_process_replacing_proxy(void* instance) { - static_cast(instance)->handle_process_replacing(); + static_cast(instance)->handle_process_replacing(); } diff --git a/src/wine-host/wine-bridge.h b/src/wine-host/bridges/vst2.h similarity index 95% rename from src/wine-host/wine-bridge.h rename to src/wine-host/bridges/vst2.h index b48c2546..fb52af1a 100644 --- a/src/wine-host/wine-bridge.h +++ b/src/wine-host/bridges/vst2.h @@ -16,7 +16,7 @@ #pragma once -#include "boost-fix.h" +#include "../boost-fix.h" #define NOMINMAX #define NOSERVICE @@ -29,23 +29,23 @@ #include #include -#include "../common/logging.h" -#include "editor.h" -#include "utils.h" +#include "../../common/logging.h" +#include "../editor.h" +#include "../utils.h" /** * A marker struct to indicate that the editor is about to be opened. * - * @see WineBridge::editor + * @see Vst2Bridge::editor */ struct EditorOpening {}; /** * This handles the communication between the Linux native VST plugin and the - * Wine VST host. The functions below should be used as callback functions in an - * `AEffect` object. + * Wine VST host when hosting VST2 plugins. The functions below should be used + * as callback functions in an `AEffect` object. */ -class WineBridge { +class Vst2Bridge { public: /** * Initializes the Windows VST plugin and set up communication with the @@ -59,7 +59,7 @@ class WineBridge { * @throw std::runtime_error Thrown when the VST plugin could not be loaded, * or if communication could not be set up. */ - WineBridge(std::string plugin_dll_path, std::string socket_endpoint_path); + Vst2Bridge(std::string plugin_dll_path, std::string socket_endpoint_path); /** * Handle events on the main thread until the plugin quits. This can't be diff --git a/src/wine-host/group-host.cpp b/src/wine-host/group-host.cpp index 76a7450d..be2b8b41 100644 --- a/src/wine-host/group-host.cpp +++ b/src/wine-host/group-host.cpp @@ -29,7 +29,7 @@ #include #include -#include "wine-bridge.h" +#include "bridges/vst2.h" // FIXME: `std::filesystem` is broken in wineg++, at least under Wine 5.8. Any // path operation will thrown an encoding related error. diff --git a/src/wine-host/individual-host.cpp b/src/wine-host/individual-host.cpp index 1d8149a3..5aa8a314 100644 --- a/src/wine-host/individual-host.cpp +++ b/src/wine-host/individual-host.cpp @@ -20,7 +20,7 @@ #include #include -#include "wine-bridge.h" +#include "bridges/vst2.h" /** * This is the default VST host application. It will load the specified VST2 @@ -55,7 +55,7 @@ int __cdecl main(int argc, char* argv[]) { #endif << std::endl; try { - WineBridge bridge(plugin_dll_path, socket_endpoint_path); + Vst2Bridge bridge(plugin_dll_path, socket_endpoint_path); std::cerr << "Finished initializing '" << plugin_dll_path << "'" << std::endl; From 8bd1dc8c50a19f844a64877f9d44d92ec2f2a997 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 18 May 2020 16:01:34 +0200 Subject: [PATCH 19/73] Encapsulate the STDOUT/STDERR capturing --- meson.build | 5 ++- src/wine-host/bridges/group.cpp | 44 +++++++++++++++++++ src/wine-host/bridges/group.h | 78 +++++++++++++++++++++++++++++++++ src/wine-host/group-host.cpp | 24 +++------- 4 files changed, 132 insertions(+), 19 deletions(-) create mode 100644 src/wine-host/bridges/group.cpp create mode 100644 src/wine-host/bridges/group.h diff --git a/meson.build b/meson.build index ed88f111..360fab2e 100644 --- a/meson.build +++ b/meson.build @@ -94,7 +94,10 @@ host_sources = [ ] individual_host_sources = host_sources + ['src/wine-host/individual-host.cpp'] -group_host_sources = host_sources + ['src/wine-host/group-host.cpp'] +group_host_sources = host_sources + [ + 'src/wine-host/bridges/group.cpp', + 'src/wine-host/group-host.cpp', +] executable( individual_host_name_64bit, diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp new file mode 100644 index 00000000..64b1599e --- /dev/null +++ b/src/wine-host/bridges/group.cpp @@ -0,0 +1,44 @@ +// 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 "group.h" + +#include + +StdIoCapture::StdIoCapture(boost::asio::io_context& io_context, + int file_descriptor) + : pipe(io_context), + target_fd(file_descriptor), + original_fd_copy(dup(file_descriptor)) { + // We'll use the second element of these two file descriptors to reopen + // `file_descriptor`, and the first one to read the captured contents from + ::pipe(pipe_fd); + + // We've already created a copy of the original file descriptor, so we can + // reopen it using the newly created pipe + dup2(pipe_fd[1], target_fd); + close(pipe_fd[1]); + + pipe.assign(pipe_fd[0]); +} + +StdIoCapture::~StdIoCapture() { + // Restore the original file descriptor and close the pipe. The other wend + // was already closed in the constructor. + dup2(original_fd_copy, target_fd); + close(original_fd_copy); + close(pipe_fd[0]); +} diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h new file mode 100644 index 00000000..89c6c866 --- /dev/null +++ b/src/wine-host/bridges/group.h @@ -0,0 +1,78 @@ +// 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 "../boost-fix.h" + +#include + +/** + * Encapsulate capturing the STDOUT or STDERR stream by opening a pipe and + * reopening the passed file descriptor as one of the ends of the newly opened + * pipe. This allows all output sent to be read from that pipe. This is needed + * to capture all (debug) output from Wine and the hosted plugins so we can + * prefix it with a timestamp and a group identifier and potentially write it to + * a log file. Since the host application is run independently of the yabridge + * instance that spawned it, this can't simply be done by the caller like we're + * doing for Wine output in individually hosted plugins. + */ +class StdIoCapture { + public: + /** + * Redirect all output sent to a file descriptor (e.g. `STDOUT_FILENO` or + * `STDERR_FILENO`) to a pipe. `StdIoCapture::pipe` can be used to read from + * this pipe. + * + * @param io_context The IO context to create the captured pipe stream on. + * @param file_descriptor The file descriptor to remap. + */ + StdIoCapture(boost::asio::io_context& io_context, int file_descriptor); + + StdIoCapture(const StdIoCapture&) = delete; + StdIoCapture& operator=(const StdIoCapture&) = delete; + + /** + * On cleanup, close the outgoing file descriptor from the pipe and restore + * the original file descriptor for the captured stream. + */ + ~StdIoCapture(); + + /** + * The pipe endpoint where all output from the original file descriptor gets + * redirected to. This can be read from like any other `Boost.Asio` stream. + */ + boost::asio::posix::stream_descriptor pipe; + + private: + /** + * The file descriptor of the stream we're capturing. + */ + const int target_fd; + + /** + * A copy of the original file descriptor. Will be used to undo + * the capture when this object gets destroyed. + */ + const int original_fd_copy; + + /** + * The two file descriptors generated by the `pipe()` function call. + * `pipe_fd[1]` is used to reopen/capture the passed file descriptor, and + * `pipe_fd[0]` can be used to read the captured output from. + */ + int pipe_fd[2]; +}; diff --git a/src/wine-host/group-host.cpp b/src/wine-host/group-host.cpp index be2b8b41..ff846183 100644 --- a/src/wine-host/group-host.cpp +++ b/src/wine-host/group-host.cpp @@ -16,8 +16,6 @@ #include "boost-fix.h" -#include -#include #include #include #include @@ -29,6 +27,7 @@ #include #include +#include "bridges/group.h" #include "bridges/vst2.h" // FIXME: `std::filesystem` is broken in wineg++, at least under Wine 5.8. Any @@ -97,25 +96,14 @@ int __cdecl main(int argc, char* argv[]) { // process the output and redirect it to a logger. Needed to capture Wine // debug output, since this process will likely outlive the yabridge // instance that originally spawned it. - int stdout_pipe[2]; - pipe(stdout_pipe); - dup2(stdout_pipe[1], STDOUT_FILENO); - close(stdout_pipe[1]); - - int stderr_pipe[2]; - pipe(stderr_pipe); - dup2(stderr_pipe[1], STDERR_FILENO); - close(stderr_pipe[1]); - boost::asio::io_context io_context; boost::asio::streambuf stdout_buffer; boost::asio::streambuf stderr_buffer; - boost::asio::posix::stream_descriptor stdout_redirect(io_context, - stdout_pipe[0]); - boost::asio::posix::stream_descriptor stderr_redirect(io_context, - stderr_pipe[0]); - log_lines(logger, stdout_redirect, stdout_buffer, "[STDOUT] "); - log_lines(logger, stderr_redirect, stderr_buffer, "[STDERR] "); + StdIoCapture stdout_redirect(io_context, STDOUT_FILENO); + StdIoCapture stderr_redirect(io_context, STDERR_FILENO); + log_lines(logger, stdout_redirect.pipe, stdout_buffer, "[STDOUT] "); + log_lines(logger, stderr_redirect.pipe, stderr_buffer, "[STDERR] "); + std::thread io_handler([&]() { io_context.run(); }); logger.log("Initializing yabridge group host version " + From b4838f8d188305238866e6c9203168a511ffcd34 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 18 May 2020 17:12:54 +0200 Subject: [PATCH 20/73] Update the version yabridge has been tested with --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b3c4cf78..4136663d 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ easy to debug and maintain. ## Tested with Yabridge has been verified to work correctly in the following VST hosts using -Wine Staging 5.5 and 5.6: +Wine Staging 5.8: - Bitwig Studio 3.1 and the beta releases of 3.2 - Carla 2.1 From 3985e1031925d12606382c2d9a1bd5f9712294d7 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 19 May 2020 10:44:59 +0200 Subject: [PATCH 21/73] Add search paths for 32-bit Boost on Fedora This will also cause the 64-bit version of the library to be picked up on any distro that's not based on Fedora, which may cause some confusing problems. Is there a right way to do this? Meson doesn't allow you to override the library search path for part of the build, and it also can't differentiate between 32-bit and 64-bit libraries by default. --- meson.build | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/meson.build b/meson.build index 360fab2e..83da7c3a 100644 --- a/meson.build +++ b/meson.build @@ -136,17 +136,27 @@ executable( if get_option('use-bitbridge') message('Bitbridge enabled, configuring a 32-bit host application') - # I honestly have no idea what the correct way to have `find_dependency()` use - # `/usr/lib32` instead of `/usr/lib` is. If anyone does know, please tell me! + # I honestly have no idea what the correct way is to have `dependency()` or + # `compiler.find_dependency()` search for 32-bit versions of libraries when + # cross-compiling. Meson also doesn't seem to respect the default linker + # search path set by the system in `find_library()`. If anyone does know how + # to properly do this, please let me know! winegcc = meson.get_compiler('cpp', native : false) boost_filesystem_dep = winegcc.find_library( 'boost_filesystem', static : true, dirs : [ - '/usr/lib32', - '/usr/lib/i386-linux-gnu', + # Used by Arch based distros '/usr/local/lib32', - '/usr/local/lib/i386-linux-gnu' + '/usr/lib32', + # Used by Debian based distros + '/usr/local/lib/i386-linux-gnu', + '/usr/lib/i386-linux-gnu', + # Used by Red Hat based distros, could cause issues though since Meson + # cannot differentiate between the 32-bit version and the regular 64-bit + # version that would normally be in /lib + '/usr/local/lib', + '/usr/lib', ] ) xcb_dep = winegcc.find_library('xcb') From c2fceb6e4af58d9f8ebf3d1006b889a923eacd5f Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 19 May 2020 11:33:34 +0200 Subject: [PATCH 22/73] Add a thread safety warning to write_object() --- src/common/communication.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/common/communication.h b/src/common/communication.h index 8cea096e..22ac3516 100644 --- a/src/common/communication.h +++ b/src/common/communication.h @@ -41,6 +41,10 @@ using InputAdapter = bitsery::InputBufferAdapter; * @param buffer The buffer to write to. This is useful for sending audio and * chunk data since that can vary in size by a lot. * + * @warning This operation is not atomic, and calling this function with the + * same socket from multiple threads at once will cause issues with the + * packets arriving out of order. + * * @relates read_object */ template From daad6f2f003a2400ae1333b0c809acae6f1b3a9a Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 19 May 2020 12:27:32 +0200 Subject: [PATCH 23/73] Fix empty line spam in log file when Wine crashes --- CHANGELOG.md | 2 ++ src/plugin/plugin-bridge.cpp | 15 +++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0c72541..969aa638 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed +- Fixed large amount of empty lines in the log file when the Wine process closes + unexpectedly. - Made the plugin and host detection slightly more robust. ## [1.1.4] - 2020-05-12 diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 5f463605..5a7b1eb6 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -612,16 +612,19 @@ void PluginBridge::async_log_pipe_lines(patched_async_pipe& pipe, boost::asio::streambuf& buffer, std::string prefix) { boost::asio::async_read_until( - pipe, buffer, '\n', [&, prefix](const auto&, size_t) { + pipe, buffer, '\n', + [&, prefix](const boost::system::error_code& error, size_t) { + // When we get an error code then that likely means that the pipe + // has been clsoed and we have reached the end of the file + if (error.failed()) { + return; + } + std::string line; std::getline(std::istream(&buffer), line); logger.log(prefix + line); - // Not sure why, but this async read will keep reading a ton of - // empty lines after the Wine process crashes - if (vst_host.running()) { - async_log_pipe_lines(pipe, buffer, prefix); - } + async_log_pipe_lines(pipe, buffer, prefix); }); } From 6d6d928838789e3c82e6d9c14c87666e5633729e Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 19 May 2020 15:29:48 +0200 Subject: [PATCH 24/73] Move all plugin group handling boilerplate --- src/plugin/plugin-bridge.cpp | 1 + src/wine-host/bridges/group.cpp | 93 +++++++++++++++++++++ src/wine-host/bridges/group.h | 143 ++++++++++++++++++++++++++++++++ src/wine-host/group-host.cpp | 121 +++++---------------------- 4 files changed, 258 insertions(+), 100 deletions(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 5a7b1eb6..dfcdb272 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -121,6 +121,7 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) std::thread([&]() { using namespace std::literals::chrono_literals; + // TODO: Figure out how this check would work with plugin gruops while (true) { if (finished_accepting_sockets) { return; diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 64b1599e..4a547368 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -17,6 +17,17 @@ #include "group.h" #include +#include +#include + +// FIXME: `std::filesystem` is broken in wineg++, at least under Wine 5.8. Any +// path operation will thrown an encoding related error +namespace fs = boost::filesystem; + +/** + * Create a logger prefix containing the group name based on the socket path. + */ +std::string create_logger_prefix(const fs::path& socket_path); StdIoCapture::StdIoCapture(boost::asio::io_context& io_context, int file_descriptor) @@ -42,3 +53,85 @@ StdIoCapture::~StdIoCapture() { close(original_fd_copy); close(pipe_fd[0]); } + +bool PluginParameters::operator==(const PluginParameters& other) const { + return removeme == other.removeme; +} + +GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) + : logger(Logger::create_from_environment( + create_logger_prefix(group_socket_path))), + io_context(), + stdout_redirect(io_context, STDOUT_FILENO), + stderr_redirect(io_context, STDERR_FILENO), + group_socket_endpoint(group_socket_path.string()), + group_socket_acceptor(io_context, group_socket_endpoint) { + // TODO: After initializing, listen for connections and spawn plugins + // the exact same way as what happens in `individual-host.cpp` + // TODO: Allow this process to exit when the last plugin exits. Make sure + // that that doesn't cause any race conditions. + + // Write this process's original STDOUT and STDERR streams to the logger + async_log_pipe_lines(stdout_redirect.pipe, stdout_buffer, "[STDOUT] "); + async_log_pipe_lines(stderr_redirect.pipe, stderr_buffer, "[STDERR] "); +} + +void GroupBridge::handle_host_plugin(const PluginParameters& parameters) { + // TODO: Start the plugin, + // TODO: Allow this process to exit when the last plugin exits. Make sure + // that that doesn't cause any race conditions. +} + +void GroupBridge::handle_incoming_connections() { + // TODO: Accept connections here + + logger.log("Now accepting incoming connections"); + io_context.run(); +} + +void GroupBridge::async_log_pipe_lines( + boost::asio::posix::stream_descriptor& pipe, + boost::asio::streambuf& buffer, + std::string prefix) { + boost::asio::async_read_until( + pipe, buffer, '\n', + [&, prefix](const boost::system::error_code& error, size_t) { + // When we get an error code then that likely means that the pipe + // has been clsoed and we have reached the end of the file + if (error.failed()) { + return; + } + + std::string line; + std::getline(std::istream(&buffer), line); + logger.log(prefix + line); + + async_log_pipe_lines(pipe, buffer, prefix); + }); +} + +std::string create_logger_prefix(const fs::path& socket_path) { + // The group socket filename will be in the format + // '/tmp/yabridge-group---.sock', + // where Wine prefix ID is just Wine prefix ran through `std::hash` to + // prevent collisions without needing complicated filenames. We want to + // extract the group name. + std::string socket_name = + socket_path.filename().replace_extension().string(); + + std::smatch group_match; + std::regex group_regexp("^yabridge-group-(.*)-[^-]+-[^-]+$", + std::regex::ECMAScript); + if (std::regex_match(socket_name, group_match, group_regexp)) { + socket_name = group_match[1].str(); + +#ifdef __i386__ + // Mark 32-bit versions to avoid potential confusion caused by 32-bit + // and regular 64-bit group processes with the same name running + // alongside eachother + socket_name += "-x32"; +#endif + } + + return "[" + socket_name + "] "; +} diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index 89c6c866..c6a5c542 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -19,6 +19,25 @@ #include "../boost-fix.h" #include +#include +#include + +#include "vst2.h" + +// TODO: Replace this with a proper struct that contains the required arguments +// for creating a PluginBridge instance +struct PluginParameters { + int removeme; + + bool operator==(const PluginParameters& p) const; +}; + +template <> +struct std::hash { + std::size_t operator()(PluginParameters const& params) const noexcept { + return std::hash{}(params.removeme); + } +}; /** * Encapsulate capturing the STDOUT or STDERR stream by opening a pipe and @@ -76,3 +95,127 @@ class StdIoCapture { */ int pipe_fd[2]; }; + +/** + * A 'plugin group' that listens on a _group socket_ for plugins to host in this + * process. Once the plugin gets loaded into a new thread the actual bridging + * process is identical to individually hosted plugins. + * + * An important detail worth mentioning here is that while this plugin group can + * throw in the constructor when another process is already listening on the + * socket, this should not be treated as an error. When using plugins groups, + * yabridge will try to connect to the group socket on initialization and it + * will launch a new group host process if it can't. If this is done for + * multiple yabridge instances at the same time, then multiple group host + * processes will be launched. Instead of using complicated inter-process + * synchronization, we'll simply allow the processes to fail when another + * process is already listening on the socket. + */ +class GroupBridge { + public: + /** + * Create a plugin group by listening on the provided socket for incoming + * plugin host requests. + * + * @param gruop_socket_path The path to the group socket endpoint. This path + * should be in the form of + * `/tmp/yabridge-group---` + * where `` is a numerical hash as explained in the + * `create_logger_prefix()` function in `./group.cpp`. + * + * @note Creating an `GroupBridge` instance has the side effect that the + * STDOUT and STDERR streams of the current process will be redirected to + * a pipe so they can be properly written to a log file. + */ + GroupBridge(boost::filesystem::path group_socket_path); + + /** + * Host a new plugin within this process. Called by proxy using + * `handle_host_plugin_proxy()` in `./group.cpp` because the Win32 + * `CreateThread` API only allows passing a single pointer to the function. + * Because we don't have access to our own thread handle, the thread that's + * listening on the group socket and that has created this thread will also + * add this thread to the `active_plugins` map. + * + * Once the plugin has exited, this thread will then remove itself from the + * `active_plugins` map. If this causes the vector to become empty, we will + * terminate this process. + * + * @param parameters Information about the plugin to launch, i.e. the path + * to the plugin and the path of the socket endpoint that will be used for + * communication. + * + * @note In the case that the process starts but no plugin gets initiated, + * then the process will never exit on its own. This should not happen + * though. + */ + void handle_host_plugin(const PluginParameters& parameters); + + /** + * Listen for new requests to spawn plugins within this process and handle + * them accordingly. Will terminate once all plugins have exited. + */ + void handle_incoming_connections(); + + private: + /** + * Continuously read from a pipe and write the output to the log file. Used + * with the IO streams captured by `stdout_redirect` and `stderr_redirect`. + * + * TODO: Merge this with `PluginBridge::async_log_pipe_lines` + * + * @param pipe The pipe to read from. + * @param buffer The stream buffer to write to. + * @param prefix Text to prepend to the line before writing to the log. + */ + void async_log_pipe_lines(boost::asio::posix::stream_descriptor& pipe, + boost::asio::streambuf& buffer, + std::string prefix); + + /** + * The logging facility used for this group host process. Since we can't + * identify which plugin is generating (debug) output, every line will only + * be prefixed with the name of the group. + */ + Logger logger; + + boost::asio::io_context io_context; + + boost::asio::streambuf stdout_buffer; + boost::asio::streambuf stderr_buffer; + /** + * Contains a pipe used for capturing this process's STDOUT stream. Needed + * to be able to process the output generated by Wine and plugins and to be + * able write it write it to an external log file. + */ + StdIoCapture stdout_redirect; + /** + * Contains a pipe used for capturing this process's STDERR stream. Needed + * to be able to process the output generated by Wine and plugins and to be + * able write it write it to an external log file. + */ + StdIoCapture stderr_redirect; + + boost::asio::local::stream_protocol::endpoint group_socket_endpoint; + /** + * The UNIX domain socket acceptor that will be used to listen for incoming + * connections to spawn new plugins within this process. + */ + boost::asio::local::stream_protocol::acceptor group_socket_acceptor; + + /** + * A map of threads that are currently hosting a plugin within this process. + * After a plugin has exited or its initialization has failed, the thread + * handling it will remove itself from this map. + */ + std::unordered_map>> + active_plugins; + /** + * A mutex to prevent two threads from simultaneously accessing the plugins + * map, and also to prevent `handle_host_plugin()` from terminating the + * process because it thinks there are no active plugins left just as a new + * plugin is being spawned. + */ + std::mutex active_plugins_mutex; +}; diff --git a/src/wine-host/group-host.cpp b/src/wine-host/group-host.cpp index ff846183..38b8d7f4 100644 --- a/src/wine-host/group-host.cpp +++ b/src/wine-host/group-host.cpp @@ -16,12 +16,7 @@ #include "boost-fix.h" -#include -#include -#include #include -#include -#include // Generated inside of build directory #include @@ -30,27 +25,6 @@ #include "bridges/group.h" #include "bridges/vst2.h" -// FIXME: `std::filesystem` is broken in wineg++, at least under Wine 5.8. Any -// path operation will thrown an encoding related error. -namespace fs = boost::filesystem; - -// TODO: Move most plumbing to another file - -/** - * Create a logger prefix containing the group name based on the socket path. - */ -std::string create_logger_prefix(const fs::path& socket_path); - -/** - * Continuously Read from a stream and write the lines to a logger instance. - * - * TODO: Merge this with the other similar function in `PluginBridge` - */ -void log_lines(Logger& logger, - boost::asio::posix::stream_descriptor& pipe, - boost::asio::streambuf& buffer, - std::string prefix); - /** * This works very similar to the host application defined in * `individual-host.cpp`, but instead of just loading a single plugin this will @@ -82,84 +56,31 @@ int __cdecl main(int argc, char* argv[]) { const std::string group_socket_endpoint_path(argv[1]); - // TODO: Before doing anything, try listening on the socket and fail - // silently (or log a message?) if another application is already - // listening on the socket. This way we don't need any complicated - // inter-process synchronization to ensure that there is a single - // active group host listening for this group. - - // Has to be initialized before redirecting the STDIO streams - Logger logger = Logger::create_from_environment( - create_logger_prefix(group_socket_endpoint_path)); - - // Redirect this process's STDOUT and STDERR streams to a pipe so we can - // process the output and redirect it to a logger. Needed to capture Wine - // debug output, since this process will likely outlive the yabridge - // instance that originally spawned it. - boost::asio::io_context io_context; - boost::asio::streambuf stdout_buffer; - boost::asio::streambuf stderr_buffer; - StdIoCapture stdout_redirect(io_context, STDOUT_FILENO); - StdIoCapture stderr_redirect(io_context, STDERR_FILENO); - log_lines(logger, stdout_redirect.pipe, stdout_buffer, "[STDOUT] "); - log_lines(logger, stderr_redirect.pipe, stderr_buffer, "[STDERR] "); - - std::thread io_handler([&]() { io_context.run(); }); - - logger.log("Initializing yabridge group host version " + - std::string(yabridge_git_version) + // TODO: Catch exception during initialization when another process is + // already listening on the socket. Make sure to print a more useful + // message instead. + try { + std::cerr << "Initializing yabridge group host version " + << yabridge_git_version #ifdef __i386__ - + " (32-bit compatibility mode)" + << " (32-bit compatibility mode)" #endif - ); + << std::endl; - // TODO: Remove debug prints - printf("This should be caught now!\n"); - std::cerr << "This too!" << std::endl; + GroupBridge bridge(group_socket_endpoint_path); - // TODO: After initializing, listen for connections and spawn plugins - // the exact same way as what happens in `individual-host.cpp` - // TODO: Allow this process to exit when the last plugin exits. Make sure - // that that doesn't cause any race conditions. + // Blocks the main thread until all plugins have exited + bridge.handle_incoming_connections(); + } catch (const std::runtime_error& error) { + // Even though the process will likely outlive the yabridge instance + // that spawns it, we can still print any initialization messages and + // errors to STDERR since at this point there will still be a yabridge + // instance capturing this process's output. + // TODO: Check if this is printed on the right stream + std::cerr << "Error while initializing the group host process:" + << std::endl; + std::cerr << error.what() << std::endl; - // TODO: This usleep() is just to ensure that the second print to stderr - // also gets processed before stopping the IO context since we're - // immediately stopping it after starting. This would not needed in - // normal use. - usleep(1000); - io_context.stop(); - io_handler.join(); -} - -void log_lines(Logger& logger, - boost::asio::posix::stream_descriptor& pipe, - boost::asio::streambuf& buffer, - std::string prefix) { - boost::asio::async_read_until(pipe, buffer, '\n', - [&, prefix](const auto&, size_t) { - std::string line; - std::getline(std::istream(&buffer), line); - logger.log(prefix + line); - - log_lines(logger, pipe, buffer, prefix); - }); -} - -std::string create_logger_prefix(const fs::path& socket_path) { - // The group socket filename will be in the format - // '/tmp/yabridge-group---.sock', - // where Wine prefix ID is just Wine prefix ran through `std::hash` to - // prevent collisions without needing complicated filenames. We want to - // extract the group name. - std::string socket_name = - socket_path.filename().replace_extension().string(); - - std::smatch group_match; - std::regex group_regexp("^yabridge-group-(.*)-[^-]+-[^-]+$", - std::regex::ECMAScript); - if (std::regex_match(socket_name, group_match, group_regexp)) { - socket_name = group_match[1].str(); + return 1; } - - return "[" + socket_name + "] "; } From 8eb01cb51999627f5aed47d974b4b1d6f5f94f5c Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 20 May 2020 18:45:33 +0200 Subject: [PATCH 25/73] Listen on the group socket and handle requests --- src/common/serialization.cpp | 4 ++ src/common/serialization.h | 27 ++++++++++++ src/plugin/plugin-bridge.cpp | 7 +++- src/wine-host/bridges/group.cpp | 70 ++++++++++++++++++++++++++++--- src/wine-host/bridges/group.h | 44 ++++++++++--------- src/wine-host/group-host.cpp | 28 ++++++------- src/wine-host/individual-host.cpp | 1 + 7 files changed, 136 insertions(+), 45 deletions(-) diff --git a/src/common/serialization.cpp b/src/common/serialization.cpp index cb9662d3..c16b0142 100644 --- a/src/common/serialization.cpp +++ b/src/common/serialization.cpp @@ -108,3 +108,7 @@ AEffect& update_aeffect(AEffect& plugin, const AEffect& updated_plugin) { return plugin; } + +bool PluginParameters::operator==(const PluginParameters& rhs) const { + return plugin_path == rhs.plugin_path && socket_path == rhs.socket_path; +} diff --git a/src/common/serialization.h b/src/common/serialization.h index 2c1fe7f7..b0810dc7 100644 --- a/src/common/serialization.h +++ b/src/common/serialization.h @@ -571,3 +571,30 @@ struct AudioBuffers { s.value4b(sample_frames); } }; + +/** + * An object containing the startup options for hosting a plugin in a plugin + * group process. These are the exact same options that would have been passed + * to `yabridge-host.exe` were the plugin to be hosted individually. + */ +struct PluginParameters { + std::string plugin_path; + std::string socket_path; + + bool operator==(const PluginParameters& rhs) const; + + template + void serialize(S& s) { + s.text1b(plugin_path, 4096); + s.text1b(socket_path, 4096); + } +}; + +template <> +struct std::hash { + std::size_t operator()(PluginParameters const& params) const noexcept { + std::hash hasher{}; + + return hasher(params.plugin_path) ^ (hasher(params.socket_path) << 1); + } +}; diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index dfcdb272..25414b29 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -121,7 +121,12 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) std::thread([&]() { using namespace std::literals::chrono_literals; - // TODO: Figure out how this check would work with plugin gruops + // TODO: For plugin groups, we should be polling whether someone is + // still listening on the group socket (and ideally we should be + // able to tell it's the same process). If we can't figure out a + // better way, then we could just return the PID when sending the + // host request to the group process and check whether that + // process is still running. while (true) { if (finished_accepting_sockets) { return; diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 4a547368..ef5aa6ab 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -20,6 +20,8 @@ #include #include +#include "../../common/communication.h" + // FIXME: `std::filesystem` is broken in wineg++, at least under Wine 5.8. Any // path operation will thrown an encoding related error namespace fs = boost::filesystem; @@ -29,6 +31,13 @@ namespace fs = boost::filesystem; */ std::string create_logger_prefix(const fs::path& socket_path); +// CreateThread() is great and allows you to pass a single value to the +// function, so we'll use this to pass both `this` and the parameters to the +// below thread function so it can do its thing. +using handle_host_plugin_parameters = std::pair; + +uint32_t WINAPI handle_host_plugin_proxy(void* param); + StdIoCapture::StdIoCapture(boost::asio::io_context& io_context, int file_descriptor) : pipe(io_context), @@ -54,10 +63,6 @@ StdIoCapture::~StdIoCapture() { close(pipe_fd[0]); } -bool PluginParameters::operator==(const PluginParameters& other) const { - return removeme == other.removeme; -} - GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) : logger(Logger::create_from_environment( create_logger_prefix(group_socket_path))), @@ -77,18 +82,58 @@ GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) } void GroupBridge::handle_host_plugin(const PluginParameters& parameters) { - // TODO: Start the plugin, + // TODO: Start the plugin // TODO: Allow this process to exit when the last plugin exits. Make sure // that that doesn't cause any race conditions. } void GroupBridge::handle_incoming_connections() { - // TODO: Accept connections here + accept_requests(); logger.log("Now accepting incoming connections"); io_context.run(); } +void GroupBridge::accept_requests() { + group_socket_acceptor.async_accept( + [&](const boost::system::error_code& error, + boost::asio::local::stream_protocol::socket socket) { + // Stop the whole process when the socket gets closed unexpectedly + if (error.failed()) { + logger.log("Error while listening for incoming connections:"); + logger.log(error.message()); + + io_context.stop(); + } + + // Read the parameters, and then host the plugin in this process + // just like if we would be hosting the plugin individually through + // `yabridge-hsot.exe`. Since we're using sockets there's no reason + // to send an acknowledgement back. One potential issue here is that + // it will be hard to tell whether a plugin has crashed before + // initialization, and that the sockets will never be accepted. For + // individually hosted plugins we poll whether the Wine process is + // still active so we can terminate early if it is not, but in this + // case the yabridge instance has to determine that this process is + // still running. + const auto parameters = read_object(socket); + + // Collisions in the generated socket names should be very rare, but + // it could in theory happen + std::lock_guard lock(active_plugins_mutex); + assert(active_plugins.find(parameters) != active_plugins.end()); + + // CreateThread() doesn't support multiple arguments and requires + // manualy memory management. + handle_host_plugin_parameters* thread_params = + new std::pair(this, parameters); + active_plugins[parameters] = + Win32Thread(handle_host_plugin_proxy, &thread_params); + + accept_requests(); + }); +} + void GroupBridge::async_log_pipe_lines( boost::asio::posix::stream_descriptor& pipe, boost::asio::streambuf& buffer, @@ -135,3 +180,16 @@ std::string create_logger_prefix(const fs::path& socket_path) { return "[" + socket_name + "] "; } + +uint32_t WINAPI handle_host_plugin_proxy(void* param) { + // The Win32 API only allows you to pass a void pointer to threads, so we + // need to use manual memory management. + auto thread_params = static_cast(param); + GroupBridge* instance = thread_params->first; + PluginParameters& parameters = thread_params->second; + delete thread_params; + + instance->handle_host_plugin(parameters); + + return 0; +} diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index c6a5c542..fe38f580 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -24,21 +24,6 @@ #include "vst2.h" -// TODO: Replace this with a proper struct that contains the required arguments -// for creating a PluginBridge instance -struct PluginParameters { - int removeme; - - bool operator==(const PluginParameters& p) const; -}; - -template <> -struct std::hash { - std::size_t operator()(PluginParameters const& params) const noexcept { - return std::hash{}(params.removeme); - } -}; - /** * Encapsulate capturing the STDOUT or STDERR stream by opening a pipe and * reopening the passed file descriptor as one of the ends of the newly opened @@ -123,6 +108,8 @@ class GroupBridge { * where `` is a numerical hash as explained in the * `create_logger_prefix()` function in `./group.cpp`. * + * @throw boost::system::system_error If we can't listen on the socket. + * * @note Creating an `GroupBridge` instance has the side effect that the * STDOUT and STDERR streams of the current process will be redirected to * a pipe so they can be properly written to a log file. @@ -132,10 +119,10 @@ class GroupBridge { /** * Host a new plugin within this process. Called by proxy using * `handle_host_plugin_proxy()` in `./group.cpp` because the Win32 - * `CreateThread` API only allows passing a single pointer to the function. - * Because we don't have access to our own thread handle, the thread that's - * listening on the group socket and that has created this thread will also - * add this thread to the `active_plugins` map. + * `CreateThread` API only allows passing a single pointer to the function + * and does not allow lambdas. Because we don't have access to our own + * thread handle, the thread that's listening on the group socket will have + * already added this thread to the `active_plugins` map. * * Once the plugin has exited, this thread will then remove itself from the * `active_plugins` map. If this causes the vector to become empty, we will @@ -158,6 +145,17 @@ class GroupBridge { void handle_incoming_connections(); private: + /** + * Listen on the group socket for incoming requests to host a new plugin + * within this group process. This will asynchronously listen on the socket, + * and for any connection made it will retrieve a `PluginParameters` object + * containing information about the plugin to host and then spawn a new + * thread to start hosting that plugin. + * + * @see handle_host_plugin + */ + void accept_requests(); + /** * Continuously read from a pipe and write the output to the log file. Used * with the IO streams captured by `stdout_redirect` and `stderr_redirect`. @@ -206,11 +204,11 @@ class GroupBridge { /** * A map of threads that are currently hosting a plugin within this process. * After a plugin has exited or its initialization has failed, the thread - * handling it will remove itself from this map. + * handling it will remove itself from this map. This is to keep track of + * the amount of plugins currently running with their associated thread + * handles. */ - std::unordered_map>> - active_plugins; + std::unordered_map active_plugins; /** * A mutex to prevent two threads from simultaneously accessing the plugins * map, and also to prevent `handle_host_plugin()` from terminating the diff --git a/src/wine-host/group-host.cpp b/src/wine-host/group-host.cpp index 38b8d7f4..4f1b9950 100644 --- a/src/wine-host/group-host.cpp +++ b/src/wine-host/group-host.cpp @@ -56,31 +56,29 @@ int __cdecl main(int argc, char* argv[]) { const std::string group_socket_endpoint_path(argv[1]); - // TODO: Catch exception during initialization when another process is - // already listening on the socket. Make sure to print a more useful - // message instead. - try { - std::cerr << "Initializing yabridge group host version " - << yabridge_git_version + std::cerr << "Initializing yabridge group host version " + << yabridge_git_version #ifdef __i386__ - << " (32-bit compatibility mode)" + << " (32-bit compatibility mode)" #endif - << std::endl; + << std::endl; + try { GroupBridge bridge(group_socket_endpoint_path); // Blocks the main thread until all plugins have exited bridge.handle_incoming_connections(); - } catch (const std::runtime_error& error) { - // Even though the process will likely outlive the yabridge instance - // that spawns it, we can still print any initialization messages and - // errors to STDERR since at this point there will still be a yabridge - // instance capturing this process's output. + } catch (const boost::system::system_error& error) { + // If another process is already listening on the socket, we'll just + // print a message and exit quietly. This could happen if the host + // starts multiple yabridge instances that all use the same plugin group + // at the same time. // TODO: Check if this is printed on the right stream - std::cerr << "Error while initializing the group host process:" + std::cerr << "Another process is already listening on this group's " + "socket, connecting to the existing process:" << std::endl; std::cerr << error.what() << std::endl; - return 1; + return 0; } } diff --git a/src/wine-host/individual-host.cpp b/src/wine-host/individual-host.cpp index 5aa8a314..acd50b7e 100644 --- a/src/wine-host/individual-host.cpp +++ b/src/wine-host/individual-host.cpp @@ -54,6 +54,7 @@ int __cdecl main(int argc, char* argv[]) { << " (32-bit compatibility mode)" #endif << std::endl; + try { Vst2Bridge bridge(plugin_dll_path, socket_endpoint_path); std::cerr << "Finished initializing '" << plugin_dll_path << "'" From 17cff5722e376d3a405df0b6b79833cb26c3290f Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 21 May 2020 17:10:33 +0200 Subject: [PATCH 26/73] Lock the active plugins map early To prevent a race condition when an exiting plugin wants to terminate the process when it think there are no plugins left just as another yabridge instance is connecting to the group socket. --- src/wine-host/bridges/group.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index ef5aa6ab..3d573b15 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -98,6 +98,8 @@ void GroupBridge::accept_requests() { group_socket_acceptor.async_accept( [&](const boost::system::error_code& error, boost::asio::local::stream_protocol::socket socket) { + std::lock_guard lock(active_plugins_mutex); + // Stop the whole process when the socket gets closed unexpectedly if (error.failed()) { logger.log("Error while listening for incoming connections:"); @@ -120,7 +122,6 @@ void GroupBridge::accept_requests() { // Collisions in the generated socket names should be very rare, but // it could in theory happen - std::lock_guard lock(active_plugins_mutex); assert(active_plugins.find(parameters) != active_plugins.end()); // CreateThread() doesn't support multiple arguments and requires From 8c22f37f29eb58db256976d43ed633f9083bddaa Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 21 May 2020 17:12:55 +0200 Subject: [PATCH 27/73] Actually host plugins in the group process --- src/wine-host/bridges/group.cpp | 42 +++++++++++++++++++++++++-------- src/wine-host/bridges/group.h | 2 +- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 3d573b15..5a84136c 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -71,20 +71,42 @@ GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) stderr_redirect(io_context, STDERR_FILENO), group_socket_endpoint(group_socket_path.string()), group_socket_acceptor(io_context, group_socket_endpoint) { - // TODO: After initializing, listen for connections and spawn plugins - // the exact same way as what happens in `individual-host.cpp` - // TODO: Allow this process to exit when the last plugin exits. Make sure - // that that doesn't cause any race conditions. - // Write this process's original STDOUT and STDERR streams to the logger async_log_pipe_lines(stdout_redirect.pipe, stdout_buffer, "[STDOUT] "); async_log_pipe_lines(stderr_redirect.pipe, stderr_buffer, "[STDERR] "); } -void GroupBridge::handle_host_plugin(const PluginParameters& parameters) { - // TODO: Start the plugin - // TODO: Allow this process to exit when the last plugin exits. Make sure - // that that doesn't cause any race conditions. +void GroupBridge::handle_host_plugin(const PluginParameters parameters) { + // At this point the `active_plugins` map will already contain a copy of + // `parameters` along with this thread's handle + // The initialization process for a plugin is identical to that in + // `../individual-host.cpp` + logger.log("Received request to host '" + parameters.plugin_path + + "' using socket '" + parameters.socket_path + "'"); + try { + Vst2Bridge bridge(parameters.plugin_path, parameters.socket_path); + logger.log("Finished initializing '" + parameters.plugin_path + "'"); + + // Blocks the main thread until the plugin shuts down + bridge.handle_dispatch(); + + logger.log("" + parameters.plugin_path + "' has exited"); + } catch (const std::runtime_error& error) { + logger.log("Error while initializing '" + parameters.plugin_path + + "':"); + logger.log(error.what()); + } + + // After the plugin has exited (either after `effClose()` or because it + // failed to initialize), we'll remove this thread's plugin from the active + // plugins. If no active plugins remain, then we'll terminate + std::lock_guard lock(active_plugins_mutex); + + active_plugins.erase(parameters); + if (active_plugins.size() == 0) { + logger.log("All plugins have exited, shutting down the group process"); + io_context.stop(); + } } void GroupBridge::handle_incoming_connections() { @@ -187,7 +209,7 @@ uint32_t WINAPI handle_host_plugin_proxy(void* param) { // need to use manual memory management. auto thread_params = static_cast(param); GroupBridge* instance = thread_params->first; - PluginParameters& parameters = thread_params->second; + PluginParameters parameters = thread_params->second; delete thread_params; instance->handle_host_plugin(parameters); diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index fe38f580..e68a7177 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -136,7 +136,7 @@ class GroupBridge { * then the process will never exit on its own. This should not happen * though. */ - void handle_host_plugin(const PluginParameters& parameters); + void handle_host_plugin(const PluginParameters parameters); /** * Listen for new requests to spawn plugins within this process and handle From 91b0ebf38b243de1978f7662b1ecbb0ad3b1b82f Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 21 May 2020 17:15:24 +0200 Subject: [PATCH 28/73] Remove mention of plugin groups flag Since there's no reason for this to be behind a flag. --- src/plugin/configuration.cpp | 1 - src/plugin/plugin-bridge.cpp | 1 - 2 files changed, 2 deletions(-) diff --git a/src/plugin/configuration.cpp b/src/plugin/configuration.cpp index a45944af..a7fd2984 100644 --- a/src/plugin/configuration.cpp +++ b/src/plugin/configuration.cpp @@ -67,6 +67,5 @@ Configuration Configuration::load_for(const fs::path& yabridge_path) { return Configuration(); } - // TODO: Disable plugin groups when not compiling with plugin group support return Configuration(config_file.value(), yabridge_path); } diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 25414b29..377c46eb 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -676,7 +676,6 @@ void PluginBridge::log_init_message() { // Include a list of enabled compile-tiem features, mostly to make debug // logs more useful - // TODO: Add a feature flag for the plugin group support init_msg << "Enabled features:" << std::endl; #ifdef USE_BITBRIDGE init_msg << "- bitbridge support" << std::endl; From fea256655d5057d2601d5a0a31d9b4bd1e58d85a Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 21 May 2020 18:09:32 +0200 Subject: [PATCH 29/73] Move process launching to a function Starting and connecting to plugin group host processes is not going to work in the intializer list. --- src/plugin/plugin-bridge.cpp | 82 +++++++++++++++++++++--------------- src/plugin/plugin-bridge.h | 15 +++++++ 2 files changed, 63 insertions(+), 34 deletions(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 377c46eb..3953f00e 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -19,11 +19,8 @@ #include #include #include -#include - -#ifdef USE_WINEDBG #include -#endif +#include // Generated inside of build directory #include @@ -74,35 +71,8 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) config(Configuration::load_for(get_this_file_location())), wine_version(get_wine_version()), wine_stdout(io_context), - wine_stderr(io_context), -#ifndef USE_WINEDBG - vst_host(vst_host_path, - // The Wine VST host needs to know which plugin to load - // and which Unix domain socket to connect to - vst_plugin_path, - socket_endpoint.path(), - bp::env = set_wineprefix(), - bp::std_out = wine_stdout, - bp::std_err = wine_stderr) -#else - // This is set up for KDE Plasma. Other desktop environments and window - // managers require some slight modifications to spawn a detached terminal - // emulator. - vst_host("/usr/bin/kstart5", - "konsole", - "--", - "-e", - "winedbg", - "--gdb", - vst_host_path.string() + ".so", - vst_plugin_path.filename(), - socket_endpoint.path(), - bp::env = set_wineprefix(), - // winedbg has no reliable way to escape spaces, so we'll start - // the process in the plugin's directory - bp::start_dir = vst_plugin_path.parent_path()) -#endif -{ + wine_stderr(io_context) { + launch_vst_host(); log_init_message(); // Print the Wine host's STDOUT and STDERR streams to the log file. This @@ -479,7 +449,12 @@ intptr_t PluginBridge::dispatch(AEffect* /*plugin*/, // loaded into the Wine process crashed during shutdown logger.log("The plugin crashed during shutdown, ignoring"); } - vst_host.terminate(); + + // Don't terminate group host processes. They will shut down + // automatically after all plugins have exited. + if (!config.group.has_value()) { + vst_host.terminate(); + } // The `stop()` method will cause the IO context to just drop all of // its work immediately and not throw any exceptions that would have @@ -634,6 +609,45 @@ void PluginBridge::async_log_pipe_lines(patched_async_pipe& pipe, }); } +void PluginBridge::launch_vst_host() { + // TODO: Connect to and launch group host processes + +#ifndef USE_WINEDBG + std::vector args{vst_host_path.string()}; +#else + // This is set up for KDE Plasma. Other desktop environments and window + // managers require some slight modifications to spawn a detached terminal + // emulator. + std::vector args{"/usr/bin/kstart5", + "konsole", + "--", + "-e", + "winedbg", + "--gdb", + vst_host_path.string() + ".so"}; +#endif + +#ifndef USE_WINEDBG + args.push_back(vst_plugin_path.string()); + const fs::path starting_dir = fs::current_path(); +#else + // winedbg has no reliable way to escape spaces, so we'll start the process + // in the plugin's directory + args.push_back(vst_plugin_path.filename().string()); + const fs::path starting_dir = vst_plugin_path.parent_path(); + + if (vst_plugin_path.filename().string().find(' ') != std::string::npos) { + logger.log("Warning: winedbg does not support paths containing spaces"); + } +#endif + + args.push_back(socket_endpoint.path()); + + vst_host = + bp::child(args, bp::env = set_wineprefix(), bp::std_out = wine_stdout, + bp::std_err = wine_stderr, bp::start_dir = starting_dir); +} + void PluginBridge::log_init_message() { std::stringstream init_msg; diff --git a/src/plugin/plugin-bridge.h b/src/plugin/plugin-bridge.h index d26aff36..e42787b1 100644 --- a/src/plugin/plugin-bridge.h +++ b/src/plugin/plugin-bridge.h @@ -109,6 +109,19 @@ class PluginBridge { boost::asio::streambuf& buffer, std::string prefix = ""); + /** + * Launch the Wine VST host to host the plugin. When using plugin groups, + * this will first try to connect to the plugin group's socket (determined + * based on group name, Wine prefix and architecture). If that fails, it + * will launch a new, detached group host process. This will likely outlive + * this plugin instance if multiple instances of yabridge using the same + * plugin group are in use. In the event that two yabridge instances are + * initialized at the same time and both instances spawn their own group + * host process, then the later one will simply terminate gracefully after + * it fails to listen on the socket. + */ + void launch_vst_host(); + /** * Format and log all relevant debug information during initialization. */ @@ -217,6 +230,8 @@ class PluginBridge { /** * The Wine process hosting the Windows VST plugin. + * + * @see launch_vst_host */ boost::process::child vst_host; From 27af0f8c1185fad026e8a12f4311dff7caecb699 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 14:00:45 +0200 Subject: [PATCH 30/73] Search for the group host when using plugin groups --- src/plugin/plugin-bridge.cpp | 6 +++--- src/plugin/plugin-bridge.h | 16 ++++++++-------- src/plugin/utils.cpp | 9 +++++---- src/plugin/utils.h | 5 ++++- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 3953f00e..fc9f64af 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -50,9 +50,10 @@ PluginBridge& get_bridge_instance(const AEffect& plugin) { } PluginBridge::PluginBridge(audioMasterCallback host_callback) - : vst_plugin_path(find_vst_plugin()), + : config(Configuration::load_for(get_this_file_location())), + vst_plugin_path(find_vst_plugin()), vst_plugin_arch(find_vst_architecture(vst_plugin_path)), - vst_host_path(find_vst_host(vst_plugin_arch)), + vst_host_path(find_vst_host(vst_plugin_arch, config.group.has_value())), // All the fields should be zero initialized because // `Vst2PluginInstance::vstAudioMasterCallback` from Bitwig's plugin // bridge will crash otherwise @@ -68,7 +69,6 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) host_callback_function(host_callback), logger(Logger::create_from_environment( create_logger_prefix(socket_endpoint.path()))), - config(Configuration::load_for(get_this_file_location())), wine_version(get_wine_version()), wine_stdout(io_context), wine_stderr(io_context) { diff --git a/src/plugin/plugin-bridge.h b/src/plugin/plugin-bridge.h index e42787b1..6b66ab99 100644 --- a/src/plugin/plugin-bridge.h +++ b/src/plugin/plugin-bridge.h @@ -74,6 +74,14 @@ class PluginBridge { float get_parameter(AEffect* plugin, int index); void set_parameter(AEffect* plugin, int index, float value); + /** + * The configuration for this instance of yabridge. Set based on the values + * from a `yabridge.toml`, if it exists. + * + * @see Configuration::load_for + */ + Configuration config; + /** * The path to the .dll being loaded in the Wine VST host. */ @@ -198,14 +206,6 @@ class PluginBridge { */ Logger logger; - /** - * The configuration for this instance of yabridge. Set based on the values - * from a `yabridge.toml`, if it exists. - * - * @see Configuration::load_for - */ - Configuration config; - /** * The version of Wine currently in use. Used in the debug output on plugin * startup. diff --git a/src/plugin/utils.cpp b/src/plugin/utils.cpp index 2fa192fa..59a1c393 100644 --- a/src/plugin/utils.cpp +++ b/src/plugin/utils.cpp @@ -111,11 +111,12 @@ PluginArchitecture find_vst_architecture(fs::path plugin_path) { throw std::runtime_error(error_msg.str()); } -fs::path find_vst_host(PluginArchitecture plugin_arch) { - // TODO: Take plugin group settings into account - auto host_name = yabridge_individual_host_name; +fs::path find_vst_host(PluginArchitecture plugin_arch, bool use_plugin_groups) { + auto host_name = use_plugin_groups ? yabridge_group_host_name + : yabridge_individual_host_name; if (plugin_arch == PluginArchitecture::vst_32) { - host_name = yabridge_individual_host_name_32bit; + host_name = use_plugin_groups ? yabridge_group_host_name_32bit + : yabridge_individual_host_name_32bit; } fs::path host_path = diff --git a/src/plugin/utils.h b/src/plugin/utils.h index ad481852..3946d008 100644 --- a/src/plugin/utils.h +++ b/src/plugin/utils.h @@ -80,11 +80,14 @@ PluginArchitecture find_vst_architecture(boost::filesystem::path); * * @param plugin_arch The architecture of the plugin, either 64-bit or 32-bit. * Used to determine which host application to use, if available. + * @param use_plugin_groups Whether the plugin is using plugin groups and we + * should be looking for the group host instead of the individual plugin host. * * @return The a path to the VST host, if found. * @throw std::runtime_error If the Wine VST host could not be found. */ -boost::filesystem::path find_vst_host(PluginArchitecture plugin_arch); +boost::filesystem::path find_vst_host(PluginArchitecture plugin_arch, + bool use_plugin_groups); /** * Find the VST plugin .dll file that corresponds to this copy of From dd843519ce437120eb3b487886a02e2558740fff Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 14:08:13 +0200 Subject: [PATCH 31/73] Rename PluginParameters to GroupReuqest --- src/common/serialization.cpp | 2 +- src/common/serialization.h | 8 +++---- src/plugin/plugin-bridge.cpp | 4 ++++ src/wine-host/bridges/group.cpp | 37 ++++++++++++++++++--------------- src/wine-host/bridges/group.h | 10 ++++----- 5 files changed, 34 insertions(+), 27 deletions(-) diff --git a/src/common/serialization.cpp b/src/common/serialization.cpp index c16b0142..4a5e14d4 100644 --- a/src/common/serialization.cpp +++ b/src/common/serialization.cpp @@ -109,6 +109,6 @@ AEffect& update_aeffect(AEffect& plugin, const AEffect& updated_plugin) { return plugin; } -bool PluginParameters::operator==(const PluginParameters& rhs) const { +bool GroupRequest::operator==(const GroupRequest& rhs) const { return plugin_path == rhs.plugin_path && socket_path == rhs.socket_path; } diff --git a/src/common/serialization.h b/src/common/serialization.h index b0810dc7..63b596c5 100644 --- a/src/common/serialization.h +++ b/src/common/serialization.h @@ -577,11 +577,11 @@ struct AudioBuffers { * group process. These are the exact same options that would have been passed * to `yabridge-host.exe` were the plugin to be hosted individually. */ -struct PluginParameters { +struct GroupRequest { std::string plugin_path; std::string socket_path; - bool operator==(const PluginParameters& rhs) const; + bool operator==(const GroupRequest& rhs) const; template void serialize(S& s) { @@ -591,8 +591,8 @@ struct PluginParameters { }; template <> -struct std::hash { - std::size_t operator()(PluginParameters const& params) const noexcept { +struct std::hash { + std::size_t operator()(GroupRequest const& params) const noexcept { std::hash hasher{}; return hasher(params.plugin_path) ^ (hasher(params.socket_path) << 1); diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index fc9f64af..be74e2f3 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -49,6 +49,10 @@ PluginBridge& get_bridge_instance(const AEffect& plugin) { return *static_cast(plugin.ptr3); } +// TODO: It would be nice to have a better way to encapsulate the small +// differences in behavior when using plugin groups, i.e. everywhere where +// we check for `config.group.has_value()` + PluginBridge::PluginBridge(audioMasterCallback host_callback) : config(Configuration::load_for(get_this_file_location())), vst_plugin_path(find_vst_plugin()), diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 5a84136c..fa7f651b 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -31,13 +31,17 @@ namespace fs = boost::filesystem; */ std::string create_logger_prefix(const fs::path& socket_path); -// CreateThread() is great and allows you to pass a single value to the -// function, so we'll use this to pass both `this` and the parameters to the -// below thread function so it can do its thing. -using handle_host_plugin_parameters = std::pair; - uint32_t WINAPI handle_host_plugin_proxy(void* param); +/** + * CreateThread() is great and allows you to pass a single value to the + * function, so we'll use this to pass both `this` and the parameters to the + * below thread function so it can do its thing. + * + * @relates handle_host_plugin_proxy + */ +using handle_host_plugin_parameters = std::pair; + StdIoCapture::StdIoCapture(boost::asio::io_context& io_context, int file_descriptor) : pipe(io_context), @@ -76,24 +80,23 @@ GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) async_log_pipe_lines(stderr_redirect.pipe, stderr_buffer, "[STDERR] "); } -void GroupBridge::handle_host_plugin(const PluginParameters parameters) { +void GroupBridge::handle_host_plugin(const GroupRequest request) { // At this point the `active_plugins` map will already contain a copy of // `parameters` along with this thread's handle // The initialization process for a plugin is identical to that in // `../individual-host.cpp` - logger.log("Received request to host '" + parameters.plugin_path + - "' using socket '" + parameters.socket_path + "'"); + logger.log("Received request to host '" + request.plugin_path + + "' using socket '" + request.socket_path + "'"); try { - Vst2Bridge bridge(parameters.plugin_path, parameters.socket_path); - logger.log("Finished initializing '" + parameters.plugin_path + "'"); + Vst2Bridge bridge(request.plugin_path, request.socket_path); + logger.log("Finished initializing '" + request.plugin_path + "'"); // Blocks the main thread until the plugin shuts down bridge.handle_dispatch(); - logger.log("" + parameters.plugin_path + "' has exited"); + logger.log("" + request.plugin_path + "' has exited"); } catch (const std::runtime_error& error) { - logger.log("Error while initializing '" + parameters.plugin_path + - "':"); + logger.log("Error while initializing '" + request.plugin_path + "':"); logger.log(error.what()); } @@ -102,7 +105,7 @@ void GroupBridge::handle_host_plugin(const PluginParameters parameters) { // plugins. If no active plugins remain, then we'll terminate std::lock_guard lock(active_plugins_mutex); - active_plugins.erase(parameters); + active_plugins.erase(request); if (active_plugins.size() == 0) { logger.log("All plugins have exited, shutting down the group process"); io_context.stop(); @@ -140,7 +143,7 @@ void GroupBridge::accept_requests() { // still active so we can terminate early if it is not, but in this // case the yabridge instance has to determine that this process is // still running. - const auto parameters = read_object(socket); + const auto parameters = read_object(socket); // Collisions in the generated socket names should be very rare, but // it could in theory happen @@ -149,7 +152,7 @@ void GroupBridge::accept_requests() { // CreateThread() doesn't support multiple arguments and requires // manualy memory management. handle_host_plugin_parameters* thread_params = - new std::pair(this, parameters); + new std::pair(this, parameters); active_plugins[parameters] = Win32Thread(handle_host_plugin_proxy, &thread_params); @@ -209,7 +212,7 @@ uint32_t WINAPI handle_host_plugin_proxy(void* param) { // need to use manual memory management. auto thread_params = static_cast(param); GroupBridge* instance = thread_params->first; - PluginParameters parameters = thread_params->second; + GroupRequest parameters = thread_params->second; delete thread_params; instance->handle_host_plugin(parameters); diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index e68a7177..33b79b99 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -128,15 +128,15 @@ class GroupBridge { * `active_plugins` map. If this causes the vector to become empty, we will * terminate this process. * - * @param parameters Information about the plugin to launch, i.e. the path - * to the plugin and the path of the socket endpoint that will be used for + * @param request Information about the plugin to launch, i.e. the path to + * the plugin and the path of the socket endpoint that will be used for * communication. * * @note In the case that the process starts but no plugin gets initiated, * then the process will never exit on its own. This should not happen * though. */ - void handle_host_plugin(const PluginParameters parameters); + void handle_host_plugin(const GroupRequest request); /** * Listen for new requests to spawn plugins within this process and handle @@ -148,7 +148,7 @@ class GroupBridge { /** * Listen on the group socket for incoming requests to host a new plugin * within this group process. This will asynchronously listen on the socket, - * and for any connection made it will retrieve a `PluginParameters` object + * and for any connection made it will retrieve a `GroupRequest` object * containing information about the plugin to host and then spawn a new * thread to start hosting that plugin. * @@ -208,7 +208,7 @@ class GroupBridge { * the amount of plugins currently running with their associated thread * handles. */ - std::unordered_map active_plugins; + std::unordered_map active_plugins; /** * A mutex to prevent two threads from simultaneously accessing the plugins * map, and also to prevent `handle_host_plugin()` from terminating the From 9b50bac1796f8894afbda356d1ab11ecb07ef7c2 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 14:28:25 +0200 Subject: [PATCH 32/73] Reply with the group process's PID after a request --- src/common/serialization.h | 15 +++++++++++++++ src/wine-host/bridges/group.cpp | 14 ++++++-------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/common/serialization.h b/src/common/serialization.h index 63b596c5..e298efdf 100644 --- a/src/common/serialization.h +++ b/src/common/serialization.h @@ -598,3 +598,18 @@ struct std::hash { return hasher(params.plugin_path) ^ (hasher(params.socket_path) << 1); } }; + +/** + * The response sent back after the group host process receives a `GroupRequest` + * object. This only holds the group process's PID because we need to know if + * the group process crashes while it is initializing the plugin to prevent us + * from waiting indefinitely for the socket to be connected to. + */ +struct GroupResponse { + int pid; + + template + void serialize(S& s) { + s.value4b(pid); + } +}; diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index fa7f651b..9a6966b1 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include "../../common/communication.h" @@ -135,15 +136,12 @@ void GroupBridge::accept_requests() { // Read the parameters, and then host the plugin in this process // just like if we would be hosting the plugin individually through - // `yabridge-hsot.exe`. Since we're using sockets there's no reason - // to send an acknowledgement back. One potential issue here is that - // it will be hard to tell whether a plugin has crashed before - // initialization, and that the sockets will never be accepted. For - // individually hosted plugins we poll whether the Wine process is - // still active so we can terminate early if it is not, but in this - // case the yabridge instance has to determine that this process is - // still running. + // `yabridge-hsot.exe`. We will reply with this process's PID so the + // yabridge plugin will be able to tell if the plugin has caused + // this process to crash during its initialization to prevent + // waiting indefinitely on the sockets to be connected to. const auto parameters = read_object(socket); + write_object(socket, GroupResponse{boost::this_process::get_id()}); // Collisions in the generated socket names should be very rare, but // it could in theory happen From 615d23e525936aa8e369150df0105d4060c2c580 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 14:34:51 +0200 Subject: [PATCH 33/73] Clean up new host process launching --- src/plugin/plugin-bridge.cpp | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index be74e2f3..4b1c2d8f 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -617,38 +617,38 @@ void PluginBridge::launch_vst_host() { // TODO: Connect to and launch group host processes #ifndef USE_WINEDBG - std::vector args{vst_host_path.string()}; + std::vector host_command{vst_host_path.string()}; #else // This is set up for KDE Plasma. Other desktop environments and window // managers require some slight modifications to spawn a detached terminal // emulator. - std::vector args{"/usr/bin/kstart5", - "konsole", - "--", - "-e", - "winedbg", - "--gdb", - vst_host_path.string() + ".so"}; + std::vector host_command{"/usr/bin/kstart5", + "konsole", + "--", + "-e", + "winedbg", + "--gdb", + vst_host_path.string() + ".so"}; #endif #ifndef USE_WINEDBG - args.push_back(vst_plugin_path.string()); + const fs::path plugin_path = vst_plugin_path; const fs::path starting_dir = fs::current_path(); #else // winedbg has no reliable way to escape spaces, so we'll start the process // in the plugin's directory - args.push_back(vst_plugin_path.filename().string()); + const fs::path plugin_path = vst_plugin_path.filename(); const fs::path starting_dir = vst_plugin_path.parent_path(); - if (vst_plugin_path.filename().string().find(' ') != std::string::npos) { + if (plugin_path.string().find(' ') != std::string::npos) { logger.log("Warning: winedbg does not support paths containing spaces"); } #endif - - args.push_back(socket_endpoint.path()); + const fs::path socket_path = socket_endpoint.path(); vst_host = - bp::child(args, bp::env = set_wineprefix(), bp::std_out = wine_stdout, + bp::child(host_command, plugin_path, socket_path, + bp::env = set_wineprefix(), bp::std_out = wine_stdout, bp::std_err = wine_stderr, bp::start_dir = starting_dir); } From 5b4b62d7c406c12ac0fbb4e15b0247984d33fde5 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 14:52:41 +0200 Subject: [PATCH 34/73] Use pid_t instead of ints for PIDs --- src/common/serialization.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/serialization.h b/src/common/serialization.h index e298efdf..c82c0def 100644 --- a/src/common/serialization.h +++ b/src/common/serialization.h @@ -606,7 +606,7 @@ struct std::hash { * from waiting indefinitely for the socket to be connected to. */ struct GroupResponse { - int pid; + pid_t pid; template void serialize(S& s) { From 9fb7f1fc0360bb7b4fdf006685dacc170c72e71e Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 18:19:37 +0200 Subject: [PATCH 35/73] Add handling for stale and active group sockets --- src/wine-host/bridges/group.cpp | 56 ++++++++++++++++++++++++++++++++- src/wine-host/bridges/group.h | 2 +- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 9a6966b1..14fa9dcc 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -27,6 +27,26 @@ // path operation will thrown an encoding related error namespace fs = boost::filesystem; +/** + * Listen on the specified endpoint if no process is already listening there, + * otherwise throw. This is needed to handle these three situations: + * + * 1. The endpoint does not already exist, and we can simply create an endpoint. + * 2. The endpoint already exists but it is stale and no process is currently + * listening. In this case we can remove the file and start listening. + * 3. The endpoint already exists and another process is currently listening on + * it. In this situation we will throw immediately and we'll terminate this + * process. + * + * If anyone knows a better way to handle this, please let me know! + * + * @throw std::runtime_error If another process is already listening on the + * endpoint. + */ +boost::asio::local::stream_protocol::acceptor create_acceptor_if_inactive( + boost::asio::io_context& io_context, + boost::asio::local::stream_protocol::endpoint& endpoint); + /** * Create a logger prefix containing the group name based on the socket path. */ @@ -75,7 +95,8 @@ GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) stdout_redirect(io_context, STDOUT_FILENO), stderr_redirect(io_context, STDERR_FILENO), group_socket_endpoint(group_socket_path.string()), - group_socket_acceptor(io_context, group_socket_endpoint) { + group_socket_acceptor( + create_acceptor_if_inactive(io_context, group_socket_endpoint)) { // Write this process's original STDOUT and STDERR streams to the logger async_log_pipe_lines(stdout_redirect.pipe, stdout_buffer, "[STDOUT] "); async_log_pipe_lines(stderr_redirect.pipe, stderr_buffer, "[STDERR] "); @@ -179,6 +200,39 @@ void GroupBridge::async_log_pipe_lines( }); } +boost::asio::local::stream_protocol::acceptor create_acceptor_if_inactive( + boost::asio::io_context& io_context, + boost::asio::local::stream_protocol::endpoint& endpoint) { + // First try to listen on the endpoint normally + try { + return boost::asio::local::stream_protocol::acceptor(io_context, + endpoint); + } catch (const boost::system::system_error& error) { + // If this failed, then either there is a stale socket file or another + // process is already is already listening. In the last case we will + // simply throw so the other process can handle the request. + std::ifstream open_sockets("/proc/net/unix"); + std::string endpoint_path = endpoint.path(); + for (std::string line; std::getline(open_sockets, line);) { + if (line.size() < endpoint_path.size()) { + continue; + } + + std::string file = line.substr(line.size() - endpoint_path.size()); + if (file == endpoint_path) { + // Another process is already listening, so we don't have to do + // anything + throw error; + } + } + + // At this point we can remove the stale socket and start listening + fs::remove(endpoint_path); + return boost::asio::local::stream_protocol::acceptor(io_context, + endpoint); + } +} + std::string create_logger_prefix(const fs::path& socket_path) { // The group socket filename will be in the format // '/tmp/yabridge-group---.sock', diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index 33b79b99..93039bef 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -104,7 +104,7 @@ class GroupBridge { * * @param gruop_socket_path The path to the group socket endpoint. This path * should be in the form of - * `/tmp/yabridge-group---` + * `/tmp/yabridge-group---.sock` * where `` is a numerical hash as explained in the * `create_logger_prefix()` function in `./group.cpp`. * From 903d977d834b2c4d93963cb9fddbda6fce057360 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 18:21:31 +0200 Subject: [PATCH 36/73] Add a function for generating group host endpoints --- src/plugin/utils.cpp | 22 +++++++++++++++++++++- src/plugin/utils.h | 28 +++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/plugin/utils.cpp b/src/plugin/utils.cpp index 59a1c393..af28b3ff 100644 --- a/src/plugin/utils.cpp +++ b/src/plugin/utils.cpp @@ -164,7 +164,27 @@ fs::path find_vst_plugin() { "VST plugin .dll file."); } -fs::path generate_endpoint_name() { +boost::filesystem::path generate_group_endpoint( + std::string group_name, + boost::filesystem::path wine_prefix, + PluginArchitecture architecture) { + std::ostringstream socket_name; + socket_name << "yabridge-group-" << group_name << "-" + << std::hash{}(wine_prefix.string()) << "-"; + switch (architecture) { + case PluginArchitecture::vst_32: + socket_name << "x32"; + break; + case PluginArchitecture::vst_64: + socket_name << "x64"; + break; + } + socket_name << ".sock"; + + return fs::temp_directory_path() / socket_name.str(); +} + +fs::path generate_plugin_endpoint() { const auto plugin_name = find_vst_plugin().filename().replace_extension("").string(); diff --git a/src/plugin/utils.h b/src/plugin/utils.h index 3946d008..2cbad0a0 100644 --- a/src/plugin/utils.h +++ b/src/plugin/utils.h @@ -112,6 +112,32 @@ boost::filesystem::path find_vst_plugin(); */ std::optional find_wineprefix(); +/** + * Generate the group socket endpoint name used based on the name of the group, + * the Wine prefix in use and the plugin architecture. The resulting format is + * `/tmp/yabridge-group---.sock`. In + * this socket name the `wine_prefix_id` is a numerical hash based on the Wine + * prefix in use. This way the same group name can be used for multiple Wine + * prefixes and for both 32 and 64 bit plugins without clashes. + * + * @param group_name The name of the plugin group. + * @param wine_prefix The name of the Wine prefix in use. This should be + * obtained by first calling `set_wineprefix()` to allow the user to override + * this, and then falling back to `$HOME/.wine` if the environment variable is + * unset. Otherwise plugins run from outwide of a Wine prefix will not be + * groupable with those run from within `~/.wine` even though they both run + * under the same prefix. + * @param architecture The architecture the plugin is using, since 64-bit + * processes can't host 32-bit plugins and the other way around. + * + * @return A socket endpoint path that corresponds to the format described + * above. + */ +boost::filesystem::path generate_group_endpoint( + std::string group_name, + boost::filesystem::path wine_prefix, + PluginArchitecture architecture); + /** * Generate a unique name for the Unix domain socket endpoint based on the VST * plugin's name. This will also generate the parent directory if it does not @@ -120,7 +146,7 @@ std::optional find_wineprefix(); * @return A path to a not yet existing Unix domain socket endpoint. * @throw std::runtime_error If no matching .dll file could be found. */ -boost::filesystem::path generate_endpoint_name(); +boost::filesystem::path generate_plugin_endpoint(); /** * Return a path to this `.so` file. This can be used to find out from where From c5c1c334d9b53d680b184f9b478b88755dc9aa73 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 18:50:10 +0200 Subject: [PATCH 37/73] Add group host support to the plugin The difference between individual hosting and group hosting will have to be encapsulated in a class to keep the rest of the plugin bridge clean. --- src/plugin/plugin-bridge.cpp | 138 ++++++++++++++++++++++++++++------- src/plugin/plugin-bridge.h | 21 ++++++ 2 files changed, 131 insertions(+), 28 deletions(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 4b1c2d8f..bc8f8a43 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -63,7 +63,7 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) // bridge will crash otherwise plugin(), io_context(), - socket_endpoint(generate_endpoint_name().string()), + socket_endpoint(generate_plugin_endpoint().string()), socket_acceptor(io_context, socket_endpoint), host_vst_dispatch(io_context), host_vst_dispatch_midi_events(io_context), @@ -95,20 +95,29 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) std::thread([&]() { using namespace std::literals::chrono_literals; - // TODO: For plugin groups, we should be polling whether someone is - // still listening on the group socket (and ideally we should be - // able to tell it's the same process). If we can't figure out a - // better way, then we could just return the PID when sending the - // host request to the group process and check whether that - // process is still running. while (true) { if (finished_accepting_sockets) { return; } - if (!vst_host.running()) { - throw std::runtime_error( - "The Wine process failed to start. Check the output above " - "for more information."); + + // When using regular individually hosted plugins we can simply + // check whether the process is still running, but Boost.Process + // does not allow you to do the same thing for a process that's not + // a child if this process. When using plugin groups we'll have to + // manually check whether the PID returned by the group host process + // is still active. + if (config.group.has_value()) { + if (kill(vst_host_pid, 0) != 0) { + throw std::runtime_error( + "The group host process has exited unexpectedly. Check " + "the output above for more information."); + } + } else { + if (!vst_host.running()) { + throw std::runtime_error( + "The Wine process failed to start. Check the output " + "above for more information."); + } } std::this_thread::sleep_for(1s); @@ -467,6 +476,7 @@ intptr_t PluginBridge::dispatch(AEffect* /*plugin*/, // These threads should now be finished because we've forcefully // terminated the Wine process, interupting their socket operations + group_host_connect_handler.join(); host_callback_handler.join(); wine_io_handler.join(); @@ -614,21 +624,21 @@ void PluginBridge::async_log_pipe_lines(patched_async_pipe& pipe, } void PluginBridge::launch_vst_host() { - // TODO: Connect to and launch group host processes + const bp::environment host_env = set_wineprefix(); #ifndef USE_WINEDBG - std::vector host_command{vst_host_path.string()}; + const std::vector host_command{vst_host_path.string()}; #else // This is set up for KDE Plasma. Other desktop environments and window // managers require some slight modifications to spawn a detached terminal // emulator. - std::vector host_command{"/usr/bin/kstart5", - "konsole", - "--", - "-e", - "winedbg", - "--gdb", - vst_host_path.string() + ".so"}; + const std::vector host_command{"/usr/bin/kstart5", + "konsole", + "--", + "-e", + "winedbg", + "--gdb", + vst_host_path.string() + ".so"}; #endif #ifndef USE_WINEDBG @@ -646,10 +656,86 @@ void PluginBridge::launch_vst_host() { #endif const fs::path socket_path = socket_endpoint.path(); - vst_host = - bp::child(host_command, plugin_path, socket_path, - bp::env = set_wineprefix(), bp::std_out = wine_stdout, - bp::std_err = wine_stderr, bp::start_dir = starting_dir); + if (!config.group.has_value()) { + vst_host = + bp::child(host_command, plugin_path, socket_path, + bp::env = host_env, bp::std_out = wine_stdout, + bp::std_err = wine_stderr, bp::start_dir = starting_dir); + return; + } + + // When using plugin groups, we'll first try to connect to an existing group + // host process and ask it to host our plugin. If no such process exists, + // then we'll start a new process. In the event that two yabridge instances + // simultaneously try to start a new group process for the same group, then + // the last process to connect to the socket will terminate gracefully and + // the first process will handle the connections for both yabridge + // instances. + fs::path wine_prefix = host_env.at("WINEPREFIX").to_string(); + if (host_env.at("WINEPREFIX").empty()) { + // Fall back to `~/.wine` if this has not been set or detected. This + // would happen if the plugin's .dll file is not inside of a Wine + // prefix. If this happens, then the Wine instance will be launched in + // the default Wine prefix, so we should reflect that here. + wine_prefix = fs::path(host_env.at("HOME").to_string()) / ".wine"; + } + + const fs::path group_socket_path = generate_group_endpoint( + config.group.value(), wine_prefix, vst_plugin_arch); + + try { + // Request the existing group host process to host our plugin, and store + // the PID of that process so we'll know if it has crashed + boost::asio::local::stream_protocol::socket group_socket(io_context); + group_socket.connect(group_socket_path.string()); + + write_object(group_socket, + GroupRequest{plugin_path.string(), socket_path.string()}); + const auto response = read_object(group_socket); + + vst_host_pid = response.pid; + } catch (const boost::system::system_error&) { + // In case we could not connect to the socket, then we'll start a + // new group host process. This process is detached immediately + // because it should run independently of this yabridge instance as + // it will likely outlive it. + vst_host = + bp::child(host_command, group_socket_path, bp::env = host_env, + bp::std_out = wine_stdout, bp::std_err = wine_stderr, + bp::start_dir = starting_dir); + vst_host_pid = vst_host.id(); + vst_host.detach(); + + // We now want to connect to the socket the in the exact same way as + // above. The only problem is that it may take some time for the + // process to start depending on Wine's current state. We'll defer + // this to a thread so we can finish the rest of the startup in the + // meantime. + group_host_connect_handler = std::thread([&, group_socket_path, + plugin_path, socket_path]() { + using namespace std::literals::chrono_literals; + + // TODO: Replace this polling with inotify when encapsulating + // the different host launch behaviors + while (vst_host.running()) { + try { + // This is the exact same connection sequence as above + boost::asio::local::stream_protocol::socket group_socket( + io_context); + group_socket.connect(group_socket_path.string()); + + write_object(group_socket, + GroupRequest{plugin_path.string(), + socket_path.string()}); + read_object(group_socket); + + return; + } catch (const boost::system::system_error&) { + std::this_thread::sleep_for(20ms); + } + } + }); + } } void PluginBridge::log_init_message() { @@ -671,10 +757,6 @@ void PluginBridge::log_init_message() { // settings in use. Printing the matched glob pattern could also be useful // but it'll be very noisy and it's likely going to be clear from the shown // values anyways. - // TODO: The group socket should take into account the: - // - wine prefix - // - group name - // - architecture init_msg << "config path: '" << config.matched_file.value_or("").string() << "'" << std::endl; diff --git a/src/plugin/plugin-bridge.h b/src/plugin/plugin-bridge.h index 6b66ab99..a4709b35 100644 --- a/src/plugin/plugin-bridge.h +++ b/src/plugin/plugin-bridge.h @@ -234,6 +234,27 @@ class PluginBridge { * @see launch_vst_host */ boost::process::child vst_host; + /** + * The PID of the vst host process. Needed for checking whether the group + * host is still active if we are connecting to an already running group + * host instance. + * + * TODO: Remove this after encapsulating the minor differences in individual + * and group host handling + */ + pid_t vst_host_pid; + /** + * A thread that waits for the group host to have started and then ask it to + * host our plugin. This is used to defer the request since it may take a + * little while until the group host process is up and running. This way we + * don't have to delay the rest of the initialization process. + * + * TODO: Remove this after encapsulating the minor differences in individual + * and group host handling + * TODO: Replace this with inotify to prevent delays and to reduce wasting + * resources + */ + std::thread group_host_connect_handler; /** * A scratch buffer for sending and receiving data during `process` and From ad6199949d486f862ee5614a7b200de5b5d9ebe5 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 19:02:27 +0200 Subject: [PATCH 38/73] Fix startup errors not being logged --- src/plugin/plugin-bridge.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index bc8f8a43..4d976ef4 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -108,15 +108,17 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) // is still active. if (config.group.has_value()) { if (kill(vst_host_pid, 0) != 0) { - throw std::runtime_error( + logger.log( "The group host process has exited unexpectedly. Check " "the output above for more information."); + std::terminate(); } } else { if (!vst_host.running()) { - throw std::runtime_error( + logger.log( "The Wine process failed to start. Check the output " "above for more information."); + std::terminate(); } } From b4b471523f44e60433fb56bdf675e649409c9a3d Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 19:52:45 +0200 Subject: [PATCH 39/73] Fix assertion in group host connection handler --- src/wine-host/bridges/group.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 14fa9dcc..5510d36a 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -137,7 +137,8 @@ void GroupBridge::handle_host_plugin(const GroupRequest request) { void GroupBridge::handle_incoming_connections() { accept_requests(); - logger.log("Now accepting incoming connections"); + logger.log( + "Group host is up and running, now accepting incoming connections"); io_context.run(); } @@ -166,14 +167,14 @@ void GroupBridge::accept_requests() { // Collisions in the generated socket names should be very rare, but // it could in theory happen - assert(active_plugins.find(parameters) != active_plugins.end()); + assert(active_plugins.find(parameters) == active_plugins.end()); // CreateThread() doesn't support multiple arguments and requires // manualy memory management. handle_host_plugin_parameters* thread_params = new std::pair(this, parameters); active_plugins[parameters] = - Win32Thread(handle_host_plugin_proxy, &thread_params); + Win32Thread(handle_host_plugin_proxy, thread_params); accept_requests(); }); From bea924c0e1c923d8274f327c64c4320d0f084f18 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 22:42:24 +0200 Subject: [PATCH 40/73] Make plugin initialization thread safe --- src/wine-host/bridges/vst2.cpp | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 22a6c861..1e309a31 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -31,6 +31,11 @@ using VstEntryPoint = AEffect*(VST_CALL_CONV*)(audioMasterCallback); * from an `AEffect` when it performs a host callback during its initialization. */ Vst2Bridge* current_bridge_instance = nullptr; +/** + * Needed for the rare event that two plugins are getting initialized at the + * same time. + */ +std::mutex current_bridge_instance_mutex; intptr_t VST_CALL_CONV host_callback_proxy(AEffect*, int, int, intptr_t, void*, float); @@ -52,8 +57,6 @@ Vst2Bridge& get_bridge_instance(const AEffect* plugin) { // This is needed during the initialization of the plugin since we can only // add our own pointer after it's done initializing if (current_bridge_instance != nullptr) { - // This should only be used during initialization - assert(plugin == nullptr || plugin->ptr1 == nullptr); return *current_bridge_instance; } @@ -106,16 +109,19 @@ Vst2Bridge::Vst2Bridge(std::string plugin_dll_path, // We'll try to do the same `get_bridge_isntance` trick as in // `plugin/plugin.cpp`, but since the plugin will probably call the host // callback while it's initializing we sadly have to use a global here. - current_bridge_instance = this; - plugin = vst_entry_point(host_callback_proxy); - if (plugin == nullptr) { - throw std::runtime_error("VST plugin at '" + plugin_dll_path + - "' failed to initialize."); - } + { + std::lock_guard lock(current_bridge_instance_mutex); + current_bridge_instance = this; + plugin = vst_entry_point(host_callback_proxy); + if (plugin == nullptr) { + throw std::runtime_error("VST plugin at '" + plugin_dll_path + + "' failed to initialize."); + } - // We only needed this little hack during initialization - current_bridge_instance = nullptr; - plugin->ptr1 = this; + // We only needed this little hack during initialization + current_bridge_instance = nullptr; + plugin->ptr1 = this; + } // Send the plugin's information to the Linux VST plugin. This is done over // the `dispatch()` socket since this has to be done only once during From 6da1909d4b20be64b4999d3c5f698dd5eedc8456 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 22:50:29 +0200 Subject: [PATCH 41/73] Fix interleaved group host initialization In case two yabridge instances start at the same time and both try to launch a group host for the same group and the second plugin connects to the first group host, then we should of course be using that process'es PID to check if the group host is still running. --- src/plugin/plugin-bridge.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 4d976ef4..0bafc39f 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -720,6 +720,8 @@ void PluginBridge::launch_vst_host() { // TODO: Replace this polling with inotify when encapsulating // the different host launch behaviors while (vst_host.running()) { + std::this_thread::sleep_for(20ms); + try { // This is the exact same connection sequence as above boost::asio::local::stream_protocol::socket group_socket( @@ -729,11 +731,14 @@ void PluginBridge::launch_vst_host() { write_object(group_socket, GroupRequest{plugin_path.string(), socket_path.string()}); - read_object(group_socket); + const auto response = + read_object(group_socket); + // If two group processes started at the same time, than the + // first one will be the one to respond to the host request + vst_host_pid = response.pid; return; } catch (const boost::system::system_error&) { - std::this_thread::sleep_for(20ms); } } }); From a246ddf3443aa6a10c87879cba3ac00ef0be09df Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 23:08:25 +0200 Subject: [PATCH 42/73] Manually close sockets when not killing process Killing the socket would otherwise cause the sockets to be closed, and both sides to shut down. --- src/plugin/plugin-bridge.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 0bafc39f..f013eb0d 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -469,6 +469,10 @@ intptr_t PluginBridge::dispatch(AEffect* /*plugin*/, // automatically after all plugins have exited. if (!config.group.has_value()) { vst_host.terminate(); + } else { + // Manually the dispatch socket will cause the host process to + // terminate + host_vst_dispatch.close(); } // The `stop()` method will cause the IO context to just drop all of From 3a9d902c721ffe77e2f77f965bd6850fc4d6080b Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 23:20:06 +0200 Subject: [PATCH 43/73] Allow all threads to return when sockets close This was not a problem with individually hosted plugins because the entire process got terminated at once, but here we all threads to shut down gracefully when a plugin's sockets get closed. I wish this wouldn't need all these try-catches, but we're not writing Haskell here. The only issue remaining is that for some reason only the first instance's editor works, at least for Serum. This might be because of the message loop. --- src/wine-host/bridges/vst2.cpp | 239 ++++++++++++++++++--------------- src/wine-host/bridges/vst2.h | 6 +- 2 files changed, 134 insertions(+), 111 deletions(-) diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 1e309a31..c2cde442 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -186,130 +186,150 @@ void Vst2Bridge::handle_dispatch() { } } -[[noreturn]] void Vst2Bridge::handle_dispatch_midi_events() { - while (true) { - receive_event( - host_vst_dispatch_midi_events, std::nullopt, [&](Event& event) { - if (BOOST_LIKELY(event.opcode == effProcessEvents)) { - // For 99% of the plugins we can just call - // `effProcessReplacing()` and be done with it, but a select - // few plugins (I could only find Kontakt that does this) - // don't actually make copies of the events they receive and - // only store pointers, meaning that they have to live at - // least until the next audio buffer gets processed. We're - // not using `passhtourhg_events()` here directly because we - // need to store a copy of the `DynamicVstEvents` struct - // before passing the generated `VstEvents` object to the - // plugin. - std::lock_guard lock(next_buffer_midi_events_mutex); +void Vst2Bridge::handle_dispatch_midi_events() { + try { + while (true) { + receive_event( + host_vst_dispatch_midi_events, std::nullopt, [&](Event& event) { + if (BOOST_LIKELY(event.opcode == effProcessEvents)) { + // For 99% of the plugins we can just call + // `effProcessReplacing()` and be done with it, but a + // select few plugins (I could only find Kontakt that + // does this) don't actually make copies of the events + // they receive and only store pointers, meaning that + // they have to live at least until the next audio + // buffer gets processed. We're not using + // `passhtourhg_events()` here directly because we need + // to store a copy of the `DynamicVstEvents` struct + // before passing the generated `VstEvents` object to + // the plugin. + std::lock_guard lock(next_buffer_midi_events_mutex); - next_audio_buffer_midi_events.push_back( - std::get(event.payload)); - DynamicVstEvents& events = - next_audio_buffer_midi_events.back(); + next_audio_buffer_midi_events.push_back( + std::get(event.payload)); + DynamicVstEvents& events = + next_audio_buffer_midi_events.back(); - // Exact same handling as in `passthrough_event`, apart from - // making a copy of the events first - const intptr_t return_value = plugin->dispatcher( - plugin, event.opcode, event.index, event.value, - &events.as_c_events(), event.option); + // Exact same handling as in `passthrough_event`, apart + // from making a copy of the events first + const intptr_t return_value = plugin->dispatcher( + plugin, event.opcode, event.index, event.value, + &events.as_c_events(), event.option); - EventResult response{return_value, nullptr, std::nullopt}; + EventResult response{return_value, nullptr, + std::nullopt}; - return response; - } else { - using namespace std::placeholders; + return response; + } else { + using namespace std::placeholders; - std::cerr << "[Warning] Received non-MIDI " - "event on MIDI processing thread" - << std::endl; + std::cerr << "[Warning] Received non-MIDI " + "event on MIDI processing thread" + << std::endl; - // Maybe this should just be a hard error instead, since it - // should never happen - return passthrough_event( - plugin, std::bind(&Vst2Bridge::dispatch_wrapper, this, - _1, _2, _3, _4, _5, _6))(event); - } - }); - } -} - -[[noreturn]] void Vst2Bridge::handle_parameters() { - while (true) { - // Both `getParameter` and `setParameter` functions are passed - // through on this socket since they have a lot of overlap. The - // presence of the `value` field tells us which one we're dealing - // with. - auto request = read_object(host_vst_parameters); - if (request.value.has_value()) { - // `setParameter` - plugin->setParameter(plugin, request.index, request.value.value()); - - ParameterResult response{std::nullopt}; - write_object(host_vst_parameters, response); - } else { - // `getParameter` - float value = plugin->getParameter(plugin, request.index); - - ParameterResult response{value}; - write_object(host_vst_parameters, response); + // Maybe this should just be a hard error instead, since + // it should never happen + return passthrough_event( + plugin, + std::bind(&Vst2Bridge::dispatch_wrapper, this, _1, + _2, _3, _4, _5, _6))(event); + } + }); } + } catch (const boost::system::system_error&) { + // The plugin has cut off communications, so we can shut down this host + // application } } -[[noreturn]] void Vst2Bridge::handle_process_replacing() { +void Vst2Bridge::handle_parameters() { + try { + while (true) { + // Both `getParameter` and `setParameter` functions are passed + // through on this socket since they have a lot of overlap. The + // presence of the `value` field tells us which one we're dealing + // with. + auto request = read_object(host_vst_parameters); + if (request.value.has_value()) { + // `setParameter` + plugin->setParameter(plugin, request.index, + request.value.value()); + + ParameterResult response{std::nullopt}; + write_object(host_vst_parameters, response); + } else { + // `getParameter` + float value = plugin->getParameter(plugin, request.index); + + ParameterResult response{value}; + write_object(host_vst_parameters, response); + } + } + } catch (const boost::system::system_error&) { + // The plugin has cut off communications, so we can shut down this host + // application + } +} + +void Vst2Bridge::handle_process_replacing() { std::vector> output_buffers(plugin->numOutputs); - while (true) { - auto request = read_object(host_vst_process_replacing, - process_buffer); + try { + while (true) { + auto request = read_object(host_vst_process_replacing, + process_buffer); - // The process functions expect a `float**` for their inputs and - // their outputs - std::vector inputs; - for (auto& buffer : request.buffers) { - inputs.push_back(buffer.data()); - } - - // We reuse the buffers to avoid some unnecessary heap allocations, - // so we need to make sure the buffers are large enough since - // plugins can change their output configuration - std::vector outputs; - output_buffers.resize(plugin->numOutputs); - for (auto& buffer : output_buffers) { - buffer.resize(request.sample_frames); - outputs.push_back(buffer.data()); - } - - // Let the plugin process the MIDI events that were received since the - // last buffer, and then clean up those events. This approach should not - // be needed but Kontakt only stores pointers to rather than copies of - // the events. - { - std::lock_guard lock(next_buffer_midi_events_mutex); - - // Any plugin made in the last fifteen years or so should support - // `processReplacing`. In the off chance it does not we can just - // emulate this behavior ourselves. - if (plugin->processReplacing != nullptr) { - plugin->processReplacing(plugin, inputs.data(), outputs.data(), - request.sample_frames); - } else { - // If we zero out this buffer then the behavior is the same as - // `processReplacing`` - for (std::vector& buffer : output_buffers) { - std::fill(buffer.begin(), buffer.end(), 0.0); - } - - plugin->process(plugin, inputs.data(), outputs.data(), - request.sample_frames); + // The process functions expect a `float**` for their inputs and + // their outputs + std::vector inputs; + for (auto& buffer : request.buffers) { + inputs.push_back(buffer.data()); } - next_audio_buffer_midi_events.clear(); - } + // We reuse the buffers to avoid some unnecessary heap allocations, + // so we need to make sure the buffers are large enough since + // plugins can change their output configuration + std::vector outputs; + output_buffers.resize(plugin->numOutputs); + for (auto& buffer : output_buffers) { + buffer.resize(request.sample_frames); + outputs.push_back(buffer.data()); + } - AudioBuffers response{output_buffers, request.sample_frames}; - write_object(host_vst_process_replacing, response, process_buffer); + // Let the plugin process the MIDI events that were received since + // the last buffer, and then clean up those events. This approach + // should not be needed but Kontakt only stores pointers to rather + // than copies of the events. + { + std::lock_guard lock(next_buffer_midi_events_mutex); + + // Any plugin made in the last fifteen years or so should + // support `processReplacing`. In the off chance it does not we + // can just emulate this behavior ourselves. + if (plugin->processReplacing != nullptr) { + plugin->processReplacing(plugin, inputs.data(), + outputs.data(), + request.sample_frames); + } else { + // If we zero out this buffer then the behavior is the same + // as `processReplacing`` + for (std::vector& buffer : output_buffers) { + std::fill(buffer.begin(), buffer.end(), 0.0); + } + + plugin->process(plugin, inputs.data(), outputs.data(), + request.sample_frames); + } + + next_audio_buffer_midi_events.clear(); + } + + AudioBuffers response{output_buffers, request.sample_frames}; + write_object(host_vst_process_replacing, response, process_buffer); + } + } catch (const boost::system::system_error&) { + // The plugin has cut off communications, so we can shut down this host + // application } } @@ -477,12 +497,15 @@ intptr_t VST_CALL_CONV host_callback_proxy(AEffect* effect, uint32_t WINAPI handle_dispatch_midi_events_proxy(void* instance) { static_cast(instance)->handle_dispatch_midi_events(); + return 0; } uint32_t WINAPI handle_parameters_proxy(void* instance) { static_cast(instance)->handle_parameters(); + return 0; } uint32_t WINAPI handle_process_replacing_proxy(void* instance) { static_cast(instance)->handle_process_replacing(); + return 0; } diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index fb52af1a..c9a6be69 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -74,9 +74,9 @@ class Vst2Bridge { // below. They're defined here because we can't use lambdas with WinAPI's // `CreateThread` which is needed to support the proper call conventions the // VST plugins expect. - [[noreturn]] void handle_dispatch_midi_events(); - [[noreturn]] void handle_parameters(); - [[noreturn]] void handle_process_replacing(); + void handle_dispatch_midi_events(); + void handle_parameters(); + void handle_process_replacing(); /** * Forward the host callback made by the plugin to the host and return the From a9841f21f530fb74b62ea3614aa5ddcba714c6f0 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 23:34:06 +0200 Subject: [PATCH 44/73] Include the group host binaries in the artifacts --- .github/workflows/build.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ec90a0c4..40511ec5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -51,8 +51,8 @@ jobs: set -e mkdir yabridge - # This is ran under dash which does not support brace expansion - cp build/libyabridge.so build/yabridge-host.exe build/yabridge-host.exe.so build/yabridge-host-32.exe build/yabridge-host-32.exe.so yabridge + # This gets run under dash which does not support brace expansion + cp build/libyabridge.so build/yabridge-host.exe build/yabridge-host.exe.so build/yabridge-host-32.exe build/yabridge-host-32.exe.so build/yabridge-group.exe build/yabridge-group.exe.so build/yabridge-group-32.exe build/yabridge-group-32.exe.so yabridge cp CHANGELOG.md README.md yabridge tar -caf "$ARCHIVE_NAME" yabridge @@ -94,8 +94,8 @@ jobs: set -e mkdir yabridge - # This is ran under dash which does not support brace expansion - cp build/libyabridge.so build/yabridge-host.exe build/yabridge-host.exe.so build/yabridge-host-32.exe build/yabridge-host-32.exe.so yabridge + # This gets run under dash which does not support brace expansion + cp build/libyabridge.so build/yabridge-host.exe build/yabridge-host.exe.so build/yabridge-host-32.exe build/yabridge-host-32.exe.so build/yabridge-group.exe build/yabridge-group.exe.so build/yabridge-group-32.exe build/yabridge-group-32.exe.so yabridge cp CHANGELOG.md README.md yabridge tar -caf "$ARCHIVE_NAME" yabridge From 333c5dac17d92912aff8a3ceec0701634c03d00e Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 22 May 2020 23:36:44 +0200 Subject: [PATCH 45/73] Clean up GitHub workflow --- .github/workflows/build.yml | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 40511ec5..018359f3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -14,6 +14,11 @@ on: release: types: [created] +defaults: + run: + # This otherwise gets run under dash which does not support brace expansion + shell: bash + jobs: build-bionic: name: Build on Ubuntu 18.04 @@ -48,11 +53,8 @@ jobs: ninja -C build - name: Create an archive for the binaries run: | - set -e - mkdir yabridge - # This gets run under dash which does not support brace expansion - cp build/libyabridge.so build/yabridge-host.exe build/yabridge-host.exe.so build/yabridge-host-32.exe build/yabridge-host-32.exe.so build/yabridge-group.exe build/yabridge-group.exe.so build/yabridge-group-32.exe build/yabridge-group-32.exe.so yabridge + cp build/libyabridge.so build/yabridge-{host,group}{,-32}.exe{,.so} yabridge cp CHANGELOG.md README.md yabridge tar -caf "$ARCHIVE_NAME" yabridge @@ -91,11 +93,8 @@ jobs: ninja -C build - name: Create an archive for the binaries run: | - set -e - mkdir yabridge - # This gets run under dash which does not support brace expansion - cp build/libyabridge.so build/yabridge-host.exe build/yabridge-host.exe.so build/yabridge-host-32.exe build/yabridge-host-32.exe.so build/yabridge-group.exe build/yabridge-group.exe.so build/yabridge-group-32.exe build/yabridge-group-32.exe.so yabridge + cp build/libyabridge.so build/yabridge-{host,group}{,-32}.exe{,.so} yabridge cp CHANGELOG.md README.md yabridge tar -caf "$ARCHIVE_NAME" yabridge From 9f544c194afabe4ab23536c347958ea857b41123 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 23 May 2020 13:01:22 +0200 Subject: [PATCH 46/73] Change hosting mode wording --- src/plugin/plugin-bridge.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index f013eb0d..d0a99f1e 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -773,7 +773,7 @@ void PluginBridge::log_init_message() { << std::endl; init_msg << "hosting mode: '"; if (config.group.has_value()) { - init_msg << "group \"" << config.group.value() << "\""; + init_msg << "in group \"" << config.group.value() << "\""; } else { init_msg << "individual"; } From f50d04ce0415e9e93db1865b433c0481d18661ff Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 23 May 2020 13:54:40 +0200 Subject: [PATCH 47/73] Defer group host shutdown This allows for a significant speedup in plugin scanning time for plugin groups, since starting Wine processes takes up pretty much the entirety of the time spent scanning plugins. --- src/wine-host/bridges/group.cpp | 30 +++++++++++++++++++++++------- src/wine-host/bridges/group.h | 13 ++++++++++++- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 5510d36a..6ad15b32 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -96,13 +96,16 @@ GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) stderr_redirect(io_context, STDERR_FILENO), group_socket_endpoint(group_socket_path.string()), group_socket_acceptor( - create_acceptor_if_inactive(io_context, group_socket_endpoint)) { + create_acceptor_if_inactive(io_context, group_socket_endpoint)), + shutdown_timer(io_context) { // Write this process's original STDOUT and STDERR streams to the logger async_log_pipe_lines(stdout_redirect.pipe, stdout_buffer, "[STDOUT] "); async_log_pipe_lines(stderr_redirect.pipe, stderr_buffer, "[STDERR] "); } void GroupBridge::handle_host_plugin(const GroupRequest request) { + using namespace std::literals::chrono_literals; + // At this point the `active_plugins` map will already contain a copy of // `parameters` along with this thread's handle // The initialization process for a plugin is identical to that in @@ -124,14 +127,27 @@ void GroupBridge::handle_host_plugin(const GroupRequest request) { // After the plugin has exited (either after `effClose()` or because it // failed to initialize), we'll remove this thread's plugin from the active - // plugins. If no active plugins remain, then we'll terminate + // plugins. If no active plugins remain, then we'll terminate this process. std::lock_guard lock(active_plugins_mutex); - active_plugins.erase(request); - if (active_plugins.size() == 0) { - logger.log("All plugins have exited, shutting down the group process"); - io_context.stop(); - } + + // Defer shutting down the process to allow for fast plugin scanning by + // allowing plugins to reuse the same group host process + shutdown_timer.expires_after(2s); + shutdown_timer.async_wait([&](const boost::system::error_code& error) { + // A previous timer gets canceled automatically when another plugin + // exits + if (error.failed()) { + return; + } + + std::lock_guard lock(active_plugins_mutex); + if (active_plugins.size() == 0) { + logger.log( + "All plugins have exited, shutting down the group process"); + io_context.stop(); + } + }); } void GroupBridge::handle_incoming_connections() { diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index 93039bef..fa2909f2 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -126,7 +126,9 @@ class GroupBridge { * * Once the plugin has exited, this thread will then remove itself from the * `active_plugins` map. If this causes the vector to become empty, we will - * terminate this process. + * terminate this process. This check will be delayed by a few seconds to + * prevent having to constantly restart the group process during plugin + * scanning. * * @param request Information about the plugin to launch, i.e. the path to * the plugin and the path of the socket endpoint that will be used for @@ -216,4 +218,13 @@ class GroupBridge { * plugin is being spawned. */ std::mutex active_plugins_mutex; + + /** + * A timer to defer shutting down the process, allowing for fast plugin + * scanning without having to start a new group host process for each + * plugin. + * + * @see handle_host_plugin + */ + boost::asio::steady_timer shutdown_timer; }; From 9c901935d43a88c2f6552e39c340be7483585b7e Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 23 May 2020 15:09:56 +0200 Subject: [PATCH 48/73] Fix editor GUIs for plugin groups Handles for things like timers should be unique on a per-thread basis in the Win32 API, but apparently window classes have to be unique for the entire application. --- src/wine-host/bridges/vst2.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index c2cde442..737dd249 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -361,8 +361,13 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, // provided by the host, and let the plugin embed itself into // the Wine window const auto x11_handle = reinterpret_cast(data); + // Win32 window classes have to be unique for the whole application. + // When hosting multiple plugins in a group process, all plugins + // should get a unique window class + const std::string window_class = + "yabridge plugin " + socket_endpoint.path(); Editor& editor_instance = - editor.emplace("yabridge plugin", plugin, x11_handle); + editor.emplace(window_class, plugin, x11_handle); return plugin->dispatcher(plugin, opcode, index, value, editor_instance.win32_handle.get(), From c387238b781e8f1cc31cc00934ce656278dc722c Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 23 May 2020 15:16:33 +0200 Subject: [PATCH 49/73] Change wording in group related log messages --- src/plugin/plugin-bridge.cpp | 2 +- src/wine-host/bridges/group.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index d0a99f1e..d14df208 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -773,7 +773,7 @@ void PluginBridge::log_init_message() { << std::endl; init_msg << "hosting mode: '"; if (config.group.has_value()) { - init_msg << "in group \"" << config.group.value() << "\""; + init_msg << "plugin group \"" << config.group.value() << "\""; } else { init_msg << "individual"; } diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 6ad15b32..b7236099 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -119,7 +119,7 @@ void GroupBridge::handle_host_plugin(const GroupRequest request) { // Blocks the main thread until the plugin shuts down bridge.handle_dispatch(); - logger.log("" + request.plugin_path + "' has exited"); + logger.log("'" + request.plugin_path + "' has exited"); } catch (const std::runtime_error& error) { logger.log("Error while initializing '" + request.plugin_path + "':"); logger.log(error.what()); From e61b70ed97c656ea3f336fcdd4d615de25deaf26 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 23 May 2020 15:25:05 +0200 Subject: [PATCH 50/73] Add the new plugin groups to the changelog #15 With this the plugin groups functionality is feature complete, although I still want to do a few rounds of refactoring and the readme has not yet been updated. --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 969aa638..49927f68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added + +- Added the ability to host multiple plugins in the same Wine process through + _plugin groups_. A plugin group is a user-defined set of plugins that will be + hosted together in the same Wine process. This allows multiple instances of + plugins to share data and communicate with eachother. Examples of plugins that + can benefit from this are Fabfilter Pro-Q 3, MMultiAnalyzer and the iZotope + mixing plugins. See the readme for instructions on how to set this up. + ### Changed - Changed architecture to use one less socket. From 124b62bf6b13058be534652ab1c446c57dc6bfab Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 23 May 2020 15:47:13 +0200 Subject: [PATCH 51/73] Change config path wording Since this field will only be populated if we are actually using values from a config file. --- src/plugin/plugin-bridge.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index d14df208..f0d10584 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -768,8 +768,8 @@ void PluginBridge::log_init_message() { // settings in use. Printing the matched glob pattern could also be useful // but it'll be very noisy and it's likely going to be clear from the shown // values anyways. - init_msg << "config path: '" - << config.matched_file.value_or("").string() << "'" + init_msg << "config from: '" + << config.matched_file.value_or("").string() << "'" << std::endl; init_msg << "hosting mode: '"; if (config.group.has_value()) { From 42c755cac88d99d679a3817a83d723b82c81b6ba Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 23 May 2020 15:57:58 +0200 Subject: [PATCH 52/73] Don't try to join nonexistent threads This fixes a shutdown crash for individually hosted plugins. One more reason to refactor the host launch behavior! --- src/plugin/plugin-bridge.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index f0d10584..3d24d090 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -482,7 +482,10 @@ intptr_t PluginBridge::dispatch(AEffect* /*plugin*/, // These threads should now be finished because we've forcefully // terminated the Wine process, interupting their socket operations - group_host_connect_handler.join(); + if (group_host_connect_handler.joinable()) { + // This thread is only used when using plugin groups + group_host_connect_handler.join(); + } host_callback_handler.join(); wine_io_handler.join(); From e546dd7b244060fb23de583fedd5b8f065395d60 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 23 May 2020 16:00:44 +0200 Subject: [PATCH 53/73] Change wording for individually hosted plugins --- src/plugin/plugin-bridge.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 3d24d090..c8734516 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -778,7 +778,7 @@ void PluginBridge::log_init_message() { if (config.group.has_value()) { init_msg << "plugin group \"" << config.group.value() << "\""; } else { - init_msg << "individual"; + init_msg << "individually"; } if (vst_plugin_arch == PluginArchitecture::vst_32) { init_msg << ", 32-bit"; From 2e68ade2a30c92ae8a907332f79510a42bef459e Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 24 May 2020 13:44:08 +0200 Subject: [PATCH 54/73] Allow handling events inside of an IO context This is needed when using multiple plugins since their GUI operations all have to be run from the same thread. --- src/wine-host/bridges/vst2.cpp | 78 +++++++++++++++++++++++----------- src/wine-host/bridges/vst2.h | 64 +++++++++++++++++++++++----- 2 files changed, 108 insertions(+), 34 deletions(-) diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 737dd249..1f6b1a21 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -16,6 +16,7 @@ #include "vst2.h" +#include #include #include "../../common/communication.h" @@ -154,31 +155,41 @@ void Vst2Bridge::handle_dispatch() { plugin, std::bind(&Vst2Bridge::dispatch_wrapper, this, _1, _2, _3, _4, _5, _6))); - // Because of the way the Win32 API works we have to process events - // on the same thread as the one the window was created on, and that - // thread is the thread that's handling dispatcher calls. Some - // plugins will also rely on the Win32 message loop to run tasks on - // a timer and to defer loading, so we have to make sure to always - // run this loop. The only exception is a in specific situation that - // can cause a race condition in some plugins because of incorrect - // assumptions made by the plugin. See the dostring for - // `Vst2Bridge::editor` for more information. - std::visit(overload{[](Editor& editor) { editor.handle_events(); }, - [](std::monostate&) { - MSG msg; + pump_message_loop(); + } + } catch (const boost::system::system_error&) { + // The plugin has cut off communications, so we can shut down this host + // application + } +} - while (PeekMessage(&msg, nullptr, 0, 0, - PM_REMOVE)) { - TranslateMessage(&msg); - DispatchMessage(&msg); - } - }, - [](EditorOpening&) { - // Don't handle any events in this - // particular case as explained in - // `Vst2Bridge::editor` - }}, - editor); +void Vst2Bridge::handle_dispatch(boost::asio::io_context& main_context) { + using namespace std::placeholders; + + // This works exactly the same as the function above, but execute the actual + // event and run the message loop from the main thread that's also + // instantiating these plugins. This is required for a few plugins to run + // multiple instances in the same process + try { + while (true) { + receive_event( + host_vst_dispatch, std::nullopt, + passthrough_event( + plugin, + [&](AEffect* plugin, int opcode, int index, intptr_t value, + void* data, float option) -> intptr_t { + std::promise dispatch_result; + boost::asio::dispatch(main_context, [&]() { + const intptr_t result = dispatch_wrapper( + plugin, opcode, index, value, data, option); + + dispatch_result.set_value(result); + }); + + return dispatch_result.get_future().get(); + })); + + boost::asio::post(main_context, [&]() { pump_message_loop(); }); } } catch (const boost::system::system_error&) { // The plugin has cut off communications, so we can shut down this host @@ -389,6 +400,25 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, } } +void Vst2Bridge::pump_message_loop() { + std::visit( + overload{[](Editor& editor) { editor.handle_events(); }, + [](std::monostate&) { + MSG msg; + + while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessage(&msg); + } + }, + [](EditorOpening&) { + // Don't handle any events in this + // particular case as explained in + // `Vst2Bridge::editor` + }}, + editor); +} + class HostCallbackDataConverter : DefaultDataConverter { public: HostCallbackDataConverter(AEffect* plugin, diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index c9a6be69..d86a6338 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -41,9 +41,20 @@ struct EditorOpening {}; /** - * This handles the communication between the Linux native VST plugin and the - * Wine VST host when hosting VST2 plugins. The functions below should be used - * as callback functions in an `AEffect` object. + * This hosts a Windows VST2 plugin, forwards messages sent by the Linux VST + * plugin and provides host callback function for the plugin to talk back. + * + * @remark Because of Win32 API limitations, all window handling has to be done + * from the same thread. For individually hosted plugins this only means that + * this class has to be initialized from the same thread as the one that calls + * `handle_dispatch()`, and thus also runs the message loop. When using plugin + * groups, however, all instantiation, editor event handling and message loop + * pumping has to be done from a single thread. Most plugins won't have any + * issues when using multiple message loops, but the Melda plugins for + * instance will only update their GUIs from the message loop of the thread + * that created the first instance. When running multiple plugins + * `handle_dispatch(io_context)` should be used to make sure all plugins + * handle their events on the same thread. */ class Vst2Bridge { public: @@ -56,6 +67,9 @@ class Vst2Bridge { * @param socket_endpoint_path A (Unix style) path to the Unix socket * endpoint the native VST plugin created to communicate over. * + * @note When using plugin groups and `handle_dispatch(io_context)`, this + * object has to be constructed from within the IO context. + * * @throw std::runtime_error Thrown when the VST plugin could not be loaded, * or if communication could not be set up. */ @@ -64,16 +78,31 @@ class Vst2Bridge { /** * Handle events on the main thread until the plugin quits. This can't be * done on another thread since some plugins (e.g. Melda) expect certain - * (but for some reason not all) events to be passed from the same thread it - * was initiated from. This is then also the same thread that should handle - * Win32 GUI events. + * events to be passed from the same thread it was initiated from. This is + * then also the same thread that should handle Win32 GUI events. */ void handle_dispatch(); - // These functions are the entry points for the `*_handler` threads defined - // below. They're defined here because we can't use lambdas with WinAPI's - // `CreateThread` which is needed to support the proper call conventions the - // VST plugins expect. + /** + * Handle events just like in the function above, but do the actual + * execution on the IO context. As explained in this class' docstring, this + * is needed because some plugins make the assumption that all of their + * instances are handled from the same thread, and that the thread that the + * first instance was initiated on will be kept alive until the VST host + * terminates. + * + * @param main_context The main IO context that's handling the event + * handling for all plugins. + * + * @note With this approach you'll have to make sure that the object was + * instantiated from the same thread as the one that runs the IO context. + */ + void handle_dispatch(boost::asio::io_context& main_context); + + // These functions are the entry points for the `*_handler` threads + // defined below. They're defined here because we can't use lambdas with + // WinAPI's `CreateThread` which is needed to support the proper call + // conventions the VST plugins expect. void handle_dispatch_midi_events(); void handle_parameters(); void handle_process_replacing(); @@ -104,6 +133,21 @@ class Vst2Bridge { void* data, float option); + /** + * Run the message loop for this plugin and potentially also for other + * plugins. This is called by both versions of `handle_dispatch()`. + * + * Because of the way the Win32 API works we have to process events on the + * same thread as the one the window was created on, and that thread is the + * thread that's handling dispatcher calls. Some plugins will also rely on + * the Win32 message loop to run tasks on a timer and to defer loading, so + * we have to make sure to always run this loop. The only exception is a in + * specific situation that can cause a race condition in some plugins + * because of incorrect assumptions made by the plugin. See the dostring for + * `Vst2Bridge::editor` for more information. + */ + void pump_message_loop(); + /** * The shared library handle of the VST plugin. I sadly could not get * Boost.DLL to work here, so we'll just load the VST plugisn by hand. From 1c44a2f2cba63095d192e06165e25f03db426a09 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 24 May 2020 14:55:09 +0200 Subject: [PATCH 55/73] Also filter audioMasterGetCurrentProcessLevel Some plugins call this every time they process audio. --- src/common/logging.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/logging.cpp b/src/common/logging.cpp index d8e9b892..5d373c31 100644 --- a/src/common/logging.cpp +++ b/src/common/logging.cpp @@ -306,7 +306,8 @@ bool Logger::should_filter_event(bool is_dispatch, int opcode) { // called tens of times per second // TODO: Figure out what opcode 52 is if ((is_dispatch && (opcode == effEditIdle || opcode == 52)) || - (!is_dispatch && opcode == audioMasterGetTime)) { + (!is_dispatch && (opcode == audioMasterGetTime || + opcode == audioMasterGetCurrentProcessLevel))) { return true; } From 85fb3a2588092285d379895fab10cfe599433b67 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 25 May 2020 15:03:03 +0200 Subject: [PATCH 56/73] Conditionally disiable the message loop from Based on a function. This is needed because the message loop should be skipped while any of the plugins is opening their GUI, similar to how `EditorOpening` worked for individually hosted plugins. --- src/wine-host/bridges/vst2.cpp | 37 ---------------------------- src/wine-host/bridges/vst2.h | 44 +++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 38 deletions(-) diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 1f6b1a21..56763662 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -19,9 +19,6 @@ #include #include -#include "../../common/communication.h" -#include "../../common/events.h" - /** * A function pointer to what should be the entry point of a VST plugin. */ @@ -163,40 +160,6 @@ void Vst2Bridge::handle_dispatch() { } } -void Vst2Bridge::handle_dispatch(boost::asio::io_context& main_context) { - using namespace std::placeholders; - - // This works exactly the same as the function above, but execute the actual - // event and run the message loop from the main thread that's also - // instantiating these plugins. This is required for a few plugins to run - // multiple instances in the same process - try { - while (true) { - receive_event( - host_vst_dispatch, std::nullopt, - passthrough_event( - plugin, - [&](AEffect* plugin, int opcode, int index, intptr_t value, - void* data, float option) -> intptr_t { - std::promise dispatch_result; - boost::asio::dispatch(main_context, [&]() { - const intptr_t result = dispatch_wrapper( - plugin, opcode, index, value, data, option); - - dispatch_result.set_value(result); - }); - - return dispatch_result.get_future().get(); - })); - - boost::asio::post(main_context, [&]() { pump_message_loop(); }); - } - } catch (const boost::system::system_error&) { - // The plugin has cut off communications, so we can shut down this host - // application - } -} - void Vst2Bridge::handle_dispatch_midi_events() { try { while (true) { diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index d86a6338..44128c7f 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -26,9 +26,13 @@ #include #include +#include #include +#include #include +#include "../../common/communication.h" +#include "../../common/events.h" #include "../../common/logging.h" #include "../editor.h" #include "../utils.h" @@ -93,11 +97,49 @@ class Vst2Bridge { * * @param main_context The main IO context that's handling the event * handling for all plugins. + * @param message_loop_blocked A function that returns true if the message + * loop is blocked. This is used to temporarily postpone running the + * message loop while a plugin is opening its GUI. * * @note With this approach you'll have to make sure that the object was * instantiated from the same thread as the one that runs the IO context. */ - void handle_dispatch(boost::asio::io_context& main_context); + template + void handle_dispatch(boost::asio::io_context& main_context, + const F& message_loop_blocked) { + // This works exactly the same as the function above, but execute the + // actual event and run the message loop from the main thread that's + // also instantiating these plugins. This is required for a few plugins + // to run multiple instances in the same process + try { + while (true) { + receive_event( + host_vst_dispatch, std::nullopt, + passthrough_event( + plugin, + [&](AEffect* plugin, int opcode, int index, + intptr_t value, void* data, + float option) -> intptr_t { + std::promise dispatch_result; + boost::asio::dispatch(main_context, [&]() { + const intptr_t result = dispatch_wrapper( + plugin, opcode, index, value, data, option); + + dispatch_result.set_value(result); + + if (!message_loop_blocked()) { + pump_message_loop(); + } + }); + + return dispatch_result.get_future().get(); + })); + } + } catch (const boost::system::system_error&) { + // The plugin has cut off communications, so we can shut down this + // host application + } + } // These functions are the entry points for the `*_handler` threads // defined below. They're defined here because we can't use lambdas with From 23f15c8d8a47ac519db5e7673cf4046028582cd1 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 25 May 2020 15:09:55 +0200 Subject: [PATCH 57/73] Rename the two handle_dispatch functions To better differentiate between their intended uses. --- src/wine-host/bridges/vst2.cpp | 8 ++++---- src/wine-host/bridges/vst2.h | 22 +++++++++++----------- src/wine-host/individual-host.cpp | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 56763662..28f62542 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -127,9 +127,9 @@ Vst2Bridge::Vst2Bridge(std::string plugin_dll_path, // `audioMasterIOChanged` host callback. write_object(host_vst_dispatch, EventResult{0, *plugin, std::nullopt}); - // This works functionally identically to the `handle_dispatch()` function - // below, but this socket will only handle MIDI events. This is needed - // because of Win32 API limitations. + // This works functionally identically to the `handle_dispatch_single()` + // function below, but this socket will only handle MIDI events. This is + // needed because of Win32 API limitations. dispatch_midi_events_handler = Win32Thread(handle_dispatch_midi_events_proxy, this); @@ -139,7 +139,7 @@ Vst2Bridge::Vst2Bridge(std::string plugin_dll_path, Win32Thread(handle_process_replacing_proxy, this); } -void Vst2Bridge::handle_dispatch() { +void Vst2Bridge::handle_dispatch_single() { using namespace std::placeholders; // For our communication we use simple threads and blocking operations diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index 44128c7f..d212c80e 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -51,13 +51,13 @@ struct EditorOpening {}; * @remark Because of Win32 API limitations, all window handling has to be done * from the same thread. For individually hosted plugins this only means that * this class has to be initialized from the same thread as the one that calls - * `handle_dispatch()`, and thus also runs the message loop. When using plugin - * groups, however, all instantiation, editor event handling and message loop - * pumping has to be done from a single thread. Most plugins won't have any - * issues when using multiple message loops, but the Melda plugins for - * instance will only update their GUIs from the message loop of the thread - * that created the first instance. When running multiple plugins - * `handle_dispatch(io_context)` should be used to make sure all plugins + * `handle_dispatch_single()`, and thus also runs the message loop. When using + * plugin groups, however, all instantiation, editor event handling and + * message loop pumping has to be done from a single thread. Most plugins + * won't have any issues when using multiple message loops, but the Melda + * plugins for instance will only update their GUIs from the message loop of + * the thread that created the first instance. When running multiple plugins + * `handle_dispatch_multi()` should be used to make sure all plugins * handle their events on the same thread. */ class Vst2Bridge { @@ -71,7 +71,7 @@ class Vst2Bridge { * @param socket_endpoint_path A (Unix style) path to the Unix socket * endpoint the native VST plugin created to communicate over. * - * @note When using plugin groups and `handle_dispatch(io_context)`, this + * @note When using plugin groups and `handle_dispatch_multi()`, this * object has to be constructed from within the IO context. * * @throw std::runtime_error Thrown when the VST plugin could not be loaded, @@ -85,7 +85,7 @@ class Vst2Bridge { * events to be passed from the same thread it was initiated from. This is * then also the same thread that should handle Win32 GUI events. */ - void handle_dispatch(); + void handle_dispatch_single(); /** * Handle events just like in the function above, but do the actual @@ -105,8 +105,8 @@ class Vst2Bridge { * instantiated from the same thread as the one that runs the IO context. */ template - void handle_dispatch(boost::asio::io_context& main_context, - const F& message_loop_blocked) { + void handle_dispatch_multi(boost::asio::io_context& main_context, + const F& message_loop_blocked) { // This works exactly the same as the function above, but execute the // actual event and run the message loop from the main thread that's // also instantiating these plugins. This is required for a few plugins diff --git a/src/wine-host/individual-host.cpp b/src/wine-host/individual-host.cpp index acd50b7e..697cedcb 100644 --- a/src/wine-host/individual-host.cpp +++ b/src/wine-host/individual-host.cpp @@ -61,7 +61,7 @@ int __cdecl main(int argc, char* argv[]) { << std::endl; // Blocks the main thread until the plugin shuts down - bridge.handle_dispatch(); + bridge.handle_dispatch_single(); } catch (const std::runtime_error& error) { std::cerr << "Error while initializing Wine VST host:" << std::endl; std::cerr << error.what() << std::endl; From bbfe522343388a322ec7a5577d6bb3e5765be18a Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 25 May 2020 15:15:32 +0200 Subject: [PATCH 58/73] Use a seperate thread for STDIO capture for groups When running the plugins on the main thread the window message loop may block for a while, which would cause the STDIO redirect to be interrupted. --- src/wine-host/bridges/group.cpp | 12 ++++++++++-- src/wine-host/bridges/group.h | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index b7236099..4368b4b9 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -92,8 +92,9 @@ GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) : logger(Logger::create_from_environment( create_logger_prefix(group_socket_path))), io_context(), - stdout_redirect(io_context, STDOUT_FILENO), - stderr_redirect(io_context, STDERR_FILENO), + stdio_context(), + stdout_redirect(stdio_context, STDOUT_FILENO), + stderr_redirect(stdio_context, STDERR_FILENO), group_socket_endpoint(group_socket_path.string()), group_socket_acceptor( create_acceptor_if_inactive(io_context, group_socket_endpoint)), @@ -101,6 +102,13 @@ GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) // Write this process's original STDOUT and STDERR streams to the logger async_log_pipe_lines(stdout_redirect.pipe, stdout_buffer, "[STDOUT] "); async_log_pipe_lines(stderr_redirect.pipe, stderr_buffer, "[STDERR] "); + + stdio_handler = std::thread([&]() { stdio_context.run(); }); +} + +GroupBridge::~GroupBridge() { + stdio_context.stop(); + stdio_handler.join(); } void GroupBridge::handle_host_plugin(const GroupRequest request) { diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index fa2909f2..aaa395e3 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -22,6 +22,8 @@ #include #include +#include + #include "vst2.h" /** @@ -116,6 +118,11 @@ class GroupBridge { */ GroupBridge(boost::filesystem::path group_socket_path); + ~GroupBridge(); + + GroupBridge(const GroupBridge&) = delete; + GroupBridge& operator=(const GroupBridge&) = delete; + /** * Host a new plugin within this process. Called by proxy using * `handle_host_plugin_proxy()` in `./group.cpp` because the Win32 @@ -180,6 +187,14 @@ class GroupBridge { Logger logger; boost::asio::io_context io_context; + /** + * A seperate IO context that handles the STDIO redirect through + * `StdIoCapture`. This is seperated the `plugin_context` above so that + * STDIO capture does not get blocked by blocking GUI operations. Since + * every GUI related operation should be run from the same thread, we can't + * just add another thread to the main IO context. + */ + boost::asio::io_context stdio_context; boost::asio::streambuf stdout_buffer; boost::asio::streambuf stderr_buffer; @@ -195,6 +210,10 @@ class GroupBridge { * able write it write it to an external log file. */ StdIoCapture stderr_redirect; + /** + * A thread that runs the `stdio_context` loop. + */ + std::thread stdio_handler; boost::asio::local::stream_protocol::endpoint group_socket_endpoint; /** From 064bb2684fa24f32a91541257b566419b9d37b93 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 25 May 2020 15:19:46 +0200 Subject: [PATCH 59/73] Init plugins and handle events on the main thread Within the plugin IO context. --- src/wine-host/bridges/group.cpp | 104 +++++++++++++++++++------------- src/wine-host/bridges/group.h | 72 +++++++++++++++------- 2 files changed, 112 insertions(+), 64 deletions(-) diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 4368b4b9..ef1c16d5 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -52,16 +52,16 @@ boost::asio::local::stream_protocol::acceptor create_acceptor_if_inactive( */ std::string create_logger_prefix(const fs::path& socket_path); -uint32_t WINAPI handle_host_plugin_proxy(void* param); +uint32_t WINAPI handle_plugin_dispatch_proxy(void* param); /** * CreateThread() is great and allows you to pass a single value to the * function, so we'll use this to pass both `this` and the parameters to the * below thread function so it can do its thing. * - * @relates handle_host_plugin_proxy + * @relates handle_plugin_dispatch_proxy */ -using handle_host_plugin_parameters = std::pair; +using handle_plugin_dispatch_parameters = std::pair; StdIoCapture::StdIoCapture(boost::asio::io_context& io_context, int file_descriptor) @@ -91,15 +91,17 @@ StdIoCapture::~StdIoCapture() { GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) : logger(Logger::create_from_environment( create_logger_prefix(group_socket_path))), - io_context(), + plugin_context(), stdio_context(), stdout_redirect(stdio_context, STDOUT_FILENO), stderr_redirect(stdio_context, STDERR_FILENO), group_socket_endpoint(group_socket_path.string()), group_socket_acceptor( - create_acceptor_if_inactive(io_context, group_socket_endpoint)), - shutdown_timer(io_context) { + create_acceptor_if_inactive(plugin_context, group_socket_endpoint)), + shutdown_timer(plugin_context) { // Write this process's original STDOUT and STDERR streams to the logger + // TODO: This works for output generated by plugins, but not for debug + // messages generated by wineserver. Is it possible to catch those? async_log_pipe_lines(stdout_redirect.pipe, stdout_buffer, "[STDOUT] "); async_log_pipe_lines(stderr_redirect.pipe, stderr_buffer, "[STDERR] "); @@ -111,36 +113,30 @@ GroupBridge::~GroupBridge() { stdio_handler.join(); } -void GroupBridge::handle_host_plugin(const GroupRequest request) { +void GroupBridge::handle_plugin_dispatch(const GroupRequest request) { using namespace std::literals::chrono_literals; - // At this point the `active_plugins` map will already contain a copy of - // `parameters` along with this thread's handle - // The initialization process for a plugin is identical to that in - // `../individual-host.cpp` - logger.log("Received request to host '" + request.plugin_path + - "' using socket '" + request.socket_path + "'"); - try { - Vst2Bridge bridge(request.plugin_path, request.socket_path); - logger.log("Finished initializing '" + request.plugin_path + "'"); + // At this point the `active_plugins` map will already contain the + // intialized plugin's `Vst2Bridge` instance and this thread's handle + auto& [thread, bridge] = active_plugins.at(request); - // Blocks the main thread until the plugin shuts down - bridge.handle_dispatch(); + // Blocks the main thread until the plugin shuts down, handling all events + // on the main IO context + // TODO: Maybe add a function to each vstbridge that returns whether the + // message loop should be postponed, and pass a function here that + // checks if this is the case for any of the plugins? + bridge->handle_dispatch_multi( + plugin_context, [&]() { return should_postpone_message_loop(); }); + logger.log("'" + request.plugin_path + "' has exited"); - logger.log("'" + request.plugin_path + "' has exited"); - } catch (const std::runtime_error& error) { - logger.log("Error while initializing '" + request.plugin_path + "':"); - logger.log(error.what()); - } - - // After the plugin has exited (either after `effClose()` or because it - // failed to initialize), we'll remove this thread's plugin from the active - // plugins. If no active plugins remain, then we'll terminate this process. + // After the plugin has exited, we'll remove this thread's plugin from the + // active plugins. If no active plugins remain, then we'll terminate the + // process. std::lock_guard lock(active_plugins_mutex); active_plugins.erase(request); - // Defer shutting down the process to allow for fast plugin scanning by - // allowing plugins to reuse the same group host process + // Defer actually shutting down the process to allow for fast plugin + // scanning by allowing plugins to reuse the same group host process shutdown_timer.expires_after(2s); shutdown_timer.async_wait([&](const boost::system::error_code& error) { // A previous timer gets canceled automatically when another plugin @@ -153,7 +149,7 @@ void GroupBridge::handle_host_plugin(const GroupRequest request) { if (active_plugins.size() == 0) { logger.log( "All plugins have exited, shutting down the group process"); - io_context.stop(); + plugin_context.stop(); } }); } @@ -163,7 +159,12 @@ void GroupBridge::handle_incoming_connections() { logger.log( "Group host is up and running, now accepting incoming connections"); - io_context.run(); + plugin_context.run(); +} + +bool GroupBridge::should_postpone_message_loop() { + // TODO: Iterate over the plugins + return false; } void GroupBridge::accept_requests() { @@ -177,7 +178,7 @@ void GroupBridge::accept_requests() { logger.log("Error while listening for incoming connections:"); logger.log(error.message()); - io_context.stop(); + plugin_context.stop(); } // Read the parameters, and then host the plugin in this process @@ -186,19 +187,36 @@ void GroupBridge::accept_requests() { // yabridge plugin will be able to tell if the plugin has caused // this process to crash during its initialization to prevent // waiting indefinitely on the sockets to be connected to. - const auto parameters = read_object(socket); + const auto request = read_object(socket); write_object(socket, GroupResponse{boost::this_process::get_id()}); // Collisions in the generated socket names should be very rare, but // it could in theory happen - assert(active_plugins.find(parameters) == active_plugins.end()); + assert(active_plugins.find(request) == active_plugins.end()); - // CreateThread() doesn't support multiple arguments and requires - // manualy memory management. - handle_host_plugin_parameters* thread_params = - new std::pair(this, parameters); - active_plugins[parameters] = - Win32Thread(handle_host_plugin_proxy, thread_params); + // The plugin has to be initiated on the IO context's thread because + // this has to be done on the same thread that's handling messages, + // and all window messages have to be handled from the same thread. + logger.log("Received request to host '" + request.plugin_path + + "' using socket '" + request.socket_path + "'"); + try { + auto bridge = std::make_unique(request.plugin_path, + request.socket_path); + logger.log("Finished initializing '" + request.plugin_path + + "'"); + + // CreateThread() doesn't support multiple arguments and + // requires manualy memory management. + handle_plugin_dispatch_parameters* thread_params = + new std::pair(this, request); + active_plugins[request] = std::pair( + Win32Thread(handle_plugin_dispatch_proxy, thread_params), + std::move(bridge)); + } catch (const std::runtime_error& error) { + logger.log("Error while initializing '" + request.plugin_path + + "':"); + logger.log(error.what()); + } accept_requests(); }); @@ -284,15 +302,15 @@ std::string create_logger_prefix(const fs::path& socket_path) { return "[" + socket_name + "] "; } -uint32_t WINAPI handle_host_plugin_proxy(void* param) { +uint32_t WINAPI handle_plugin_dispatch_proxy(void* param) { // The Win32 API only allows you to pass a void pointer to threads, so we // need to use manual memory management. - auto thread_params = static_cast(param); + auto thread_params = static_cast(param); GroupBridge* instance = thread_params->first; GroupRequest parameters = thread_params->second; delete thread_params; - instance->handle_host_plugin(parameters); + instance->handle_plugin_dispatch(parameters); return 0; } diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index aaa395e3..074d0ee5 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -124,12 +124,12 @@ class GroupBridge { GroupBridge& operator=(const GroupBridge&) = delete; /** - * Host a new plugin within this process. Called by proxy using - * `handle_host_plugin_proxy()` in `./group.cpp` because the Win32 - * `CreateThread` API only allows passing a single pointer to the function - * and does not allow lambdas. Because we don't have access to our own - * thread handle, the thread that's listening on the group socket will have - * already added this thread to the `active_plugins` map. + * Run a plugin's dispatcher and message loop, processing all events on the + * main IO context. The plugin will have already been created in + * `accept_requests` since it has to be initiated inside of the IO context's + * thread. Called by proxy using `handle_plugin_dispatch_proxy()` in + * `./group.cpp` because the Win32 `CreateThread` API only allows passing a + * single pointer to the function and does not allow lambdas. * * Once the plugin has exited, this thread will then remove itself from the * `active_plugins` map. If this causes the vector to become empty, we will @@ -145,7 +145,7 @@ class GroupBridge { * then the process will never exit on its own. This should not happen * though. */ - void handle_host_plugin(const GroupRequest request); + void handle_plugin_dispatch(const GroupRequest request); /** * Listen for new requests to spawn plugins within this process and handle @@ -153,15 +153,33 @@ class GroupBridge { */ void handle_incoming_connections(); + /** + * Returns true if the message loop should not be run at this time. This is + * necessary because hosts will always call either `effEditOpen()` and then + * `effEditGetRect()` or the other way around. If the message loop is + * handled in between these two actions, then some plugins will either + * freeze or sometimes outright crash. Because every plugin has to be run + * from the same thread, this is a simple way to synchronize blocking the + * mesage loop between the different plugin instances. + */ + bool should_postpone_message_loop(); + private: /** * Listen on the group socket for incoming requests to host a new plugin - * within this group process. This will asynchronously listen on the socket, - * and for any connection made it will retrieve a `GroupRequest` object - * containing information about the plugin to host and then spawn a new - * thread to start hosting that plugin. + * within this group process. This will read a `GoupRequest` object + * containing information about the plugin, reply with this process's PID so + * the yabridge instance can tell if the plugin crashed during + * initialization, and it will then try to initialize the plugin. After + * intialization the plugin handling will be handed over to a new thread + * running `handle_plugin_dispatch()`. Because of the way the Win32 API + * works, all plugins have to be initialized from the same thread, and all + * event handling and message loop interaction also has to be done from that + * thread, which is why we initialize the plugin here and use the + * `handle_dispatch_multi()` function to run events within the same + * `plugin_context`. * - * @see handle_host_plugin + * @see handle_plugin_dispatch */ void accept_requests(); @@ -186,7 +204,12 @@ class GroupBridge { */ Logger logger; - boost::asio::io_context io_context; + /** + * The IO context that connections will be accepted on, and that any plugin + * operations that may involve the Win32 mesasge loop (e.g. initialization + * and most `AEffect::dispatcher()` calls) should be run on. + */ + boost::asio::io_context plugin_context; /** * A seperate IO context that handles the STDIO redirect through * `StdIoCapture`. This is seperated the `plugin_context` above so that @@ -223,16 +246,23 @@ class GroupBridge { boost::asio::local::stream_protocol::acceptor group_socket_acceptor; /** - * A map of threads that are currently hosting a plugin within this process. - * After a plugin has exited or its initialization has failed, the thread - * handling it will remove itself from this map. This is to keep track of - * the amount of plugins currently running with their associated thread - * handles. + * A map of threads that are currently hosting a plugin within this process + * along with their plugin instance. After a plugin has exited or its + * initialization has failed, the thread handling it will remove itself from + * this map. This is to keep track of the amount of plugins currently + * running with their associated thread handles. + * + * TODO: Check again if we can just use std::thread here instead, that would + * make everything much simpler. `std::thread` was a problem with + * gdiplus in the past as Serum would randomly crash because calling + * conventions were nto being respected. */ - std::unordered_map active_plugins; + std::unordered_map>> + active_plugins; /** * A mutex to prevent two threads from simultaneously accessing the plugins - * map, and also to prevent `handle_host_plugin()` from terminating the + * map, and also to prevent `handle_plugin_dispatch()` from terminating the * process because it thinks there are no active plugins left just as a new * plugin is being spawned. */ @@ -243,7 +273,7 @@ class GroupBridge { * scanning without having to start a new group host process for each * plugin. * - * @see handle_host_plugin + * @see handle_plugin_dispatch */ boost::asio::steady_timer shutdown_timer; }; From 9a350239906440fea552d272ad6e26dac2b03152 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 26 May 2020 11:11:34 +0200 Subject: [PATCH 60/73] Split X11 and Win32 event handling X11 events should always be handled since it's thread safe and they don't block. --- src/wine-host/bridges/vst2.cpp | 3 ++- src/wine-host/bridges/vst2.h | 5 +++-- src/wine-host/editor.cpp | 13 ++++++++----- src/wine-host/editor.h | 13 +++++++++---- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 28f62542..0331356a 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -152,7 +152,8 @@ void Vst2Bridge::handle_dispatch_single() { plugin, std::bind(&Vst2Bridge::dispatch_wrapper, this, _1, _2, _3, _4, _5, _6))); - pump_message_loop(); + handle_win32_events(); + handle_x11_events(); } } catch (const boost::system::system_error&) { // The plugin has cut off communications, so we can shut down this host diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index d212c80e..6f6404a5 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -126,10 +126,11 @@ class Vst2Bridge { plugin, opcode, index, value, data, option); dispatch_result.set_value(result); - if (!message_loop_blocked()) { - pump_message_loop(); + handle_win32_events(); } + + handle_x11_events(); }); return dispatch_result.get_future().get(); diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 7279e2f3..955ffe72 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -87,9 +87,9 @@ Editor::Editor(const std::string& window_class_name, // The Win32 API will block the `DispatchMessage` call when opening e.g. a // dropdown, but it will still allow timers to be run so the GUI can still // update in the background. Because of this we send `effEditIdle` to the - // plugin on a timer. The refresh rate is purposely fairly low since we - // we'll also trigger this manually in `Editor::handle_events()` whenever - // the plugin is not busy. + // plugin on a timer. The refresh rate is purposely fairly low since the + // host will call `effEditIdle()` explicitely when the plugin is not busy. + // TODO: Add a `KillTimer()` now that we are hosting multiple plugins SetTimer(win32_handle.get(), idle_timer_id, 100, nullptr); // We need to tell the Wine window it has been moved whenever the window @@ -137,7 +137,7 @@ void Editor::send_idle_event() { plugin->dispatcher(plugin, effEditIdle, 0, 0, nullptr, 0); } -void Editor::handle_events() { +void Editor::handle_win32_events() { MSG msg; // The null value for the second argument is needed to handle interaction @@ -158,8 +158,9 @@ void Editor::handle_events() { TranslateMessage(&msg); DispatchMessage(&msg); } +} - // Handle X11 events +void Editor::handle_x11_events() { // TODO: Initiating drag-and-drop in Serum _sometimes_ causes the GUI to // update while dragging while other times it does not. From all the // plugins I've tested this only happens in Serum though. @@ -195,6 +196,8 @@ void Editor::handle_events() { // We can't directly use the `event.x` and `event.y` coordinates // because the parent window may also be embedded inside another // window. + // TODO: With plugin groups this has to be done any time the + // mouse cursor enters the window on a FOCUS_IN event const auto translate_cookie = xcb_translate_coordinates( x11_connection.get(), parent_window, root, 0, 0); const xcb_translate_coordinates_reply_t* diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 1d127665..a58eb70e 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -101,11 +101,16 @@ class Editor { void send_idle_event(); /** - * Pump messages from the editor GUI's event loop until all events are - * process. Must be run from the same thread the GUI was created in because - * of Win32 limitations. + * Pump messages from the editor loop loop until all events are process. + * Must be run from the same thread the GUI was created in because of Win32 + * limitations. */ - void handle_events(); + void handle_win32_events(); + + /** + * Handle X11 events sent to the window our editor is embedded in. + */ + void handle_x11_events(); private: /** From 16fce5577d30ddb09769aa163f58c66c0025b7ea Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 26 May 2020 11:12:36 +0200 Subject: [PATCH 61/73] Skip the message loop when an editor is opening This is a bit more restrictive than the old approach that only skipped when `effEditOpen()` got called after `effEditGetRect()`. Not sure why the Melda plugins block indefinitely on the message loop without this now. --- src/wine-host/bridges/group.cpp | 19 +++++++-- src/wine-host/bridges/group.h | 2 +- src/wine-host/bridges/vst2.cpp | 74 ++++++++++++++++++++------------- src/wine-host/bridges/vst2.h | 58 ++++++++++++++------------ 4 files changed, 93 insertions(+), 60 deletions(-) diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index ef1c16d5..607201fe 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -125,8 +125,8 @@ void GroupBridge::handle_plugin_dispatch(const GroupRequest request) { // TODO: Maybe add a function to each vstbridge that returns whether the // message loop should be postponed, and pass a function here that // checks if this is the case for any of the plugins? - bridge->handle_dispatch_multi( - plugin_context, [&]() { return should_postpone_message_loop(); }); + bridge->handle_dispatch_multi(plugin_context, + [&]() { return should_skip_message_loop(); }); logger.log("'" + request.plugin_path + "' has exited"); // After the plugin has exited, we'll remove this thread's plugin from the @@ -162,8 +162,19 @@ void GroupBridge::handle_incoming_connections() { plugin_context.run(); } -bool GroupBridge::should_postpone_message_loop() { - // TODO: Iterate over the plugins +bool GroupBridge::should_skip_message_loop() { + // We do not need additional locking since the call to `AEffect::dispatcher` + // and the actual event handling and message loop handling are performed + // within the IO context and these values thus can't change while another + // the message loop is being running + std::lock_guard lock(active_plugins_mutex); + for (auto& [parameters, value] : active_plugins) { + auto& [thread, bridge] = value; + if (bridge->should_skip_message_loop()) { + return true; + } + } + return false; } diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index 074d0ee5..87fd9d07 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -162,7 +162,7 @@ class GroupBridge { * from the same thread, this is a simple way to synchronize blocking the * mesage loop between the different plugin instances. */ - bool should_postpone_message_loop(); + bool should_skip_message_loop(); private: /** diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 0331356a..7ebd852b 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -139,6 +139,10 @@ Vst2Bridge::Vst2Bridge(std::string plugin_dll_path, Win32Thread(handle_process_replacing_proxy, this); } +bool Vst2Bridge::should_skip_message_loop() { + return editor_is_opening; +} + void Vst2Bridge::handle_dispatch_single() { using namespace std::placeholders; @@ -152,7 +156,12 @@ void Vst2Bridge::handle_dispatch_single() { plugin, std::bind(&Vst2Bridge::dispatch_wrapper, this, _1, _2, _3, _4, _5, _6))); - handle_win32_events(); + // Don't run them message loop during the two step process of + // opening the plugin editor since some plugins don't expect this + if (!should_skip_message_loop()) { + handle_win32_events(); + } + handle_x11_events(); } } catch (const boost::system::system_error&) { @@ -320,18 +329,27 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, case effEditGetRect: { // Some plugins will have a race condition if the message loops gets // handled between the call to `effEditGetRect()` and - // `effEditOpen()`, although this behavior never appears on Windows - // as hosts will always either call these functions in sequence or - // in reverse. We need to temporarily stop handling messages when - // this happens. - if (!std::holds_alternative(editor)) { - editor = EditorOpening{}; - } + // `effEditOpen()`, since this won't ever happen on Windows and + // plugins thus assume that this can't happen at all. If + // `effEditOpen()` has not yet been called, then we'll mark the + // editor as currently opening to prevent the message loop from + // running. + editor_is_opening = !editor.has_value(); return plugin->dispatcher(plugin, opcode, index, value, data, option); } break; case effEditOpen: { + // As explained above, if `effEditGetRect()` was called first then + // after this the editor has finally been opened. Otherwise if this + // function was first then we'll say that the editor is in the + // process of being opened as the host will call `effEditGetRect()` + // next. + // TODO: Without plugin groups only the `effEditGetRect()` -> + // `effEditOpen()`, is skipping the message loop in between + // `effEditOpen()` and `effEditGetRect()` really needed? + editor_is_opening = !editor_is_opening; + // Create a Win32 window through Wine, embed it into the window // provided by the host, and let the plugin embed itself into // the Wine window @@ -341,19 +359,17 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, // should get a unique window class const std::string window_class = "yabridge plugin " + socket_endpoint.path(); - Editor& editor_instance = - editor.emplace(window_class, plugin, x11_handle); + editor.emplace(window_class, plugin, x11_handle); return plugin->dispatcher(plugin, opcode, index, value, - editor_instance.win32_handle.get(), - option); + editor->win32_handle.get(), option); } break; case effEditClose: { const intptr_t return_value = plugin->dispatcher(plugin, opcode, index, value, data, option); // Cleanup is handled through RAII - editor = std::monostate(); + editor.reset(); return return_value; } break; @@ -364,23 +380,23 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, } } -void Vst2Bridge::pump_message_loop() { - std::visit( - overload{[](Editor& editor) { editor.handle_events(); }, - [](std::monostate&) { - MSG msg; +void Vst2Bridge::handle_win32_events() { + if (editor.has_value()) { + editor->handle_win32_events(); + } else { + MSG msg; - while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) { - TranslateMessage(&msg); - DispatchMessage(&msg); - } - }, - [](EditorOpening&) { - // Don't handle any events in this - // particular case as explained in - // `Vst2Bridge::editor` - }}, - editor); + while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessage(&msg); + } + } +} + +void Vst2Bridge::handle_x11_events() { + if (editor.has_value()) { + editor->handle_x11_events(); + } } class HostCallbackDataConverter : DefaultDataConverter { diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index 6f6404a5..4a35db09 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -37,13 +37,6 @@ #include "../editor.h" #include "../utils.h" -/** - * A marker struct to indicate that the editor is about to be opened. - * - * @see Vst2Bridge::editor - */ -struct EditorOpening {}; - /** * This hosts a Windows VST2 plugin, forwards messages sent by the Linux VST * plugin and provides host callback function for the plugin to talk back. @@ -79,6 +72,17 @@ class Vst2Bridge { */ Vst2Bridge(std::string plugin_dll_path, std::string socket_endpoint_path); + /** + * Returns true if the message loop should be skipped. This happens when the + * editor is in the process of being opened. In VST hosts on Windows + * `effEditOpen()` and `effEditGetRect()` will always be called in sequence, + * but in our approach there will be an opportunity to handle events in + * between these two calls. Most plugins will handle this just fine, but + * some plugins end up blocking indefinitely while waiting for the other + * function to be called, hence why this function is needed. + */ + bool should_skip_message_loop(); + /** * Handle events on the main thread until the plugin quits. This can't be * done on another thread since some plugins (e.g. Melda) expect certain @@ -178,7 +182,9 @@ class Vst2Bridge { /** * Run the message loop for this plugin and potentially also for other - * plugins. This is called by both versions of `handle_dispatch()`. + * plugins. This is only used in `handle_dispatch_single()`, as this will be + * run on a timer when using plugin groups. The caller should first check + * whether the event loop can be run through `should_skip_message_loop()`. * * Because of the way the Win32 API works we have to process events on the * same thread as the one the window was created on, and that thread is the @@ -189,7 +195,13 @@ class Vst2Bridge { * because of incorrect assumptions made by the plugin. See the dostring for * `Vst2Bridge::editor` for more information. */ - void pump_message_loop(); + void handle_win32_events(); + + /** + * Handle X11 events for the editor window if it is open. This can be run + * safely from any thread. + */ + void handle_x11_events(); /** * The shared library handle of the VST plugin. I sadly could not get @@ -280,23 +292,17 @@ class Vst2Bridge { * Wine window, and embedding that Wine window into a window provided by the * host. Should be empty when the editor is not open. * - * This field can have three possible states: + * There is some special behavior with regards to message handling when the + * editor is in the process of being opened, see + * `should_postpone_message_loop()`. * - * - `std::nullopt` when the editor is closed. - * - An `Editor` object when the editor is open. - * - `EditorOpening` when the editor is not yet open, but the host has - * already called `effEditGetRect()` and is about to call `effEditOpen()`. - * This is needed because there is a race condition in some bugs that - * cause them to crash or enter an infinite Win32 message loop when - * `effEditGetRect()` gets dispatched and we then enter the message loop - * loop before `effEditOpen()` gets called. Most plugins will handle this - * just fine, but a select few plugins make the assumption that the editor - * is already open once `effEditGetRect()` has been called, even if - * `effEditOpen` has not yet been dispatched. VST hsots on Windows will - * call these two events in sequence, so the bug would never occur there. - * To work around this we'll use this third state to temporarily stop - * processing Windows events in the one or two ticks between these two - * events. + * @see should_postpone_message_loop */ - std::variant editor; + std::optional editor; + + /** + * Keeps track of when the editor is being opened during the two-phase + * process of the host calling `effEditOpen()` and `effEditGetRect()`. + */ + bool editor_is_opening = false; }; From 198807a15a9a91d4457a9fd37371c908e3a14662 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 26 May 2020 12:09:27 +0200 Subject: [PATCH 62/73] Run events async and centralized for group hosts At a 30 fps rate with a limit on the number of window messages per frame. This is somehow needed for Melda plugins, as they otherwise dispatch tiemr messages indefinitely after opening a second editor with seemingly no way around it. With this and some refactoring #15 should be almost done. --- src/wine-host/bridges/group.cpp | 64 ++++++++++++++++++++++++++++----- src/wine-host/bridges/group.h | 15 ++++++++ src/wine-host/bridges/vst2.cpp | 49 +++++++++++++++++++++---- src/wine-host/bridges/vst2.h | 58 +++++------------------------- 4 files changed, 122 insertions(+), 64 deletions(-) diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 607201fe..1251c3ba 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -27,6 +27,13 @@ // path operation will thrown an encoding related error namespace fs = boost::filesystem; +using namespace std::literals::chrono_literals; + +/** + * The delay between calls to the event loop at a more than cinematic 30 fps. + */ +constexpr std::chrono::duration event_loop_interval = 1000ms / 30; + /** * Listen on the specified endpoint if no process is already listening there, * otherwise throw. This is needed to handle these three situations: @@ -98,6 +105,7 @@ GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) group_socket_endpoint(group_socket_path.string()), group_socket_acceptor( create_acceptor_if_inactive(plugin_context, group_socket_endpoint)), + events_timer(plugin_context), shutdown_timer(plugin_context) { // Write this process's original STDOUT and STDERR streams to the logger // TODO: This works for output generated by plugins, but not for debug @@ -114,19 +122,13 @@ GroupBridge::~GroupBridge() { } void GroupBridge::handle_plugin_dispatch(const GroupRequest request) { - using namespace std::literals::chrono_literals; - // At this point the `active_plugins` map will already contain the // intialized plugin's `Vst2Bridge` instance and this thread's handle auto& [thread, bridge] = active_plugins.at(request); - // Blocks the main thread until the plugin shuts down, handling all events - // on the main IO context - // TODO: Maybe add a function to each vstbridge that returns whether the - // message loop should be postponed, and pass a function here that - // checks if this is the case for any of the plugins? - bridge->handle_dispatch_multi(plugin_context, - [&]() { return should_skip_message_loop(); }); + // Blocks this thread until the plugin shuts down, handling all events on + // the main IO context + bridge->handle_dispatch_multi(plugin_context); logger.log("'" + request.plugin_path + "' has exited"); // After the plugin has exited, we'll remove this thread's plugin from the @@ -156,6 +158,7 @@ void GroupBridge::handle_plugin_dispatch(const GroupRequest request) { void GroupBridge::handle_incoming_connections() { accept_requests(); + async_handle_events(); logger.log( "Group host is up and running, now accepting incoming connections"); @@ -233,6 +236,49 @@ void GroupBridge::accept_requests() { }); } +void GroupBridge::async_handle_events() { + // Try to keep a steady framerate, but add in delays to let other events get + // handled if the GUI message handling somehow takes very long. + events_timer.expires_at( + std::max(events_timer.expiry() + event_loop_interval, + std::chrono::steady_clock::now() + 5ms)); + events_timer.async_wait([&](const boost::system::error_code& error) { + if (error.failed()) { + return; + } + + { + // Always handle X11 events + std::lock_guard lock(active_plugins_mutex); + for (auto& [parameters, value] : active_plugins) { + auto& [thread, bridge] = value; + bridge->handle_x11_events(); + } + } + + // Handle Win32 messages unless plugins are in the middle of opening + // their editor + // TODO: Check if those same weird crashes with Serum are happening + // again with these normal threads + if (!should_skip_message_loop()) { + MSG msg; + + // Keep the loop responsive by not handling too many events at once + // TODO: For some reason the Melda plugins run into a seemingly + // infinite timer loop for a little while after opening a + // second editor. Without this limit everything will get + // blocked indefinitely. How could this be fixed? + for (int i = 0; + i < 20 && PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE); i++) { + TranslateMessage(&msg); + DispatchMessage(&msg); + } + } + + async_handle_events(); + }); +} + void GroupBridge::async_log_pipe_lines( boost::asio::posix::stream_descriptor& pipe, boost::asio::streambuf& buffer, diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index 87fd9d07..ae610964 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -183,6 +183,13 @@ class GroupBridge { */ void accept_requests(); + /** + * Handle both Win32 messages and X11 events on a timer within the IO + * context. This is a centralized replacement for the event handling in + * `Vst2Bridge::handle_dispatch_single` for plugin groups. + */ + void async_handle_events(); + /** * Continuously read from a pipe and write the output to the log file. Used * with the IO streams captured by `stdout_redirect` and `stderr_redirect`. @@ -268,6 +275,14 @@ class GroupBridge { */ std::mutex active_plugins_mutex; + /** + * A timer used to repeatedly handle the Win32 message loop and the X11 + * events. + * + 8 @see async_handle_events + */ + boost::asio::steady_timer events_timer; + /** * A timer to defer shutting down the process, allowing for fast plugin * scanning without having to start a new group host process for each diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 7ebd852b..b80688f3 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -16,9 +16,13 @@ #include "vst2.h" +#include #include #include +#include "../../common/communication.h" +#include "../../common/events.h" + /** * A function pointer to what should be the entry point of a VST plugin. */ @@ -156,12 +160,7 @@ void Vst2Bridge::handle_dispatch_single() { plugin, std::bind(&Vst2Bridge::dispatch_wrapper, this, _1, _2, _3, _4, _5, _6))); - // Don't run them message loop during the two step process of - // opening the plugin editor since some plugins don't expect this - if (!should_skip_message_loop()) { - handle_win32_events(); - } - + handle_win32_events(); handle_x11_events(); } } catch (const boost::system::system_error&) { @@ -170,6 +169,38 @@ void Vst2Bridge::handle_dispatch_single() { } } +void Vst2Bridge::handle_dispatch_multi(boost::asio::io_context& main_context) { + // This works exactly the same as the function above, but execute the + // actual event and run the message loop from the main thread that's + // also instantiating these plugins. This is required for a few plugins + // to run multiple instances in the same process + try { + while (true) { + receive_event( + host_vst_dispatch, std::nullopt, + passthrough_event( + plugin, + [&](AEffect* plugin, int opcode, int index, intptr_t value, + void* data, float option) -> intptr_t { + std::promise dispatch_result; + boost::asio::dispatch(main_context, [&]() { + const intptr_t result = dispatch_wrapper( + plugin, opcode, index, value, data, option); + + dispatch_result.set_value(result); + }); + + // The message loop and X11 event handling will be run + // separately on a timer + return dispatch_result.get_future().get(); + })); + } + } catch (const boost::system::system_error&) { + // The plugin has cut off communications, so we can shut down this + // host application + } +} + void Vst2Bridge::handle_dispatch_midi_events() { try { while (true) { @@ -381,6 +412,12 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, } void Vst2Bridge::handle_win32_events() { + // Don't run them message loop during the two step process of opening the + // plugin editor since some plugins don't expect this + if (should_skip_message_loop()) { + return; + } + if (editor.has_value()) { editor->handle_win32_events(); } else { diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index 4a35db09..5abfd252 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -26,13 +26,9 @@ #include #include -#include #include -#include #include -#include "../../common/communication.h" -#include "../../common/events.h" #include "../../common/logging.h" #include "../editor.h" #include "../utils.h" @@ -101,50 +97,20 @@ class Vst2Bridge { * * @param main_context The main IO context that's handling the event * handling for all plugins. - * @param message_loop_blocked A function that returns true if the message - * loop is blocked. This is used to temporarily postpone running the - * message loop while a plugin is opening its GUI. * * @note With this approach you'll have to make sure that the object was * instantiated from the same thread as the one that runs the IO context. + * @note This appraoch does _not_ handle any events. This has to be done on + * a timer within the IO context since otherwise things would become very + * messy very quick. */ - template - void handle_dispatch_multi(boost::asio::io_context& main_context, - const F& message_loop_blocked) { - // This works exactly the same as the function above, but execute the - // actual event and run the message loop from the main thread that's - // also instantiating these plugins. This is required for a few plugins - // to run multiple instances in the same process - try { - while (true) { - receive_event( - host_vst_dispatch, std::nullopt, - passthrough_event( - plugin, - [&](AEffect* plugin, int opcode, int index, - intptr_t value, void* data, - float option) -> intptr_t { - std::promise dispatch_result; - boost::asio::dispatch(main_context, [&]() { - const intptr_t result = dispatch_wrapper( - plugin, opcode, index, value, data, option); + void handle_dispatch_multi(boost::asio::io_context& main_context); - dispatch_result.set_value(result); - if (!message_loop_blocked()) { - handle_win32_events(); - } - - handle_x11_events(); - }); - - return dispatch_result.get_future().get(); - })); - } - } catch (const boost::system::system_error&) { - // The plugin has cut off communications, so we can shut down this - // host application - } - } + /** + * Handle X11 events for the editor window if it is open. This can be run + * safely from any thread. + */ + void handle_x11_events(); // These functions are the entry points for the `*_handler` threads // defined below. They're defined here because we can't use lambdas with @@ -197,12 +163,6 @@ class Vst2Bridge { */ void handle_win32_events(); - /** - * Handle X11 events for the editor window if it is open. This can be run - * safely from any thread. - */ - void handle_x11_events(); - /** * The shared library handle of the VST plugin. I sadly could not get * Boost.DLL to work here, so we'll just load the VST plugisn by hand. From 9b847fdc31e6b13a78a80c3869cb997ccdbfd3b9 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 26 May 2020 12:19:37 +0200 Subject: [PATCH 63/73] Add todo regarding offscreen window coordinates --- src/wine-host/editor.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 955ffe72..355dbbec 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -196,8 +196,9 @@ void Editor::handle_x11_events() { // We can't directly use the `event.x` and `event.y` coordinates // because the parent window may also be embedded inside another // window. - // TODO: With plugin groups this has to be done any time the - // mouse cursor enters the window on a FOCUS_IN event + // TODO: This seems to get clamped at (0, 0), causing large + // windows dragged off screen on the top or the left + // borders to act up const auto translate_cookie = xcb_translate_coordinates( x11_connection.get(), parent_window, root, 0, 0); const xcb_translate_coordinates_reply_t* From fc35b6c9c85dc824ab702ce4c62d1e5c4f155ba6 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 26 May 2020 17:07:14 +0200 Subject: [PATCH 64/73] Show the init message before launching VST host --- src/plugin/plugin-bridge.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index c8734516..e6816f3e 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -76,8 +76,8 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) wine_version(get_wine_version()), wine_stdout(io_context), wine_stderr(io_context) { - launch_vst_host(); log_init_message(); + launch_vst_host(); // Print the Wine host's STDOUT and STDERR streams to the log file. This // should be done before trying to accept the sockets as otherwise we will From 0c047f9a66a713bc6d9c860343f833233bb8778c Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 26 May 2020 17:09:09 +0200 Subject: [PATCH 65/73] Work around a memory corruption issue on unload I'm really not sure what is happening here, and it might just be a winelib thing. --- src/wine-host/bridges/vst2.cpp | 10 ++++++---- src/wine-host/bridges/vst2.h | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index b80688f3..a52683de 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -67,7 +67,10 @@ Vst2Bridge& get_bridge_instance(const AEffect* plugin) { Vst2Bridge::Vst2Bridge(std::string plugin_dll_path, std::string socket_endpoint_path) - : plugin_handle(LoadLibrary(plugin_dll_path.c_str()), FreeLibrary), + // See `plugin_handle`s docstring for information on why we're leaking + // memory here + // : plugin_handle(LoadLibrary(plugin_dll_path.c_str()), FreeLibrary), + : plugin_handle(LoadLibrary(plugin_dll_path.c_str())), io_context(), socket_endpoint(socket_endpoint_path), host_vst_dispatch(io_context), @@ -85,9 +88,8 @@ Vst2Bridge::Vst2Bridge(std::string plugin_dll_path, // there are some older deprecated names that legacy plugins may still use VstEntryPoint vst_entry_point = nullptr; for (auto name : {"VSTPluginMain", "main_plugin", "main"}) { - vst_entry_point = - reinterpret_cast(reinterpret_cast( - GetProcAddress(plugin_handle.get(), name))); + vst_entry_point = reinterpret_cast( + reinterpret_cast(GetProcAddress(plugin_handle, name))); if (vst_entry_point != nullptr) { break; diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index 5abfd252..0622c891 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -165,10 +165,19 @@ class Vst2Bridge { /** * The shared library handle of the VST plugin. I sadly could not get - * Boost.DLL to work here, so we'll just load the VST plugisn by hand. + * Boost.DLL to work here, so we'll just load the VST plugins by hand. + * + * FIXME: I don't know why, but `FreeLibrary()` seems to corrupt memory. + * This leads to a lot of weird behavior, such as plugins crashing as + * soon as other plugins get loaded or calls to `LoadLibrary()` + * returning null pointers while they would otherwise load fine + * without the prior call to `FreeLibrary`. We are leaking memory + * here until this is fixed, but it should not be a huge issue since + * this leak only exists for plugin groups. */ - std::unique_ptr, decltype(&FreeLibrary)> - plugin_handle; + // std::unique_ptr, decltype(&FreeLibrary)> + // plugin_handle; + HMODULE plugin_handle; /** * The loaded plugin's `AEffect` struct, obtained using the above library From be969a69d0b8752a1d32ac8b0dfab386827eca26 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 26 May 2020 19:39:35 +0200 Subject: [PATCH 66/73] Document plugin groups --- README.md | 59 ++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 4136663d..fc31b17d 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,58 @@ handle it accordingly. ### Plugin groups -TODO: Document +Some plugins have the ability to communicate with other instances of that same +plugin or with other plugins made by the same manufacturer. This is often used +in mixing plugins to allow different tracks to reference each other without +having to route audio between them. Examples of plugins that do this are +Fabfilter Pro-Q 3, MMultiAnalyzer and the iZotope mixing plugins. For this to +work, all instances of a particular plugin have to be hosted in the same +process. + +Yabridge has the concept of _plugin groups_, which are user defined groups of +plugins that will all be hosted in the same process. These plugins groups can be +configured using a `yabridge.toml` file located either in the same directory as +the symlink of or copy to `libyabridge.so`, or in any directories above it. This +file contains case sensitive +[glob](https://www.man7.org/linux/man-pages/man7/glob.7.html) patterns that are +used to match the names of `*.so` files relative to that `yabridge.toml` file. +These patterns can also match an entire directory. For simplicity's sake only +the first `yabridge.toml` file found and only the first glob pattern matched +within that file are considered. An example `yabridge.toml` file looks like +this: + +```toml +# ~/.wine/drive_c/Program Files/Steinberg/VstPlugins/yabridge.toml + +["FabFilter Pro-Q 3.so"] +group = "fabfilter" + +["MeldaProduction/Tools/MMultiAnalyzer.so"] +group = "melda" + +# Matches an entire directory and all files inside it. Make sure to not include +# a trailing slash. +["ToneBoosters"] +group = "toneboosters" + +["iZotope*/Neutron *"] +group = "izotope" + +["iZotope7/Insight 2.so"] +group = "izotope" + +# This won't do anything, since the pattern above has already matched this file +["iZotope7/Neutron 2 Mix Tap.so"] +group = "This will be ignored!" + +# Don't do this! This matches all plugins in this directory and all of its +# subdirectories, causing all of them to be hosted in a single process. While +# this would increase startup performance considerably, it will also break any +# form of individual plugin sandboxing provided by the host and could +# potentially introduce all kinds of weird issues. +# ["*"] +# group = "all" +``` ### Wine prefixes @@ -192,12 +243,6 @@ Aside from that, these are some known caveats: - Most recent **iZotope** plugins don't have a functional GUI in a typical out of the box Wine setup because of missing dependencies. Please let me know if you know which dependencies are needed for these plugins to render correctly. -- Some plugins, such as **Fabfilter Pro-Q 3**, are able to communicate between - different instances of the same plugin by relying on the fact that they're all - loaded into the same process. Right now this is something that yabridge does - not do as it would break any form of sandboxing, meaning that if one plugin - were to crash, all other plugins would go down with it. If this is something - you need for your workflow, please let me know. There are also some VST2.X extension features that have not been implemented yet because I haven't needed them myself. Let me know if you need any of these From ab4d35886e8ad76026c8fbbbf7e7a033e0f505ad Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 26 May 2020 18:49:44 +0200 Subject: [PATCH 67/73] Add a fix for the keyboard focus in Bitwig 3.2 --- CHANGELOG.md | 2 + src/wine-host/editor.cpp | 120 ++++++++++++++++++++++----------------- src/wine-host/editor.h | 7 +++ 3 files changed, 78 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49927f68..5f15c613 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed +- Steal keyboard focus when clicking on the plugin editor window to account for + the new keyboard focus behavior in Bitwig Studio 3.2. - Fixed large amount of empty lines in the log file when the Wine process closes unexpectedly. - Made the plugin and host detection slightly more robust. diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 355dbbec..6d8483b0 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -100,6 +100,10 @@ Editor::Editor(const std::string& window_class_name, xcb_change_window_attributes(x11_connection.get(), topmost_window, XCB_CW_EVENT_MASK, &topmost_event_mask); xcb_flush(x11_connection.get()); + const uint32_t parent_event_mask = XCB_EVENT_MASK_FOCUS_CHANGE; + xcb_change_window_attributes(x11_connection.get(), parent_window, + XCB_CW_EVENT_MASK, &parent_event_mask); + xcb_flush(x11_connection.get()); // Embed the Win32 window into the window provided by the host. Instead of // using the XEmbed protocol, we'll register a few events and manage the @@ -172,65 +176,79 @@ void Editor::handle_x11_events() { // before the root window, i.e. the window that's actually going to // get dragged around the by the user. In most cases this is the // same as `parent_window`. - case XCB_CONFIGURE_NOTIFY: { - // We're purposely not using XEmbed. This has the consequence - // that wine still thinks that any X and Y coordinates are - // relative to the x11 window root instead of the parent window - // provided by the DAW, causing all sorts of GUI interactions to - // break. To alleviate this we'll just lie to Wine and tell it - // that it's located at the parent window's location on the root - // window. We also will keep the child window at its largest - // possible size to allow for smooth resizing. This works - // because the embedding hierarchy is DAW window -> Win32 window - // (created in this class) -> VST plugin window created by the - // plugin itself. In this case it doesn't matter that the Win32 - // window is larger than the part of the client area the plugin - // draws to since any excess will be clipped off by the parent - // window. - const auto query_cookie = - xcb_query_tree(x11_connection.get(), parent_window); - xcb_window_t root = xcb_query_tree_reply(x11_connection.get(), - query_cookie, nullptr) - ->root; + case XCB_CONFIGURE_NOTIFY: + fix_window_coordinates(); + break; + // The coordinates should also be reset when the user clicks on the + // window. This is sometimes necessary when opening multiple editors + // in a single plugin group. + case XCB_FOCUS_IN: + fix_window_coordinates(); - // We can't directly use the `event.x` and `event.y` coordinates - // because the parent window may also be embedded inside another - // window. - // TODO: This seems to get clamped at (0, 0), causing large - // windows dragged off screen on the top or the left - // borders to act up - const auto translate_cookie = xcb_translate_coordinates( - x11_connection.get(), parent_window, root, 0, 0); - const xcb_translate_coordinates_reply_t* - translated_coordiantes = xcb_translate_coordinates_reply( - x11_connection.get(), translate_cookie, nullptr); - - xcb_configure_notify_event_t translated_event{}; - translated_event.response_type = XCB_CONFIGURE_NOTIFY; - translated_event.event = child_window; - translated_event.window = child_window; - // This should be set to the same sizes the window was created - // on. Since we're not using `SetWindowPos` to resize the - // Window, Wine can get a bit confused when we suddenly report a - // different client area size. Without this certain plugins - // (such as those by Valhalla DSP) would break. - translated_event.width = client_area.width; - translated_event.height = client_area.height; - translated_event.x = translated_coordiantes->dst_x; - translated_event.y = translated_coordiantes->dst_y; - - xcb_send_event(x11_connection.get(), false, child_window, - XCB_EVENT_MASK_STRUCTURE_NOTIFY | - XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY, - reinterpret_cast(&translated_event)); + // Explicitely request input focus when the user clicks on the + // window. This is needed for Bitwig Studio 3.2, as the parent + // window now captures all keyboard events and forwards them to + // the main Bitwig Studio window instead of allowing the child + // window to handle those events. + xcb_set_input_focus(x11_connection.get(), + XCB_INPUT_FOCUS_PARENT, child_window, + XCB_CURRENT_TIME); xcb_flush(x11_connection.get()); - } break; + + break; } free(generic_event); } } +void Editor::fix_window_coordinates() { + // We're purposely not using XEmbed. This has the consequence that wine + // still thinks that any X and Y coordinates are relative to the x11 window + // root instead of the parent window provided by the DAW, causing all sorts + // of GUI interactions to break. To alleviate this we'll just lie to Wine + // and tell it that it's located at the parent window's location on the root + // window. We also will keep the child window at its largest possible size + // to allow for smooth resizing. This works because the embedding hierarchy + // is DAW window -> Win32 window (created in this class) -> VST plugin + // window created by the plugin itself. In this case it doesn't matter that + // the Win32 window is larger than the part of the client area the plugin + // draws to since any excess will be clipped off by the parent window. + const auto query_cookie = + xcb_query_tree(x11_connection.get(), parent_window); + xcb_window_t root = + xcb_query_tree_reply(x11_connection.get(), query_cookie, nullptr)->root; + + // We can't directly use the `event.x` and `event.y` coordinates because the + // parent window may also be embedded inside another window. + // TODO: This seems to get clamped at (0, 0), causing large windows dragged + // off screen on the top or the left borders to act up + const auto translate_cookie = xcb_translate_coordinates( + x11_connection.get(), parent_window, root, 0, 0); + const xcb_translate_coordinates_reply_t* translated_coordiantes = + xcb_translate_coordinates_reply(x11_connection.get(), translate_cookie, + nullptr); + + xcb_configure_notify_event_t translated_event{}; + translated_event.response_type = XCB_CONFIGURE_NOTIFY; + translated_event.event = child_window; + translated_event.window = child_window; + // This should be set to the same sizes the window was created on. Since + // we're not using `SetWindowPos` to resize the Window, Wine can get a bit + // confused when we suddenly report a different client area size. Without + // this certain plugins (such as those by Valhalla DSP) would break. + translated_event.width = client_area.width; + translated_event.height = client_area.height; + translated_event.x = translated_coordiantes->dst_x; + translated_event.y = translated_coordiantes->dst_y; + + xcb_send_event( + x11_connection.get(), false, child_window, + XCB_EVENT_MASK_STRUCTURE_NOTIFY | XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY, + reinterpret_cast(&translated_event)); + xcb_flush(x11_connection.get()); +} + LRESULT CALLBACK window_proc(HWND handle, UINT message, WPARAM wParam, diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index a58eb70e..a2c5773f 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -113,6 +113,13 @@ class Editor { void handle_x11_events(); private: + /** + * Lie to the Wine window about its coordinates on the screen for + * reparenting without using XEmbed. See the comment at the top of the + * implementation on why this is needed. + */ + void fix_window_coordinates(); + /** * A pointer to the currently active window. Will be a null pointer if no * window is active. From d65281d691783e456e75790c42e25299d21cb6f2 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 27 May 2020 13:50:52 +0200 Subject: [PATCH 68/73] Clarify local coordinate fix function name --- src/wine-host/editor.cpp | 6 +++--- src/wine-host/editor.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 6d8483b0..37b4386c 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -177,13 +177,13 @@ void Editor::handle_x11_events() { // get dragged around the by the user. In most cases this is the // same as `parent_window`. case XCB_CONFIGURE_NOTIFY: - fix_window_coordinates(); + fox_local_coordinates(); break; // The coordinates should also be reset when the user clicks on the // window. This is sometimes necessary when opening multiple editors // in a single plugin group. case XCB_FOCUS_IN: - fix_window_coordinates(); + fox_local_coordinates(); // Explicitely request input focus when the user clicks on the // window. This is needed for Bitwig Studio 3.2, as the parent @@ -202,7 +202,7 @@ void Editor::handle_x11_events() { } } -void Editor::fix_window_coordinates() { +void Editor::fox_local_coordinates() { // We're purposely not using XEmbed. This has the consequence that wine // still thinks that any X and Y coordinates are relative to the x11 window // root instead of the parent window provided by the DAW, causing all sorts diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index a2c5773f..9cde4ea9 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -118,7 +118,7 @@ class Editor { * reparenting without using XEmbed. See the comment at the top of the * implementation on why this is needed. */ - void fix_window_coordinates(); + void fox_local_coordinates(); /** * A pointer to the currently active window. Will be a null pointer if no From 5a0c3c4627d4af4429a8a7f3f3e861ef1f1287af Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 27 May 2020 14:05:57 +0200 Subject: [PATCH 69/73] Simplify the opening editor behaviour again This partially reverts 16fce5577d30ddb09769aa163f58c66c0025b7ea. --- src/wine-host/bridges/vst2.cpp | 96 ++++++++++++++++------------------ src/wine-host/bridges/vst2.h | 39 ++++++++++---- 2 files changed, 73 insertions(+), 62 deletions(-) diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index a52683de..726b1a9c 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -146,7 +146,7 @@ Vst2Bridge::Vst2Bridge(std::string plugin_dll_path, } bool Vst2Bridge::should_skip_message_loop() { - return editor_is_opening; + return std::holds_alternative(editor); } void Vst2Bridge::handle_dispatch_single() { @@ -362,27 +362,18 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, case effEditGetRect: { // Some plugins will have a race condition if the message loops gets // handled between the call to `effEditGetRect()` and - // `effEditOpen()`, since this won't ever happen on Windows and - // plugins thus assume that this can't happen at all. If - // `effEditOpen()` has not yet been called, then we'll mark the - // editor as currently opening to prevent the message loop from - // running. - editor_is_opening = !editor.has_value(); + // `effEditOpen()`, although this behavior never appears on Windows + // as hosts will always either call these functions in sequence or + // in reverse. We need to temporarily stop handling messages when + // this happens. + if (!std::holds_alternative(editor)) { + editor = EditorOpening{}; + } return plugin->dispatcher(plugin, opcode, index, value, data, option); } break; case effEditOpen: { - // As explained above, if `effEditGetRect()` was called first then - // after this the editor has finally been opened. Otherwise if this - // function was first then we'll say that the editor is in the - // process of being opened as the host will call `effEditGetRect()` - // next. - // TODO: Without plugin groups only the `effEditGetRect()` -> - // `effEditOpen()`, is skipping the message loop in between - // `effEditOpen()` and `effEditGetRect()` really needed? - editor_is_opening = !editor_is_opening; - // Create a Win32 window through Wine, embed it into the window // provided by the host, and let the plugin embed itself into // the Wine window @@ -392,17 +383,19 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, // should get a unique window class const std::string window_class = "yabridge plugin " + socket_endpoint.path(); + Editor& editor_instance = + editor.emplace(window_class, plugin, x11_handle); - editor.emplace(window_class, plugin, x11_handle); return plugin->dispatcher(plugin, opcode, index, value, - editor->win32_handle.get(), option); + editor_instance.win32_handle.get(), + option); } break; case effEditClose: { const intptr_t return_value = plugin->dispatcher(plugin, opcode, index, value, data, option); // Cleanup is handled through RAII - editor.reset(); + editor = std::monostate(); return return_value; } break; @@ -414,28 +407,28 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, } void Vst2Bridge::handle_win32_events() { - // Don't run them message loop during the two step process of opening the - // plugin editor since some plugins don't expect this - if (should_skip_message_loop()) { - return; - } + std::visit( + overload{[](Editor& editor) { editor.handle_win32_events(); }, + [](std::monostate&) { + MSG msg; - if (editor.has_value()) { - editor->handle_win32_events(); - } else { - MSG msg; - - while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) { - TranslateMessage(&msg); - DispatchMessage(&msg); - } - } + while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessage(&msg); + } + }, + [](EditorOpening&) { + // Don't handle any events in this + // particular case as explained in + // `Vst2Bridge::editor` + }}, + editor); } void Vst2Bridge::handle_x11_events() { - if (editor.has_value()) { - editor->handle_x11_events(); - } + std::visit(overload{[](Editor& editor) { editor.handle_x11_events(); }, + [](auto&) {}}, + editor); } class HostCallbackDataConverter : DefaultDataConverter { @@ -453,17 +446,18 @@ class HostCallbackDataConverter : DefaultDataConverter { return WantsVstTimeInfo{}; break; case audioMasterIOChanged: - // This is a helpful event that indicates that the VST plugin's - // `AEffect` struct has changed. Writing these results back is - // done inside of `passthrough_event`. + // This is a helpful event that indicates that the VST + // plugin's `AEffect` struct has changed. Writing these + // results back is done inside of `passthrough_event`. return AEffect(*plugin); break; case audioMasterProcessEvents: return DynamicVstEvents(*static_cast(data)); break; - // We detect whether an opcode should return a string by checking - // whether there's a zeroed out buffer behind the void pointer. This - // works for any host, but not all plugins zero out their buffers. + // We detect whether an opcode should return a string by + // checking whether there's a zeroed out buffer behind the void + // pointer. This works for any host, but not all plugins zero + // out their buffers. case audioMasterGetVendorString: case audioMasterGetProductString: return WantsString{}; @@ -482,11 +476,11 @@ class HostCallbackDataConverter : DefaultDataConverter { void write(const int opcode, void* data, const EventResult& response) { switch (opcode) { case audioMasterGetTime: - // Write the returned `VstTimeInfo` struct into a field and make - // the function return a poitner to it in the function below. - // Depending on whether the host supported the requested time - // information this operations returns either a null pointer or - // a pointer to a `VstTimeInfo` object. + // Write the returned `VstTimeInfo` struct into a field and + // make the function return a poitner to it in the function + // below. Depending on whether the host supported the + // requested time information this operations returns either + // a null pointer or a pointer to a `VstTimeInfo` object. if (std::holds_alternative(response.payload)) { time_info = std::nullopt; } else { @@ -502,8 +496,8 @@ class HostCallbackDataConverter : DefaultDataConverter { intptr_t return_value(const int opcode, const intptr_t original) { switch (opcode) { case audioMasterGetTime: { - // Return a pointer to the `VstTimeInfo` object written in the - // function above + // Return a pointer to the `VstTimeInfo` object written in + // the function above VstTimeInfo* time_info_pointer = nullptr; if (time_info.has_value()) { time_info_pointer = &time_info.value(); diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index 0622c891..5bd1fc93 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -33,6 +33,13 @@ #include "../editor.h" #include "../utils.h" +/** + * A marker struct to indicate that the editor is about to be opened. + * + * @see Vst2Bridge::editor + */ +struct EditorOpening {}; + /** * This hosts a Windows VST2 plugin, forwards messages sent by the Linux VST * plugin and provides host callback function for the plugin to talk back. @@ -75,7 +82,9 @@ class Vst2Bridge { * but in our approach there will be an opportunity to handle events in * between these two calls. Most plugins will handle this just fine, but * some plugins end up blocking indefinitely while waiting for the other - * function to be called, hence why this function is needed. + * function to be called, hence why this function is needed. For + * individually hosted plugins this check is done implicitely in + * `Vst2Bridge::handle_win32_events()`. */ bool should_skip_message_loop(); @@ -261,17 +270,25 @@ class Vst2Bridge { * Wine window, and embedding that Wine window into a window provided by the * host. Should be empty when the editor is not open. * - * There is some special behavior with regards to message handling when the - * editor is in the process of being opened, see - * `should_postpone_message_loop()`. + * This field can have three possible states: + * + * - `std::nullopt` when the editor is closed. + * - An `Editor` object when the editor is open. + * - `EditorOpening` when the editor is not yet open, but the host has + * already called `effEditGetRect()` and is about to call `effEditOpen()`. + * This is needed because there is a race condition in some bugs that + * cause them to crash or enter an infinite Win32 message loop when + * `effEditGetRect()` gets dispatched and we then enter the message loop + * loop before `effEditOpen()` gets called. Most plugins will handle this + * just fine, but a select few plugins make the assumption that the editor + * is already open once `effEditGetRect()` has been called, even if + * `effEditOpen` has not yet been dispatched. VST hsots on Windows will + * call these two events in sequence, so the bug would never occur there. + * To work around this we'll use this third state to temporarily stop + * processing Windows events in the one or two ticks between these two + * events. * * @see should_postpone_message_loop */ - std::optional editor; - - /** - * Keeps track of when the editor is being opened during the two-phase - * process of the host calling `effEditOpen()` and `effEditGetRect()`. - */ - bool editor_is_opening = false; + std::variant editor; }; From 276e4ac02f168cc61d333b38b4cf87e0338fce1f Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 27 May 2020 14:29:31 +0200 Subject: [PATCH 70/73] Make the local coordinate fix more robust Without this fix and when using plugin groups, hovering over an opened window that has not yet been interacted with shows some weird behavior. --- src/wine-host/editor.cpp | 38 ++++++++++++++++++++++---------------- src/wine-host/editor.h | 2 +- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 37b4386c..1e329355 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -92,15 +92,20 @@ Editor::Editor(const std::string& window_class_name, // TODO: Add a `KillTimer()` now that we are hosting multiple plugins SetTimer(win32_handle.get(), idle_timer_id, 100, nullptr); - // We need to tell the Wine window it has been moved whenever the window - // it's been embedded in gets moved around. In most cases this is - // `parent_window`, but for instance REAPER reparents `parent_window` in - // another window so we'll have to find the correct window first. + // Because we're not using XEmbed Wine will interpret any local coordinates + // as global coordinates. To work around this we'll tell the Wine window + // it's located at its actual coordinates on screen rather than somewhere + // within. For robustness's sake this should be done both when the actual + // window the Wine window is embedded in (which may not be the parent + // window) is moved or resized, and when the user moves his mouse over the + // window. We also want to set keyboard focus when the user clicks on the + // Windows since Bitwig 3.2 now explicitely requires this. const uint32_t topmost_event_mask = XCB_EVENT_MASK_STRUCTURE_NOTIFY; xcb_change_window_attributes(x11_connection.get(), topmost_window, XCB_CW_EVENT_MASK, &topmost_event_mask); xcb_flush(x11_connection.get()); - const uint32_t parent_event_mask = XCB_EVENT_MASK_FOCUS_CHANGE; + const uint32_t parent_event_mask = + XCB_EVENT_MASK_FOCUS_CHANGE | XCB_EVENT_MASK_ENTER_WINDOW; xcb_change_window_attributes(x11_connection.get(), parent_window, XCB_CW_EVENT_MASK, &parent_event_mask); xcb_flush(x11_connection.get()); @@ -172,18 +177,20 @@ void Editor::handle_x11_events() { while ((generic_event = xcb_poll_for_event(x11_connection.get())) != nullptr) { switch (generic_event->response_type & event_type_mask) { - // We're listening for ConfigureNotify events on the topmost window - // before the root window, i.e. the window that's actually going to - // get dragged around the by the user. In most cases this is the - // same as `parent_window`. + // We're listening for `ConfigureNotify` events on the topmost + // window before the root window, i.e. the window that's actually + // going to get dragged around the by the user. In most cases this + // is the same as `parent_window`. When either this window gets + // moved, or when the user moves his mouse over our window, the + // local coordinates should be updated. The additional `EnterWindow` + // check is sometimes necessary for using multiple editor windows + // within a single plugin group. case XCB_CONFIGURE_NOTIFY: - fox_local_coordinates(); + case XCB_ENTER_NOTIFY: + fix_local_coordinates(); break; - // The coordinates should also be reset when the user clicks on the - // window. This is sometimes necessary when opening multiple editors - // in a single plugin group. case XCB_FOCUS_IN: - fox_local_coordinates(); + fix_local_coordinates(); // Explicitely request input focus when the user clicks on the // window. This is needed for Bitwig Studio 3.2, as the parent @@ -194,7 +201,6 @@ void Editor::handle_x11_events() { XCB_INPUT_FOCUS_PARENT, child_window, XCB_CURRENT_TIME); xcb_flush(x11_connection.get()); - break; } @@ -202,7 +208,7 @@ void Editor::handle_x11_events() { } } -void Editor::fox_local_coordinates() { +void Editor::fix_local_coordinates() { // We're purposely not using XEmbed. This has the consequence that wine // still thinks that any X and Y coordinates are relative to the x11 window // root instead of the parent window provided by the DAW, causing all sorts diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 9cde4ea9..99c083e1 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -118,7 +118,7 @@ class Editor { * reparenting without using XEmbed. See the comment at the top of the * implementation on why this is needed. */ - void fox_local_coordinates(); + void fix_local_coordinates(); /** * A pointer to the currently active window. Will be a null pointer if no From 941f915dfe525a2648972a018e1ab90008e53cd8 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 27 May 2020 15:24:54 +0200 Subject: [PATCH 71/73] Move the architecture section to docs/ It's getting a bit unwieldy to be left in the readme. --- README.md | 123 ------------------------------------------- docs/architecture.md | 122 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 123 deletions(-) create mode 100644 docs/architecture.md diff --git a/README.md b/README.md index fc31b17d..f42f4437 100644 --- a/README.md +++ b/README.md @@ -359,126 +359,3 @@ modifications in `src/plugin/plugin-bridge.cpp`. To enable this, simply run: ```shell meson configure build --buildtype=debug -Duse-winedbg=true ``` - -## Architecture - -The project consists of two components: a Linux native VST plugin -(`libyabridge.so`) and a VST host that runs under Wine -(`yabridge-host.exe`/`yabridge-host.exe.so`, and -`yabridge-host-32.exe`/`yabridge-host-32.exe.so` if the bitbirdge is enabled). -I'll refer to the copy of or the symlink to `libyabridge.so` as _the plugin_, -the native Linux VST host that's hosting the plugin as _the native VST host_, -the Wine VST host application that's hosting a Windows `.dll` file as _the Wine -VST host_, and the Windows VST plugin that's being loaded in the Wine VST host -as the _Windows VST plugin_. The whole process works as follows: - -1. Some copy of or a symlink to `libyabridge.so` gets loaded as a VST plugin in - a Linux VST host. This file should have been renamed to match a Windows VST - plugin `.dll` file in the same directory. For instance, if there's a - `Serum_x64.dll` file you'd like to bridge, then there should be a symlink to - `libyabridge.so` named `Serum_x64.so`. -2. The plugin first attempts to locate and determine: - - - The Windows VST plugin `.dll` file that should be loaded. - - - The architecture of that VST plugin file. This is done by inspecting the - headers if the `.dll` file. - - - The location of the Wine VST host. This will depend on the architecture - detected for the plugin. If the plugin was compiled for the `x86_64` - architecture or the 'Any CPU' target, then we will look for - `yabridge-host.exe`. If the plugin was compiled for the `x86` architecture, - when we'll search for `yabridge-host-32.exe`. - - We will first search for this file alongside the actual location of - `libyabridge.so`. This is useful for development, as it allows you to use a - symlink to `libyabridge.so` directly from the build directory causing - yabridge to automatically pick up the right version of the Wine VST host. - If this file cannot be found, then it will fall back to searching through - the search path. - - - The Wine prefix the plugin is located in. If the `WINEPREFIX` environment - variable is specified, then that will be used instead. - -3. The plugin then sets up a Unix domain socket endpoint to communicate with the - Wine VST host somewhere in a temporary directory and starts listening on it. - I chose to communicate over Unix domain sockets rather than using shared - memory directly because this way you get low latency communication with - without any busy waits or manual synchronisation for free. The added benefit - is that it also makes it possible to send arbitrarily large chunks of data - without having to split it up first. This is useful for transmitting audio - and preset data which may have any arbitrary size. -4. The plugin launches the Wine VST host in the detected wine prefix, passing - the name of the `.dll` file it should be loading and the path to the Unix - domain socket that was just created as its arguments. -5. Communication gets set up using multiple sockets over the end point created - previously. This allows us to easily handle multiple data streams from - different threads using blocking read operations for synchronization. Doing - this greatly simplifies the way communication works without compromising on - latency. The following types of events each get their own socket: - - - Calls from the native VST host to the plugin's `dispatcher()` function. - These get forwarded to the Windows VST plugin through the Wine VST host. - - Calls from the native VST host to the plugin's `dispatcher()` function with - the `effProcessEvents` opcode. These also get forwarded to the Windows VST - plugin through the Wine VST host. This has to be handled separately from - all other events because of limitations of the Win32 API. Without doing - this the plugin would not be able to receive any MIDI events while the GUI - is being resized or a dropdown menu or message box is shown. - - Host callback calls from the Windows VST plugin through the - `audioMasterCallback` function. These get forwarded to the native VST host - through the plugin. - - Both the `dispatcher()` and `audioMasterCallback()` functions are handled - in the same way, with some minor variations on how payload data gets - serialized depending on the opcode of the event being sent. See the section - below this for more details on this procedure. - - - Calls from the native VST host to the plugin's `getParameter()` and - `setParameter()` functions. Both functions get forwarded to the Windows VST - plugin through the Wine VST host using a single socket because they're very - similar and don't need any complicated behaviour. - - Calls from the native VST host to the plugin's `processReplacing()` - function. This function gets forwarded to the Windows VST plugin through - the Wine VST. In the rare event that the plugin does not support - `processReplacing()` and only supports The deprecated commutative - `process()` function, then the Wine VST host will emulate the behavior of - `processReplacing()` instead. - - The operations described above involving the host -> plugin `dispatcher()`and - plugin -> host `audioMaster()` functions are all handled by first serializing - the function parameters and any payload data into a binary format so they can - be sent over a socket. The objects used for encoding both the requests and - the responses for theses events can be found in `src/common/serialization.h`, - and the functions that actually read and write these objects over the sockets - are located in `src/common/communication.h`. The actual binary serialization - is handled using [bitsery](https://github.com/fraillt/bitsery). - - Actually sending and receiving the events happens in the `send_event()` and - `receive_event()` functions. When calling either `dispatch()` or - `audioMaster()`, the caller will oftentimes either pass along some kind of - data structure through the void pointer function argument, or they expect the - function's return value to be a pointer to some kind of struct provided by - the plugin or host. The behaviour for reading from and writing into these - void pointers and returning pointers to objects when needed is encapsulated - in the `DispatchDataConverter` and `HostCallbackDataCovnerter` classes for - the `dispatcher()` and `audioMaster()` functions respectively. For operations - involving the plugin editor there is also some extra glue in - `Vst2Bridge::dispatch_wrapper`. On the receiving end of the function calls, - the `passthrough_event()` function which calls the callback functions and - handles the marshalling between our data types created by the - `*DataConverter` classes and the VST API's different pointer types. This - behaviour is separated from `receive_event()` so we can handle MIDI events - separately. This is needed because a select few plugins only store pointers - to the received events rather than copies of the objects. Because of this, - the received event data must live at least until the next audio buffer gets - processed so it needs to be stored temporarily. - -6. The Wine VST host loads the Windows VST plugin and starts forwarding messages - over the sockets described above. -7. After the Windows VST plugin has started loading we will forward all values - from the plugin's `AEffect` struct to the Linux native VST plugin over the - `dispatcher()` socket. This is only done once at startup. After this point - the plugin will stop blocking and has finished loading. - -TODO: Document plugin groups diff --git a/docs/architecture.md b/docs/architecture.md new file mode 100644 index 00000000..c265b0a1 --- /dev/null +++ b/docs/architecture.md @@ -0,0 +1,122 @@ +# Architecture + +The project consists of two components: a Linux native VST plugin +(`libyabridge.so`) and a VST host that runs under Wine +(`yabridge-host.exe`/`yabridge-host.exe.so`, and +`yabridge-host-32.exe`/`yabridge-host-32.exe.so` if the bitbirdge is enabled). +I'll refer to the copy of or the symlink to `libyabridge.so` as _the plugin_, +the native Linux VST host that's hosting the plugin as _the native VST host_, +the Wine VST host application that's hosting a Windows `.dll` file as _the Wine +VST host_, and the Windows VST plugin that's being loaded in the Wine VST host +as the _Windows VST plugin_. The whole process works as follows: + +1. Some copy of or a symlink to `libyabridge.so` gets loaded as a VST plugin in + a Linux VST host. This file should have been renamed to match a Windows VST + plugin `.dll` file in the same directory. For instance, if there's a + `Serum_x64.dll` file you'd like to bridge, then there should be a symlink to + `libyabridge.so` named `Serum_x64.so`. +2. The plugin first attempts to locate and determine: + + - The Windows VST plugin `.dll` file that should be loaded. + + - The architecture of that VST plugin file. This is done by inspecting the + headers if the `.dll` file. + + - The location of the Wine VST host. This will depend on the architecture + detected for the plugin. If the plugin was compiled for the `x86_64` + architecture or the 'Any CPU' target, then we will look for + `yabridge-host.exe`. If the plugin was compiled for the `x86` architecture, + when we'll search for `yabridge-host-32.exe`. + + We will first search for this file alongside the actual location of + `libyabridge.so`. This is useful for development, as it allows you to use a + symlink to `libyabridge.so` directly from the build directory causing + yabridge to automatically pick up the right version of the Wine VST host. + If this file cannot be found, then it will fall back to searching through + the search path. + + - The Wine prefix the plugin is located in. If the `WINEPREFIX` environment + variable is specified, then that will be used instead. + +3. The plugin then sets up a Unix domain socket endpoint to communicate with the + Wine VST host somewhere in a temporary directory and starts listening on it. + I chose to communicate over Unix domain sockets rather than using shared + memory directly because this way you get low latency communication with + without any busy waits or manual synchronisation for free. The added benefit + is that it also makes it possible to send arbitrarily large chunks of data + without having to split it up first. This is useful for transmitting audio + and preset data which may have any arbitrary size. +4. The plugin launches the Wine VST host in the detected wine prefix, passing + the name of the `.dll` file it should be loading and the path to the Unix + domain socket that was just created as its arguments. +5. Communication gets set up using multiple sockets over the end point created + previously. This allows us to easily handle multiple data streams from + different threads using blocking read operations for synchronization. Doing + this greatly simplifies the way communication works without compromising on + latency. The following types of events each get their own socket: + + - Calls from the native VST host to the plugin's `dispatcher()` function. + These get forwarded to the Windows VST plugin through the Wine VST host. + - Calls from the native VST host to the plugin's `dispatcher()` function with + the `effProcessEvents` opcode. These also get forwarded to the Windows VST + plugin through the Wine VST host. This has to be handled separately from + all other events because of limitations of the Win32 API. Without doing + this the plugin would not be able to receive any MIDI events while the GUI + is being resized or a dropdown menu or message box is shown. + - Host callback calls from the Windows VST plugin through the + `audioMasterCallback` function. These get forwarded to the native VST host + through the plugin. + + Both the `dispatcher()` and `audioMasterCallback()` functions are handled + in the same way, with some minor variations on how payload data gets + serialized depending on the opcode of the event being sent. See the section + below this for more details on this procedure. + + - Calls from the native VST host to the plugin's `getParameter()` and + `setParameter()` functions. Both functions get forwarded to the Windows VST + plugin through the Wine VST host using a single socket because they're very + similar and don't need any complicated behaviour. + - Calls from the native VST host to the plugin's `processReplacing()` + function. This function gets forwarded to the Windows VST plugin through + the Wine VST. In the rare event that the plugin does not support + `processReplacing()` and only supports The deprecated commutative + `process()` function, then the Wine VST host will emulate the behavior of + `processReplacing()` instead. + + The operations described above involving the host -> plugin `dispatcher()`and + plugin -> host `audioMaster()` functions are all handled by first serializing + the function parameters and any payload data into a binary format so they can + be sent over a socket. The objects used for encoding both the requests and + the responses for theses events can be found in `src/common/serialization.h`, + and the functions that actually read and write these objects over the sockets + are located in `src/common/communication.h`. The actual binary serialization + is handled using [bitsery](https://github.com/fraillt/bitsery). + + Actually sending and receiving the events happens in the `send_event()` and + `receive_event()` functions. When calling either `dispatch()` or + `audioMaster()`, the caller will oftentimes either pass along some kind of + data structure through the void pointer function argument, or they expect the + function's return value to be a pointer to some kind of struct provided by + the plugin or host. The behaviour for reading from and writing into these + void pointers and returning pointers to objects when needed is encapsulated + in the `DispatchDataConverter` and `HostCallbackDataCovnerter` classes for + the `dispatcher()` and `audioMaster()` functions respectively. For operations + involving the plugin editor there is also some extra glue in + `Vst2Bridge::dispatch_wrapper`. On the receiving end of the function calls, + the `passthrough_event()` function which calls the callback functions and + handles the marshalling between our data types created by the + `*DataConverter` classes and the VST API's different pointer types. This + behaviour is separated from `receive_event()` so we can handle MIDI events + separately. This is needed because a select few plugins only store pointers + to the received events rather than copies of the objects. Because of this, + the received event data must live at least until the next audio buffer gets + processed so it needs to be stored temporarily. + +6. The Wine VST host loads the Windows VST plugin and starts forwarding messages + over the sockets described above. +7. After the Windows VST plugin has started loading we will forward all values + from the plugin's `AEffect` struct to the Linux native VST plugin over the + `dispatcher()` socket. This is only done once at startup. After this point + the plugin will stop blocking and has finished loading. + +TODO: Document plugin groups From 40f88b948fb006b57a3a4cc453661817a3476a1d Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 27 May 2020 15:43:43 +0200 Subject: [PATCH 72/73] Document behaviour differences with plugin groups --- docs/architecture.md | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/docs/architecture.md b/docs/architecture.md index c265b0a1..75cbcd98 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -119,4 +119,32 @@ as the _Windows VST plugin_. The whole process works as follows: `dispatcher()` socket. This is only done once at startup. After this point the plugin will stop blocking and has finished loading. -TODO: Document plugin groups +## Plugin groups + +When using plugin groups, the startup and event handling behavior is slightly +different. + +- First of all, instead of directly spawning a Wine process to host the plugin, + yabridge will either: + + - Connect to an existing group host process that matches the plugin's + combination of group name, Wine prefix, and Windows VST plugin architecture, + and ask it to host the Windows VST plugin. + - Spawn a new group process and detach it from the process, then proceed as + normal by connecting to that process as described above. When two yabridge + instances are initialized simultaneously and both try to launch a new group + process, then the process that manages to listen on the group's socket first + will handle both instances. + +- Events, both Win32 messages and `dispatcher()` events, are handled slightly + differently when using plugin groups. Because most of the Win32 API cannot be + used from multiple threads, all plugin initialization and all event handling + has to be done from the same thread. To achieve this, yabridge will use a + slightly modified version of the `dispatcher()` handler that executes the + actual events for all plugins within a single Boost.Asio IO context. +- Win32 messages are now also handled on a timer within the same IO context so + mentioned above. This behavior is different from individually hosted plugins, + where the message loop can simply be run after every event. If any of the + plugins within the plugin group is in a state that would cause the message + loop to fail, such as when a plugin is in the process of opening its editor + GUI, then the message loop will be skipped temporarily. From 706b34eeb49d0300a18a1d8f0df66ff5cd7c20d4 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 27 May 2020 15:44:14 +0200 Subject: [PATCH 73/73] Rearrange order of Win32 and X11 event handling Should not matter that much, but a potential situation where you would want to have handled the X11 events first is when the editor enters a blocking message loop while it is waiting on a GUI component just as the window gets moved by an external program or window manager. --- src/wine-host/bridges/vst2.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 726b1a9c..92e98785 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -162,8 +162,8 @@ void Vst2Bridge::handle_dispatch_single() { plugin, std::bind(&Vst2Bridge::dispatch_wrapper, this, _1, _2, _3, _4, _5, _6))); - handle_win32_events(); handle_x11_events(); + handle_win32_events(); } } catch (const boost::system::system_error&) { // The plugin has cut off communications, so we can shut down this host