diff --git a/README.md b/README.md index 8246cd6b..e82843a7 100644 --- a/README.md +++ b/README.md @@ -11,15 +11,13 @@ There are a few things that should be done before releasing this, including: - Fix implementation bugs: - Phase Plant doesn't play any sound until the editor has been opened? - - Phase Plant sometimes crashes during `effEditIdle()`, not sure what - situation triggers this. - - Phase plant can't load large chunks. Seems to be a problem with Phase Plant - since other plugins don't have this issue. + - Large chunk buffers can't be sent over the socket in one go, this causes + issues with presets that are multiple megabytes in size. + - Serum crashed and audio engine froze while browsing through Serum presets in + the browser? - KiloHearts plugins create a ridiculous amount of file descriptor leaks in wineserver when esync is enabled. I haven't come across any other plugins that do this. Not sure if this is fixable in yabridge. - - Serum crashed and audio engine froze while browsing through Serum presets in - the browser? - Polish GUIs even further. There are some todos left in `src/wine-host/editor.{h,cpp}`. - Add missing details if any to the architecture section. diff --git a/src/common/events.h b/src/common/events.h index af5ffdf1..e430aa30 100644 --- a/src/common/events.h +++ b/src/common/events.h @@ -190,30 +190,39 @@ void passthrough_event(boost::asio::local::stream_protocol::socket& socket, // arbitrary C-style string. std::array string_buffer; string_buffer[0] = 0; + // This buffer is only used for retrieving chunk data and will be allocated + // as needed + std::vector binary_buffer; + void* data = std::visit( - overload{ - [&](const std::nullptr_t&) -> void* { return nullptr; }, - [&](const std::string& s) -> void* { - return const_cast(s.c_str()); - }, - [&](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. - return reinterpret_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; }, - [&](VstParameterProperties& props) -> void* { return &props; }, - [&](WantsVstRect&) -> void* { return string_buffer.data(); }, - [&](const WantsVstTimeInfo&) -> void* { return nullptr; }, - [&](WantsString&) -> void* { return string_buffer.data(); }}, + 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()); + }, + [&](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. + return reinterpret_cast(window_handle); + }, + [&](const AEffect&) -> void* { return nullptr; }, + [&](DynamicVstEvents& events) -> void* { + return &events.as_c_events(); + }, + [&](WantsChunkBuffer&) -> void* { + binary_buffer.resize(binary_buffer_size); + return binary_buffer.data(); + }, + [&](VstIOProperties& props) -> void* { return &props; }, + [&](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, @@ -256,8 +265,9 @@ void passthrough_event(boost::asio::local::stream_protocol::socket& socket, // 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 - return std::string(*static_cast(data), - return_value); + const uint8_t* chunk_data = *static_cast(data); + return std::vector(chunk_data, + chunk_data + return_value); }, [&](VstIOProperties& props) -> EventResposnePayload { return props; diff --git a/src/common/logging.cpp b/src/common/logging.cpp index 07333259..1b8e815a 100644 --- a/src/common/logging.cpp +++ b/src/common/logging.cpp @@ -173,6 +173,7 @@ void Logger::log_event(bool is_dispatch, message << "<" << s.size() << " bytes>"; } }, + [&](const std::vector&) { message << ""; }, [&](const intptr_t&) { message << ""; }, [&](const AEffect&) { message << ""; }, [&](const DynamicVstEvents& events) { @@ -226,6 +227,9 @@ void Logger::log_event_response(bool is_dispatch, message << ", <" << s.size() << " bytes>"; } }, + [&](const std::vector& buffer) { + message << "<" << buffer.size() << "byte chunk>"; + }, [&](const AEffect&) { message << ", "; }, [&](const VstIOProperties&) { message << ", "; }, [&](const VstParameterProperties& props) { diff --git a/src/common/serialization.h b/src/common/serialization.h index 28241128..ff761c5c 100644 --- a/src/common/serialization.h +++ b/src/common/serialization.h @@ -51,11 +51,11 @@ constexpr size_t max_midi_events = max_buffer_size / sizeof(size_t); [[maybe_unused]] constexpr size_t max_string_length = 64; /** - * The size for a buffer in which we're receiving chunks. Allow for up to 1 GB + * The size for a buffer in which we're receiving chunks. Allow for up to 50 MB * chunks. Hopefully no plugin will come anywhere near this limit, but it will * add up when plugins start to audio samples in their presets. */ -constexpr size_t binary_buffer_size = 1 << 30; +constexpr size_t binary_buffer_size = 50 << 20; // The cannonical overloading template for `std::visitor`, not sure why this // isn't part of the standard library @@ -218,6 +218,10 @@ struct WantsString {}; * size. We can replace `std::string` with `char*` once it does for * clarity's sake. * + * - A byte vector for handling chunk data during `effSetChunk()`. We can't + reuse the regular string handling here since the data may contain null + bytes and `std::string::as_c_str()` might cut off everything after the + first null byte. * - An X11 window handle. * - Specific data structures from `aeffextx.h`. For instance an event with the * opcode `effProcessEvents` the hosts passes a `VstEvents` struct containing @@ -226,15 +230,19 @@ struct WantsString {}; * * - Some empty buffer for the plugin to write its own data to, for instance for * a plugin to report its name or the label for a certain parameter. There are - * two separate cases here: + * two separate cases here. This is typically a short null terminated + * C-string. We'll assume this as the default case when none of the above + * options apply. * * - Either the plugin writes arbitrary data and uses its return value to * indicate how much data was written (i.e. for the `effGetChunk` opcode). + * For this we use a vector of bytes instead of a string since * - Or the plugin will write a short null terminated C-string there. We'll * assume that this is the default if none of the above options apply. */ using EventPayload = std::variant, size_t, AEffect, DynamicVstEvents, @@ -251,9 +259,10 @@ void serialize(S& s, EventPayload& payload) { bitsery::ext::StdVariant{ [](S&, std::nullptr_t&) {}, [](S& s, std::string& string) { - // `binary_buffer_size` and not `max_string_length` because we - // also use this to send back large chunk data - s.text1b(string, binary_buffer_size); + s.text1b(string, max_string_length); + }, + [](S& s, std::vector& buffer) { + s.container1b(buffer, binary_buffer_size); }, [](S& s, size_t& window_handle) { s.value8b(window_handle); }, [](S& s, AEffect& effect) { s.object(effect); }, @@ -312,16 +321,16 @@ struct Event { * * - Nothing, on which case only the return value from the callback function * gets passed along. - * - Some buffer stored in a `std::string`. This is typically read from and - * written as C-style string, but in the case of `effGetChunk` this is some - * blob of binary data that should be written to `HostBridge::chunk_data` - * instenad. + * - A (short) string. + * - Some binary blob stored as a byte vector. During `effGetChunk` this will + contain some chunk data that should be written to `HostBridge::chunk_data`. * - A specific struct in response to an event such as `audioMasterGetTime` or * `audioMasterIOChanged`. * - An X11 window pointer for the editor window. */ using EventResposnePayload = std::variant, AEffect, VstIOProperties, VstParameterProperties, @@ -334,10 +343,10 @@ void serialize(S& s, EventResposnePayload& payload) { bitsery::ext::StdVariant{ [](S&, std::nullptr_t&) {}, [](S& s, std::string& string) { - // `binary_buffer_size` and not `max_string_length` - // because we also use this to send back large chunk - // data - s.text1b(string, binary_buffer_size); + s.text1b(string, max_string_length); + }, + [](S& s, std::vector& buffer) { + s.container1b(buffer, binary_buffer_size); }, [](S& s, AEffect& effect) { s.object(effect); }, [](S& s, VstIOProperties& props) { s.object(props); }, diff --git a/src/plugin/host-bridge.cpp b/src/plugin/host-bridge.cpp index f0239ef1..96724607 100644 --- a/src/plugin/host-bridge.cpp +++ b/src/plugin/host-bridge.cpp @@ -206,11 +206,13 @@ class DispatchDataConverter : DefaultDataConverter { case effGetChunk: return WantsChunkBuffer(); break; - case effSetChunk: + case effSetChunk: { + const uint8_t* chunk_data = static_cast(data); + // When the host passes a chunk it will use the value parameter // to tell us its length - return std::string(static_cast(data), value); - break; + return std::vector(chunk_data, chunk_data + value); + } break; case effProcessEvents: return DynamicVstEvents(*static_cast(data)); break; @@ -243,8 +245,8 @@ class DispatchDataConverter : DefaultDataConverter { // Write the chunk data to some publically accessible place in // `HostBridge` and write a pointer to that struct to the data // pointer - - const auto buffer = std::get(response.payload); + const auto buffer = + std::get>(response.payload); chunk.assign(buffer.begin(), buffer.end()); *static_cast(data) = chunk.data();