Wrap event result data in an std::variant

Gets a bit more complicated this way, but this avoids having to use
string to manually serialize and deserialize arbitrary objects.

The new options for `AEffect` and `VstTimeInfo` structs are not yet
used.

src/common/events.h

@@ -61,16 +61,19 @@ class DefaultDataConverter {
     virtual void write(const int /*opcode*/,
                        void* data,
                        const EventResult& response) {
-        if (response.data.has_value()) {
+        // The default behavior is to handle this as a null terminated C-style
-            char* output = static_cast<char*>(data);
+        // string
+        std::visit(overload{[&](const auto&) {},
+                            [&](const std::string& s) {
+                                char* output = static_cast<char*>(data);
 
-            // For correctness we will copy the entire buffer and add a
+                                // We use std::string for easy transport but in
-            // terminating null byte ourselves. In practice `response.data` will
+                                // practice we're always writing null terminated
-            // only ever contain C-style strings, but this would work with any
+                                // C-style strings
-            // other data format that can contain null bytes.
+                                std::copy(s.begin(), s.end(), output);
-            std::copy(response.data->begin(), response.data->end(), output);
+                                output[s.size()] = 0;
-            output[response.data->size()] = 0;
+                            }},
-        }
+                   response.payload);
     }
 
     /**
@@ -139,7 +142,7 @@ intptr_t send_event(boost::asio::local::stream_protocol::socket& socket,
     if (logging.has_value()) {
         auto [logger, is_dispatch] = logging.value();
         logger.log_event_response(is_dispatch, response.return_value,
-                                  response.data);
+                                  response.payload);
     }
 
     data_converter.write(opcode, data, response);
@@ -200,28 +203,26 @@ void passthrough_event(boost::asio::local::stream_protocol::socket& socket,
     //      report some data back?
     const auto response_data = std::visit(
         overload{
-            [&](WantsChunkBuffer&) -> std::optional<std::string> {
+            [&](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
                 return std::string(*static_cast<char**>(data), return_value);
             },
-            [&](WantsVstTimeInfo&) -> std::optional<std::string> {
+            [&](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!
-                // TODO: Maybe add a variant from these return types similar to
+                // TODO: Use the fancy new VstTimeInfo variant here
-                //       `EventPayload`, even though this is as far as I'm aware
-                //       the only non-string/buffer being returned.
                 return std::string(reinterpret_cast<const char*>(return_value),
                                    sizeof(VstTimeInfo));
             },
-            [&](WantsString&) -> std::optional<std::string> {
+            [&](WantsString&) -> EventResposnePayload {
                 return std::string(static_cast<char*>(data));
             },
-            [&](auto) -> std::optional<std::string> { return std::nullopt; }},
+            [&](auto) -> EventResposnePayload { return std::monostate(); }},
         event.payload);
 
     if (logging.has_value()) {

src/common/logging.cpp

@@ -185,7 +185,7 @@ void Logger::log_event(bool is_dispatch,
 
 void Logger::log_event_response(bool is_dispatch,
                                 intptr_t return_value,
-                                std::optional<std::string> payload) {
+                                const EventResposnePayload& payload) {
     if (BOOST_UNLIKELY(verbosity >= Verbosity::events)) {
         std::ostringstream message;
         if (is_dispatch) {
@@ -195,9 +195,21 @@ void Logger::log_event_response(bool is_dispatch,
         }
 
         message << return_value;
-        if (payload.has_value()) {
+
-            message << ", \"" << payload.value() << "\"";
+        std::visit(
-        }
+            overload{[&](const std::monostate&) {},
+                     [&](const std::string& s) {
+                         if (s.size() < 32) {
+                             message << ", \"" << s << "\"";
+                         } else {
+                             // Long strings contain binary data that we
+                             // probably don't want to print
+                             message << ", <" << s.size() << " bytes>";
+                         }
+                     },
+                     [&](const AEffect&) { message << ", <AEffect_object>"; },
+                     [&](const VstTimeInfo&) { message << ", <time_info>"; }},
+            payload);
 
         log(message.str());
     }

src/common/logging.h

@@ -90,6 +90,7 @@ class Logger {
     void log_set_parameter_response();
     // If is_dispatch is true, then use opcode names from the plugin's dispatch
     // function. Otherwise use names for the host callback function opcodes.
+    // TODO: Cosnt references for both payloads
     void log_event(bool is_dispatch,
                    int opcode,
                    int index,
@@ -98,7 +99,7 @@ class Logger {
                    float option);
     void log_event_response(bool is_dispatch,
                             intptr_t return_value,
-                            std::optional<std::string> payload);
+                            const EventResposnePayload& payload);
 
    private:
     /**

src/common/serialization.h

@@ -64,6 +64,43 @@ struct overload : Ts... {
 template <class... Ts>
 overload(Ts...)->overload<Ts...>;
 
+/**
+ * The serialization function for `AEffect` structs. This will s serialize all
+ * of the values but it will not touch any of the pointer fields. That way you
+ * can deserialize to an existing `AEffect` instance.
+ */
+template <typename S>
+void serialize(S& s, AEffect& plugin) {
+    s.value4b(plugin.magic);
+    s.value4b(plugin.numPrograms);
+    s.value4b(plugin.numParams);
+    s.value4b(plugin.numInputs);
+    s.value4b(plugin.numOutputs);
+    s.value4b(plugin.flags);
+    s.value4b(plugin.initialDelay);
+    s.value4b(plugin.empty3a);
+    s.value4b(plugin.empty3b);
+    s.value4b(plugin.unkown_float);
+    s.value4b(plugin.uniqueID);
+    s.value4b(plugin.version);
+}
+
+template <typename S>
+void serialize(S& s, VstTimeInfo& time_info) {
+    s.value8b(time_info.samplePos);
+    s.value8b(time_info.sampleRate);
+    s.value8b(time_info.nanoSeconds);
+    s.value8b(time_info.ppqPos);
+    s.value8b(time_info.tempo);
+    s.value8b(time_info.barStartPos);
+    s.value8b(time_info.cycleStartPos);
+    s.value8b(time_info.cycleEndPos);
+    s.value4b(time_info.timeSigNumerator);
+    s.value4b(time_info.timeSigDenominator);
+    s.container1b(time_info.empty3);
+    s.value4b(time_info.flags);
+}
+
 /**
  * A wrapper around `VstEvents` that stores the data in a vector instead of a
  * C-style array. Needed until bitsery supports C-style arrays
@@ -223,6 +260,36 @@ struct Event {
     }
 };
 
+/**
+ * The response for an event. This is usually either:
+ *
+ * - 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 specific struct in response to an event such as `audioMasterGetTime` or
+ *   `audioMasterIOChanged`.
+ */
+using EventResposnePayload =
+    std::variant<std::monostate, std::string, AEffect, VstTimeInfo>;
+
+template <typename S>
+void serialize(S& s, EventResposnePayload& payload) {
+    s.ext(payload,
+          bitsery::ext::StdVariant{
+              [](S&, std::monostate&) {},
+              [](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& s, AEffect& effect) { s.object(effect); },
+              [](S& s, VstTimeInfo& time_info) { s.object(time_info); }});
+}
+
 /**
  * AN instance of this should be sent back as a response to an incoming event.
  */
@@ -232,18 +299,17 @@ struct EventResult {
      */
     intptr_t return_value;
     /**
-     * If present, this should get written into the void pointer passed to the
+     * Events typically either just return their return value or write a string
-     * dispatch function.
+     * into the void pointer, but sometimes an event response should forward
+     * some kind of special struct.
      */
-    std::optional<std::string> data;
+    EventResposnePayload payload;
 
     template <typename S>
     void serialize(S& s) {
         s.value8b(return_value);
-        // `binary_buffer_size` and not `max_string_length` because we also use
+
-        // this to send back large chunk data
+        s.object(payload);
-        s.ext(data, bitsery::ext::StdOptional(),
-              [](S& s, auto& v) { s.text1b(v, binary_buffer_size); });
     }
 };
 
@@ -301,24 +367,3 @@ struct AudioBuffers {
         s.value4b(sample_frames);
     }
 };
