From 1ee0ffef8b31ffc5c69dfbd640ef0334edd4cff1 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 11 Mar 2020 15:52:56 +0100 Subject: [PATCH] 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 | 35 +++++------ src/common/logging.cpp | 20 +++++-- src/common/logging.h | 3 +- src/common/serialization.h | 101 +++++++++++++++++++++++--------- src/plugin/host-bridge.cpp | 9 ++- src/wine-host/plugin-bridge.cpp | 9 ++- 6 files changed, 122 insertions(+), 55 deletions(-) diff --git a/src/common/events.h b/src/common/events.h index 5c033b68..c6d670f0 100644 --- a/src/common/events.h +++ b/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()) { - char* output = static_cast(data); + // The default behavior is to handle this as a null terminated C-style + // string + std::visit(overload{[&](const auto&) {}, + [&](const std::string& s) { + char* output = static_cast(data); - // For correctness we will copy the entire buffer and add a - // terminating null byte ourselves. In practice `response.data` will - // only ever contain C-style strings, but this would work with any - // other data format that can contain null bytes. - std::copy(response.data->begin(), response.data->end(), output); - output[response.data->size()] = 0; - } + // We use std::string for easy transport but in + // practice we're always writing null terminated + // C-style strings + std::copy(s.begin(), s.end(), output); + output[s.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 { + [&](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(data), return_value); }, - [&](WantsVstTimeInfo&) -> std::optional { + [&](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 - // `EventPayload`, even though this is as far as I'm aware - // the only non-string/buffer being returned. + // TODO: Use the fancy new VstTimeInfo variant here return std::string(reinterpret_cast(return_value), sizeof(VstTimeInfo)); }, - [&](WantsString&) -> std::optional { + [&](WantsString&) -> EventResposnePayload { return std::string(static_cast(data)); }, - [&](auto) -> std::optional { return std::nullopt; }}, + [&](auto) -> EventResposnePayload { return std::monostate(); }}, event.payload); if (logging.has_value()) { diff --git a/src/common/logging.cpp b/src/common/logging.cpp index 348705d1..34cc3523 100644 --- a/src/common/logging.cpp +++ b/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 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 << ", "; }, + [&](const VstTimeInfo&) { message << ", "; }}, + payload); log(message.str()); } diff --git a/src/common/logging.h b/src/common/logging.h index e0eca0e5..22903654 100644 --- a/src/common/logging.h +++ b/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 payload); + const EventResposnePayload& payload); private: /** diff --git a/src/common/serialization.h b/src/common/serialization.h index 5765801c..e2ba077f 100644 --- a/src/common/serialization.h +++ b/src/common/serialization.h @@ -64,6 +64,43 @@ struct overload : Ts... { template overload(Ts...)->overload; +/** + * 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 +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 +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; + +template +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 - * dispatch function. + * Events typically either just return their return value or write a string + * into the void pointer, but sometimes an event response should forward + * some kind of special struct. */ - std::optional data; + EventResposnePayload payload; template 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.ext(data, bitsery::ext::StdOptional(), - [](S& s, auto& v) { s.text1b(v, binary_buffer_size); }); + + s.object(payload); } }; @@ -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 -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); -} diff --git a/src/plugin/host-bridge.cpp b/src/plugin/host-bridge.cpp index bc661bb7..1def9114 100644 --- a/src/plugin/host-bridge.cpp +++ b/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(response.payload); + chunk.assign(buffer.begin(), buffer.end()); - *static_cast(data) = chunk.data(); + *static_cast(data) = chunk.data(); + } break; default: DefaultDataConverter::write(opcode, data, response); diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp index 134b6aa5..0586acdf 100644 --- a/src/wine-host/plugin-bridge.cpp +++ b/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( - static_cast(response.data->data())); + // TODO: Use the fancy new `VstTimeINfo` variant here + { + std::string buffer = + std::get(response.payload); + time = *static_cast( + static_cast(buffer.data())); + } break; default: DefaultDataConverter::write(opcode, data, response);