From 421ed21901dc748dcf902288ff834dec5f82fd4a Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 20 Apr 2020 23:36:17 +0200 Subject: [PATCH] Get rid of now no longer needed synchronisation This was mostly a workaround to get Serum to not crash when audio was being processed during a specific point of its `WM_PAINT` message handler. This is no longer needed when using `CreateThread` instead of `std::thread`. --- src/wine-host/editor.cpp | 52 +++++++++++---------------------- src/wine-host/editor.h | 17 ----------- src/wine-host/plugin-bridge.cpp | 12 ++------ src/wine-host/plugin-bridge.h | 11 ------- 4 files changed, 20 insertions(+), 72 deletions(-) diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index e676bb32..43d4f1d8 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -39,7 +39,6 @@ WindowClass::~WindowClass() { Editor::Editor(const std::string& window_class_name, AEffect* effect, - std::mutex& effect_mutex, const size_t parent_window_handle) : window_class(window_class_name), // Create a window without any decoratiosn for easy embedding. The @@ -64,7 +63,6 @@ Editor::Editor(const std::string& window_class_name, child_window(get_x11_handle(win32_handle.get())), // Needed to send update messages on a timer plugin(effect), - processing_mutex(effect_mutex), x11_connection(xcb_connect(nullptr, nullptr), &xcb_disconnect) { // 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 @@ -97,19 +95,25 @@ void Editor::send_idle_event() { } void Editor::handle_events() { - win32_event_loop(); + send_idle_event(); - { - // Always send the `effEditIdle` event manually instead of relying on - // the timer to match the update frequency with that of the native VST - // host. Because some plugins, such as those using GDI+ like Serum, have - // data race issues when drawing at the same time as we're processing - // sound, we'll update the GUI and process the resulting `WM_PAINT` - // event while temporarily blocking the processing thread. - std::lock_guard lock(processing_mutex); + // The null value for the second argument is needed to handle interaction + // with child GUI components + MSG msg; + while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) { + // This timer would periodically send `effEditIdle` events so the editor + // remains responsive even during blocking GUI operations such as open + // dropdowns or message boxes. We filter it out here because we will + // send sent the event manually every time the host calls + // `effEditIdle()`. It will still be fired implicitely when the GUI + // thread gets blocked. + if (msg.message == WM_TIMER && msg.wParam == idle_timer_id && + msg.hwnd == win32_handle.get()) { + continue; + } - send_idle_event(); - win32_event_loop(); + TranslateMessage(&msg); + DispatchMessage(&msg); } // Handle X11 events @@ -206,28 +210,6 @@ LRESULT CALLBACK window_proc(HWND handle, return DefWindowProc(handle, message, wParam, lParam); } -void Editor::win32_event_loop() { - MSG msg; - - // The null value for the second argument is needed to handle interaction - // with child GUI components - while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) { - // This timer would periodically send `effEditIdle` events so the editor - // remains responsive even during blocking GUI operations such as open - // dropdowns or message boxes. We filter it out here because we will - // send sent the event manually every time the host calls - // `effEditIdle()`. It will still be fired implicitely when the GUI - // thread gets blocked. - if (msg.message == WM_TIMER && msg.wParam == idle_timer_id && - msg.hwnd == win32_handle.get()) { - continue; - } - - TranslateMessage(&msg); - DispatchMessage(&msg); - } -} - xcb_window_t get_x11_handle(HWND win32_handle) { return reinterpret_cast( GetProp(win32_handle, "__wine_x11_whole_window")); diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 6006bfdb..b02546c6 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -13,7 +13,6 @@ #include #include -#include #include #include @@ -59,8 +58,6 @@ class Editor { * windows. * @param effect The plugin this window is being created for. Used to send * `effEditIdle` messages on a timer. - * @param processing_mutex The mutex belonging to `effect` audio processing - * routine. * @param parent_window_handle The X11 window handle passed by the VST host * for the editor to embed itself into. * @@ -68,7 +65,6 @@ class Editor { */ Editor(const std::string& window_class_name, AEffect* effect, - std::mutex& processing_mutex, const size_t parent_window_handle); /** @@ -100,14 +96,6 @@ class Editor { win32_handle; private: - /** - * Run the win32 message handling loop, filtering out the events generated - * by the `effEditIdle` timer since those are sent manually on every - * invocation of `handle_events()` to match the `effEditIdle` events sent by - * the DAW. - */ - void win32_event_loop(); - /** * The window handle of the editor window created by the DAW. */ @@ -121,11 +109,6 @@ class Editor { *Needed to handle idle updates through a timer */ AEffect* plugin; - /** - * The mutex belonging to `plugin`'s audio processing routine, see - * `PluginBridge::processing_mutex`'s docstring for more information. - */ - std::mutex& processing_mutex; /** * A pointer to the currently active window. Will be a null pointer if no diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp index 884ab9ab..d1c25970 100644 --- a/src/wine-host/plugin-bridge.cpp +++ b/src/wine-host/plugin-bridge.cpp @@ -220,13 +220,8 @@ void PluginBridge::handle_dispatch() { outputs.push_back(buffer.data()); } - // Some plugins (e.g. Serum) don't allow audio processing while the GUI - // is being updated - { - std::lock_guard lock(processing_mutex); - plugin->processReplacing(plugin, inputs.data(), outputs.data(), - request.sample_frames); - } + plugin->processReplacing(plugin, inputs.data(), outputs.data(), + request.sample_frames); AudioBuffers response{output_buffers, request.sample_frames}; write_object(host_vst_process_replacing, response, process_buffer); @@ -258,8 +253,7 @@ intptr_t PluginBridge::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); - editor.emplace("yabridge plugin", plugin, processing_mutex, - x11_handle); + editor.emplace("yabridge plugin", plugin, x11_handle); return plugin->dispatcher(plugin, opcode, index, value, editor->win32_handle.get(), option); diff --git a/src/wine-host/plugin-bridge.h b/src/wine-host/plugin-bridge.h index 5d456d2e..d3c213ae 100644 --- a/src/wine-host/plugin-bridge.h +++ b/src/wine-host/plugin-bridge.h @@ -108,17 +108,6 @@ class PluginBridge { * handle. */ AEffect* plugin; - /** - * A mutex to prevent certain operations from being performed - * simultaneously. The VST specification does not specify which operations - * have to be thread safe and which do not. Most plugins don't have any - * issues as long as all editor related events are sent from the GUI thread. - * At the moment this mutex is only used to prevent GUI updates (through the - * `effEditIdle()` event and the `WM_PAINT` message that follows) and audio - * processing from being performed at the same time. This prevents data - * races within Serum. - */ - std::mutex processing_mutex; boost::asio::io_context io_context; boost::asio::local::stream_protocol::endpoint socket_endpoint;