-
-/**
- * The serialization function for `AEffect` structs. This will s serialize all
- * of the values but it will not touch any of the pointer fields. That way you
- * can deserialize to an existing `AEffect` instance.
- */
-template <typename S>
-void serialize(S& s, AEffect& plugin) {
-    s.value4b(plugin.magic);
-    s.value4b(plugin.numPrograms);
-    s.value4b(plugin.numParams);
-    s.value4b(plugin.numInputs);
-    s.value4b(plugin.numOutputs);
-    s.value4b(plugin.flags);
-    s.value4b(plugin.initialDelay);
-    s.value4b(plugin.empty3a);
-    s.value4b(plugin.empty3b);
-    s.value4b(plugin.unkown_float);
-    s.value4b(plugin.uniqueID);
-    s.value4b(plugin.version);
-}

src/plugin/host-bridge.cpp

@@ -187,10 +187,13 @@ 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
-                assert(response.data.has_value());
+                {
-                chunk.assign(response.data->begin(), response.data->end());
+                    std::string buffer =
+                        std::get<std::string>(response.payload);
+                    chunk.assign(buffer.begin(), buffer.end());
 
-                *static_cast<void**>(data) = chunk.data();
+                    *static_cast<void**>(data) = chunk.data();
+                }
                 break;
             default:
                 DefaultDataConverter::write(opcode, data, response);

src/wine-host/plugin-bridge.cpp

@@ -230,8 +230,13 @@ class HostCallbackDataConverter : DefaultDataConverter {
             case audioMasterGetTime:
                 // Write the returned `VstTimeInfo` struct into a field and make
                 // the function return a poitner to it in the function below
-                time = *static_cast<const VstTimeInfo*>(
+                // TODO: Use the fancy new `VstTimeINfo` variant here
-                    static_cast<const void*>(response.data->data()));
+                {
+                    std::string buffer =
+                        std::get<std::string>(response.payload);
+                    time = *static_cast<const VstTimeInfo*>(
+                        static_cast<const void*>(buffer.data()));
+                }
                 break;
             default:
                 DefaultDataConverter::write(opcode, data, response);