From ed8e3ba114129a8bf8a5ece6734ee54ee04cb3f1 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 1 May 2020 14:06:06 +0200 Subject: [PATCH] Refactor event receiving to optimize MIDI handlign This keeps compatibility with some weirdly designed plugins (such as Kontakt) while avoiding some unnecessary data transformations. Before this we'd convert from a `DynamicVstEvents` object to a `VstEvents` object, back to a `DynamicVstEvents` and then finally back into another `VstEvents` object. With this change we can skip the second half of the conversions. --- README.md | 19 +- src/common/events.h | 296 ++++++++++++++++++-------------- src/plugin/host-bridge.cpp | 6 +- src/wine-host/plugin-bridge.cpp | 57 +++--- 4 files changed, 215 insertions(+), 163 deletions(-) diff --git a/README.md b/README.md index 5893b3ab..1d2555b9 100644 --- a/README.md +++ b/README.md @@ -289,16 +289,19 @@ process works as follows: sockets. The actual binary serialization is handled using [bitsery](https://github.com/fraillt/bitsery). - Sending and receiving events happen in the `send_event()` and - `passthrough_event()` functions. The `passthrough_event()` function calls the - callback functions and handles the marshalling between our data types and the - VST API's different pointer types. Reading data and writing the results back - for host-to-plugin `dispatcher()` calls and for plugin-to-host + Sending and receiving host -> plugin and plugin -> host events happen in the + `send_event()` and `receive_event()` functions. Reading data and writing the + results back for host-to-plugin `dispatcher()` calls and for plugin-to-host `audioMaster()` callbacks happen in the `DispatchDataConverter` and `HostCallbackDataConverter` classes respectively, with a bit of extra glue - for GUI related operations in `PluginBridge::dispatch_wrapper`. Rewriting all - of this tightly coupled logic to be all in one place sadly only makes things - even more complicated. + for GUI related operations in `PluginBridge::dispatch_wrapper`. On the + receiving end, the `passthrough_event()` function calls the callback + functions and handles the marshalling between our data types and the VST + API's different pointer types. This behaviour is separated from + `receive_event()` so we can some special handling for MIDI events, since a + select few plugins only store pointers to the received events rather than + copies of the objects. This requires the received event data to live at least + until the next audio buffer gets processed. 6. The Wine VST host loads the Windows VST plugin and starts forwarding messages over the sockets described above. diff --git a/src/common/events.h b/src/common/events.h index 37f6100b..7cd1740a 100644 --- a/src/common/events.h +++ b/src/common/events.h @@ -98,7 +98,7 @@ class DefaultDataConverter { * the parameters and return value of this function. * * @param socket The socket to write over, should be the same socket the other - * endpoint is using to call `passthrough_event()`. + * endpoint is using to call `receive_event()`. * @param write_mutex A mutex to ensure that only one thread can write to * the socket at once. Needed because VST hosts and plugins can and sometimes * will call the `dispatch()` or `audioMaster()` functions from multiple @@ -113,6 +113,7 @@ class DefaultDataConverter { * for sending `dispatch()` events or host callbacks. Optional since it * doesn't have to be done on both sides. * + * @relates receive_event * @relates passthrough_event */ template @@ -160,27 +161,29 @@ intptr_t send_event(boost::asio::local::stream_protocol::socket& socket, } /** - * Receive an event from a socket and pass it through to some callback function. - * This is used for both the host -> plugin 'dispatch' events and the plugin -> - * host 'audioMaster' host callbacks. This callback function is either one of - * those functions. + * Receive an event from a socket, call a function to generate a response, and + * write the response back over the socket. This is usually used together with + * `passthrough_event()` which passes the event data through to an event + * dispatcher function. This behaviour is split into two functions to avoid + * redundant data conversions when handling MIDI data, as some plugins require + * the received data to be temporarily stored until the next event audio buffer + * gets processed. * * @param socket The socket to receive on and to send the response back to. * @param logging A pair containing a logger instance and whether or not this is * for sending `dispatch()` events or host callbacks. Optional since it * doesn't have to be done on both sides. - * @param plugin The `AEffect` instance that should be passed to the callback - * function. - * @param callback The function to call with the arguments received from the - * socket. + * @param callback The function used to generate a response out of an event. + * + * @tparam F A function type in the form of `EventResponse(Event)`. * * @relates send_event + * @relates passthrough_event */ template -void passthrough_event(boost::asio::local::stream_protocol::socket& socket, - std::optional> logging, - AEffect* plugin, - F callback) { +void receive_event(boost::asio::local::stream_protocol::socket& socket, + std::optional> logging, + F callback) { auto event = read_object(socket); if (logging.has_value()) { auto [logger, is_dispatch] = logging.value(); @@ -188,124 +191,161 @@ void passthrough_event(boost::asio::local::stream_protocol::socket& socket, event.payload, event.option); } - // This buffer is used to write strings and small objects to. We'll - // initialize it with a single null to prevent it from being read as some - // arbitrary C-style string. - std::array string_buffer; - string_buffer[0] = 0; - - void* data = std::visit( - overload{ - [&](const std::nullptr_t&) -> void* { return nullptr; }, - [&](const std::string& s) -> void* { - return const_cast(s.c_str()); - }, - [&](const std::vector& buffer) -> void* { - return const_cast(buffer.data()); - }, - [&](native_size_t& window_handle) -> void* { - // This is the X11 window handle that the editor should reparent - // itself to. We have a special wrapper around the dispatch - // function that intercepts `effEditOpen` events and creates a - // Win32 window and then finally embeds the X11 window Wine - // created into this wnidow handle. Make sure to convert the - // window ID first to `size_t` in case this is the 32-bit host. - return reinterpret_cast( - static_cast(window_handle)); - }, - [&](const AEffect&) -> void* { return nullptr; }, - [&](DynamicVstEvents& events) -> void* { - return &events.as_c_events(); - }, - [&](WantsChunkBuffer&) -> void* { return string_buffer.data(); }, - [&](VstIOProperties& props) -> void* { return &props; }, - [&](VstMidiKeyName& key_name) -> void* { return &key_name; }, - [&](VstParameterProperties& props) -> void* { return &props; }, - [&](WantsVstRect&) -> void* { return string_buffer.data(); }, - [&](const WantsVstTimeInfo&) -> void* { return nullptr; }, - [&](WantsString&) -> void* { return string_buffer.data(); }}, - event.payload); - - const intptr_t return_value = callback(plugin, event.opcode, event.index, - event.value, data, event.option); - - // Only write back data when needed, this depends on the event payload type - const auto response_data = std::visit( - overload{[&](auto) -> EventResposnePayload { return nullptr; }, - [&](const AEffect& updated_plugin) -> EventResposnePayload { - // This is a bit of a special case! Instead of writing some - // return value, we will update values on the native VST - // plugin's `AEffect` object. This is triggered by the - // `audioMasterIOChanged` callback from the hsoted VST - // plugin. - - // These are the same fields written by bitsery in the - // initialization of `HostBridge`. I can't think of a way t - // oreuse the serializer without first having to serialize - // `updated_plugin` first though. - plugin->magic = updated_plugin.magic; - plugin->numPrograms = updated_plugin.numPrograms; - plugin->numParams = updated_plugin.numParams; - plugin->numInputs = updated_plugin.numInputs; - plugin->numOutputs = updated_plugin.numOutputs; - plugin->flags = updated_plugin.flags; - plugin->initialDelay = updated_plugin.initialDelay; - plugin->empty3a = updated_plugin.empty3a; - plugin->empty3b = updated_plugin.empty3b; - plugin->unkown_float = updated_plugin.unkown_float; - plugin->uniqueID = updated_plugin.uniqueID; - plugin->version = updated_plugin.version; - - return nullptr; - }, - [&](WantsChunkBuffer&) -> EventResposnePayload { - // In this case the plugin will have written its data - // stored in an array to which a pointer is stored in - // `data`, with the return value from the event determines - // how much data the plugin has written - const uint8_t* chunk_data = *static_cast(data); - return std::vector(chunk_data, - chunk_data + return_value); - }, - [&](VstIOProperties& props) -> EventResposnePayload { - return props; - }, - [&](VstMidiKeyName& key_name) -> EventResposnePayload { - return key_name; - }, - [&](VstParameterProperties& props) -> EventResposnePayload { - return props; - }, - [&](WantsVstRect&) -> EventResposnePayload { - // The plugin has written a pointer to a VstRect struct - // into the data poitner - return **static_cast(data); - }, - [&](WantsVstTimeInfo&) -> EventResposnePayload { - // Not sure why the VST API has twenty different ways of - // returning structs, but in this case the value returned - // from the callback function is actually a pointer to a - // `VstTimeInfo` struct! It can also be a null pointer if - // the host doesn't support this. - const auto time_info = - reinterpret_cast(return_value); - if (time_info == nullptr) { - return nullptr; - } else { - return *time_info; - } - }, - [&](WantsString&) -> EventResposnePayload { - return std::string(static_cast(data)); - }}, - event.payload); - + EventResult response = callback(event); if (logging.has_value()) { auto [logger, is_dispatch] = logging.value(); - logger.log_event_response(is_dispatch, event.opcode, return_value, - response_data); + logger.log_event_response(is_dispatch, event.opcode, + response.return_value, response.payload); } - EventResult response{return_value, response_data}; write_object(socket, response); } + +/** + * Create a callback function that takes an `Event` object, decodes the data + * into the expected format for VST2 function calls, calls the given function + * (either `AEffect::dispatcher()` for host -> plugin events or `audioMaster()` + * for plugin -> host events), and serializes the results back into an + * `EventResult` object. I'd rather not get too Haskell-y in my C++, but this is + * the cleanest solution for this problem. + * + * This is the receiving analogue of the `*DataCovnerter` objects. + * + * @param plugin The `AEffect` instance that should be passed to the callback + * function. + * @param callback The function to call with the arguments received from the + * socket. + * + * @tparam A function with the same signature as `AEffect::dispatcher` or + * `audioMasterCallback`. + * + * @return A `EventResult(Event)` callback function that can be passed to + * `receive_event`. + * + * @relates receive_event + */ +template +auto passthrough_event(AEffect* plugin, F callback) { + return [=](Event& event) -> EventResult { + // This buffer is used to write strings and small objects to. We'll + // initialize it with a single null to prevent it from being read as + // some arbitrary C-style string. + std::array string_buffer; + string_buffer[0] = 0; + + void* data = std::visit( + overload{ + [&](const std::nullptr_t&) -> void* { return nullptr; }, + [&](const std::string& s) -> void* { + return const_cast(s.c_str()); + }, + [&](const std::vector& buffer) -> void* { + return const_cast(buffer.data()); + }, + [&](native_size_t& window_handle) -> void* { + // This is the X11 window handle that the editor should + // reparent itself to. We have a special wrapper around the + // dispatch function that intercepts `effEditOpen` events + // and creates a Win32 window and then finally embeds the + // X11 window Wine created into this wnidow handle. Make + // sure to convert the window ID first to `size_t` in case + // this is the 32-bit host. + return reinterpret_cast( + static_cast(window_handle)); + }, + [&](const AEffect&) -> void* { return nullptr; }, + [&](DynamicVstEvents& events) -> void* { + return &events.as_c_events(); + }, + [&](WantsChunkBuffer&) -> void* { + return string_buffer.data(); + }, + [&](VstIOProperties& props) -> void* { return &props; }, + [&](VstMidiKeyName& key_name) -> void* { return &key_name; }, + [&](VstParameterProperties& props) -> void* { return &props; }, + [&](WantsVstRect&) -> void* { return string_buffer.data(); }, + [&](const WantsVstTimeInfo&) -> void* { return nullptr; }, + [&](WantsString&) -> void* { return string_buffer.data(); }}, + event.payload); + + const intptr_t return_value = callback( + plugin, event.opcode, event.index, event.value, data, event.option); + + // Only write back data when needed, this depends on the event payload + // type + const auto response_data = std::visit( + overload{ + [&](auto) -> EventResposnePayload { return nullptr; }, + [&](const AEffect& updated_plugin) -> EventResposnePayload { + // This is a bit of a special case! Instead of writing some + // return value, we will update values on the native VST + // plugin's `AEffect` object. This is triggered by the + // `audioMasterIOChanged` callback from the hsoted VST + // plugin. + + // These are the same fields written by bitsery in the + // initialization of `HostBridge`. I can't think of a way t + // oreuse the serializer without first having to serialize + // `updated_plugin` first though. + plugin->magic = updated_plugin.magic; + plugin->numPrograms = updated_plugin.numPrograms; + plugin->numParams = updated_plugin.numParams; + plugin->numInputs = updated_plugin.numInputs; + plugin->numOutputs = updated_plugin.numOutputs; + plugin->flags = updated_plugin.flags; + plugin->initialDelay = updated_plugin.initialDelay; + plugin->empty3a = updated_plugin.empty3a; + plugin->empty3b = updated_plugin.empty3b; + plugin->unkown_float = updated_plugin.unkown_float; + plugin->uniqueID = updated_plugin.uniqueID; + plugin->version = updated_plugin.version; + + return nullptr; + }, + [&](WantsChunkBuffer&) -> EventResposnePayload { + // In this case the plugin will have written its data + // stored in an array to which a pointer is stored in + // `data`, with the return value from the event determines + // how much data the plugin has written + const uint8_t* chunk_data = *static_cast(data); + return std::vector(chunk_data, + chunk_data + return_value); + }, + [&](VstIOProperties& props) -> EventResposnePayload { + return props; + }, + [&](VstMidiKeyName& key_name) -> EventResposnePayload { + return key_name; + }, + [&](VstParameterProperties& props) -> EventResposnePayload { + return props; + }, + [&](WantsVstRect&) -> EventResposnePayload { + // The plugin has written a pointer to a VstRect struct + // into the data poitner + return **static_cast(data); + }, + [&](WantsVstTimeInfo&) -> EventResposnePayload { + // Not sure why the VST API has twenty different ways of + // returning structs, but in this case the value returned + // from the callback function is actually a pointer to a + // `VstTimeInfo` struct! It can also be a null pointer if + // the host doesn't support this. + const auto time_info = + reinterpret_cast(return_value); + if (time_info == nullptr) { + return nullptr; + } else { + return *time_info; + } + }, + [&](WantsString&) -> EventResposnePayload { + return std::string(static_cast(data)); + }}, + event.payload); + + EventResult response{return_value, response_data}; + + return response; + }; +} diff --git a/src/plugin/host-bridge.cpp b/src/plugin/host-bridge.cpp index 9684edd6..86be85ba 100644 --- a/src/plugin/host-bridge.cpp +++ b/src/plugin/host-bridge.cpp @@ -172,9 +172,9 @@ HostBridge::HostBridge(audioMasterCallback host_callback) host_callback_handler = std::thread([&]() { try { while (true) { - passthrough_event(vst_host_callback, - std::pair(logger, false), - &plugin, host_callback_function); + receive_event( + vst_host_callback, std::pair(logger, false), + passthrough_event(&plugin, host_callback_function)); } } catch (const boost::system::system_error&) { // This happens when the sockets got closed because the plugin diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp index fc699900..993ef007 100644 --- a/src/wine-host/plugin-bridge.cpp +++ b/src/wine-host/plugin-bridge.cpp @@ -147,9 +147,10 @@ void PluginBridge::handle_dispatch() { // lockstep anyway try { while (true) { - passthrough_event(host_vst_dispatch, std::nullopt, plugin, - std::bind(&PluginBridge::dispatch_wrapper, this, - _1, _2, _3, _4, _5, _6)); + receive_event(host_vst_dispatch, std::nullopt, + passthrough_event( + plugin, std::bind(&PluginBridge::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 @@ -179,39 +180,47 @@ void PluginBridge::handle_dispatch() { [[noreturn]] void PluginBridge::handle_dispatch_midi_events() { while (true) { - // TODO: Refactor `passthrough_event()` to factor out the data - // conversion specifically for this case so we don't have to do - // this `DynamicVstEvents -> VstEvents -> void* -> - // DynamicVstEvents -> VstEvents -> void*` conversion dance - passthrough_event( - host_vst_dispatch_midi_events, std::nullopt, plugin, - [&](AEffect* plugin, int opcode, int index, intptr_t value, - void* data, float option) { - if (BOOST_LIKELY(opcode == effProcessEvents)) { + 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. This - // does mean that we have to reconstruct the - // `DynamicVstEvents` object first. - // HACK: Is there a cleaner way to do this, or a way to - // avoid having to store temporary copies of this? + // 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); - DynamicVstEvents& events = - next_audio_buffer_midi_events.emplace_back( - *static_cast(data)); - return plugin->dispatcher(plugin, opcode, index, value, - &events.as_c_events(), option); + 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); + + EventResult response{return_value, nullptr}; + + return response; } else { + using namespace std::placeholders; + std::cerr << "[Warning] Received non-MIDI " "event on MIDI processing thread" << std::endl; - return dispatch_wrapper(plugin, opcode, index, value, data, - option); + // Maybe this should just be a hard error instead, since it + // should never happen + return passthrough_event( + plugin, std::bind(&PluginBridge::dispatch_wrapper, this, + _1, _2, _3, _4, _5, _6))(event); } }); }