Implement another EventPayload for binary data

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<char, max_string_length> buffer;
+    std::vector<char> binary_buffer;
+    std::array<char, max_string_length> string_buffer;
     void* data = std::visit(
         overload{[&](const std::nullptr_t&) -> void* { return nullptr; },
                  [&](const std::string& s) -> void* {
                      return const_cast<char*>(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 =
+    const auto response_data = std::visit(
-        std::holds_alternative<NeedsBuffer>(event.payload)
+        overload{
-            ? std::optional(std::string(static_cast<char*>(data)))
+            [&](WantsBinaryBuffer&) -> std::optional<std::string> {
-            : std::nullopt;
+                // In this case the return value from the event determines how
+                // much data the plugin has written
+                return std::string(static_cast<char*>(data), return_value);
+            },
+            [&](WantsString&) -> std::optional<std::string> {
+                return std::string(static_cast<char*>(data));
+            },
+            [&](auto) -> std::optional<std::string> { return std::nullopt; }},
+        event.payload);
 
     if (logging.has_value()) {
         auto [logger, is_dispatch] = *logging;

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 << "<writable_buffer>"; }},
+                [&](const WantsBinaryBuffer&) {
+                    message << "<writable_buffer>";
+                },
+                [&](const WantsString&) { message << "<writable_string>"; }},
             payload);
 
         message << ")";

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 <class... Ts>
@@ -106,11 +111,17 @@ class alignas(16) DynamicVstEvents {
 };
 
 /**
- * Marker struct to indicate that that the event requires some buffer to write
+ * Marker struct to indicate that that the event needs to write arbitrary data
- * its results into. This is to prevent us from having to unnecessarily sending
+ * to the void pointer, with the return value indicating the amount of data
- * around empty arrays.
+ * 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
+ *   a plugin to report its name or the label for a certain parameter. There are
- *   assume that this is the default if none of the above options apply.
+ *   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 =
+using EventPayload = std::variant<std::nullptr_t,
-    std::variant<std::nullptr_t, std::string, DynamicVstEvents, NeedsBuffer>;
+                                  std::string,
+                                  DynamicVstEvents,
+                                  WantsBinaryBuffer,
+                                  WantsString>;
 
 template <typename S>
 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 <typename S>
     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); });
     }
 };