diff --git a/src/common/communication.h b/src/common/communication.h index b84aa558..47a628c3 100644 --- a/src/common/communication.h +++ b/src/common/communication.h @@ -125,7 +125,7 @@ struct DefaultDataConverter { if (c_string[0] != 0) { return std::string(c_string); } else { - return NeedsBuffer{}; + return WantsString{}; } } }; @@ -220,29 +220,44 @@ void passthrough_event(boost::asio::local::stream_protocol::socket& socket, // Some buffer for the event to write to if needed. We only pass around a // marker struct to indicate that this is indeed the case. - std::array buffer; + std::vector binary_buffer; + std::array string_buffer; void* data = std::visit( overload{[&](const std::nullptr_t&) -> void* { return nullptr; }, [&](const std::string& s) -> void* { return const_cast(s.c_str()); }, - // TODO: Check if the deserialization leaks memory [&](DynamicVstEvents& events) -> void* { return &events.as_c_events(); }, - [&](NeedsBuffer&) -> void* { return buffer.data(); }}, + [&](WantsBinaryBuffer&) -> void* { + // Only allocate when we actually need this, i.e. when + // we're getting a chunk from the plugin + binary_buffer.resize(binary_buffer_size); + return binary_buffer.data(); + }, + [&](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 for empty buffers + // Only write back data when needed, this depends on the event payload type // XXX: Is it possbile here that we got passed a non empty buffer (i.e. // because it was not zeroed out by the host) for an event that should // report some data back? - const auto response_data = - std::holds_alternative(event.payload) - ? std::optional(std::string(static_cast(data))) - : std::nullopt; + const auto response_data = std::visit( + overload{ + [&](WantsBinaryBuffer&) -> std::optional { + // In this case the return value from the event determines how + // much data the plugin has written + return std::string(static_cast(data), return_value); + }, + [&](WantsString&) -> std::optional { + return std::string(static_cast(data)); + }, + [&](auto) -> std::optional { return std::nullopt; }}, + event.payload); if (logging.has_value()) { auto [logger, is_dispatch] = *logging; diff --git a/src/common/logging.cpp b/src/common/logging.cpp index 545ea9fb..fff8fb38 100644 --- a/src/common/logging.cpp +++ b/src/common/logging.cpp @@ -164,7 +164,10 @@ void Logger::log_event(bool is_dispatch, [&](const DynamicVstEvents& events) { message << "<" << events.events.size() << " midi_events>"; }, - [&](const NeedsBuffer&) { message << ""; }}, + [&](const WantsBinaryBuffer&) { + message << ""; + }, + [&](const WantsString&) { message << ""; }}, payload); message << ")"; diff --git a/src/common/serialization.h b/src/common/serialization.h index 917178e0..92b9ffcb 100644 --- a/src/common/serialization.h +++ b/src/common/serialization.h @@ -50,6 +50,11 @@ constexpr size_t max_midi_events = 32; */ constexpr size_t max_string_length = 64; +/** + * The size for a buffer in which we're receiving chunks. + */ +constexpr size_t binary_buffer_size = 2 << 20; + // The cannonical overloading template for `std::visitor`, not sure why this // isn't part of the standard library template @@ -106,11 +111,17 @@ class alignas(16) DynamicVstEvents { }; /** - * Marker struct to indicate that that the event requires some buffer to write - * its results into. This is to prevent us from having to unnecessarily sending - * around empty arrays. + * Marker struct to indicate that that the event needs to write arbitrary data + * to the void pointer, with the return value indicating the amount of data + * actually written. */ -struct NeedsBuffer {}; +struct WantsBinaryBuffer {}; + +/** + * Marker struct to indicate that that the event requires some buffer to write + * a C-string into. + */ +struct WantsString {}; /** * VST events are passed a void pointer that can contain a variety of different @@ -132,11 +143,19 @@ struct NeedsBuffer {}; * TODO: A lot of these are still missing * * - 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. We'll - * assume that this is the default if none of the above options apply. + * a plugin to report its name or the label for a certain parameter. There are + * two separate cases here: + * + * - Either the plugin writes arbitrary data and uses its return value to + * indicate how much data was written (i.e. for the `effGetChunk` opcode). + * - 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; +using EventPayload = std::variant; template void serialize(S& s, EventPayload& payload) { @@ -151,7 +170,7 @@ void serialize(S& s, EventPayload& payload) { s.container1b(event.dump); }); }, - [](S&, NeedsBuffer&) {}}); + [](S&, WantsBinaryBuffer&) {}, [](S&, WantsString&) {}}); } /** @@ -210,8 +229,10 @@ struct EventResult { template void serialize(S& s) { s.value8b(return_value); + // `max_chunk_buffer_size` and not `max_string_length` because we also + // use this to send back large chunk data s.ext(data, bitsery::ext::StdOptional(), - [](S& s, auto& v) { s.text1b(v, max_string_length); }); + [](S& s, auto& v) { s.text1b(v, binary_buffer_size); }); } };