From 8dad15b597c501fb487d6d20970cf77b05057d9b Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 9 Mar 2020 21:32:49 +0100 Subject: [PATCH] Always use resizable buffers It was a slight problem for audio buffers, but events can apparently also have an arbitrary size because of chunks. --- src/common/communication.h | 75 ++++++++++++++------------------- src/common/serialization.h | 34 --------------- src/plugin/host-bridge.cpp | 7 ++- src/plugin/host-bridge.h | 2 +- src/wine-host/plugin-bridge.cpp | 7 ++- src/wine-host/plugin-bridge.h | 2 +- 6 files changed, 40 insertions(+), 87 deletions(-) diff --git a/src/common/communication.h b/src/common/communication.h index 0ba26bec..b84aa558 100644 --- a/src/common/communication.h +++ b/src/common/communication.h @@ -30,46 +30,35 @@ #include "logging.h" #include "serialization.h" -// I don't want to editor 'include/vestige/aeffectx.h`. That's why this type -// trait and the above serialization function are here.` Clang complains that -// `buffer` should be qualified (and only in some cases), so `buffer_t` it is. -template -struct buffer_t { - using type = typename T::buffer_type; -}; +template +using OutputAdapter = bitsery::OutputBufferAdapter; -template <> -struct buffer_t { - using type = ArrayBuffer<128>; -}; +template +using InputAdapter = bitsery::InputBufferAdapter; /** * Serialize an object using bitsery and write it to a socket. * * @param socket The Boost.Asio socket to write to. * @param object The object to write to the stream. - * @param buffer The buffer to write to. Only needed for when sending audio - * because their buffers might be quite large. + * @param buffer The buffer to write to. This is useful for sending audio and + * chunk data since that can vary in size by a lot. * * @relates read_object */ template -inline void write_object(Socket& socket, - const T& object, - typename buffer_t::type& buffer) { - bitsery::ext::PointerLinkingContext serializer_context{}; - auto length = - bitsery::quickSerialization::type>>( - serializer_context, buffer, object); +inline void write_object( + Socket& socket, + const T& object, + std::vector buffer = std::vector(64)) { + const size_t size = + bitsery::quickSerialization>>( + buffer, object); - socket.send(boost::asio::buffer(buffer, length)); -} - -template -inline void write_object(Socket& socket, const T& object) { - typename buffer_t::type buffer; - write_object(socket, object, buffer); + // Tell the other side how large the object is so it can prepare a buffer + // large enough before sending the data + socket.send(boost::asio::buffer(std::array{size})); + socket.send(boost::asio::buffer(buffer, size)); } /** @@ -80,8 +69,8 @@ inline void write_object(Socket& socket, const T& object) { * @param object The object to deserialize to, if given. This can be used to * update an existing `AEffect` struct without losing the pointers set by the * host and the bridge. - * @param buffer The buffer to write to. Only needed for when sending audio - * because their buffers might be quite large. + * @param buffer The buffer to read into. This is useful for sending audio and + * chunk data since that can vary in size by a lot. * * @throw std::runtime_error If the conversion to an object was not successful. * @@ -90,16 +79,22 @@ inline void write_object(Socket& socket, const T& object) { template inline T& read_object(Socket& socket, T& object, - typename buffer_t::type& buffer) { - auto message_length = socket.receive(boost::asio::buffer(buffer)); + std::vector buffer = std::vector(64)) { + std::array message_length; + socket.receive(boost::asio::buffer(message_length)); + + // Make sure the buffer is large enough + const size_t size = message_length[0]; + buffer.resize(size); + + const auto actual_size = socket.receive(boost::asio::buffer(buffer)); + assert(size == actual_size); - bitsery::ext::PointerLinkingContext serializer_context{}; auto [_, success] = - bitsery::quickDeserialization::type>>( - serializer_context, {buffer.begin(), message_length}, object); + bitsery::quickDeserialization>>( + {buffer.begin(), size}, object); - if (!success) { + if (BOOST_UNLIKELY(!success)) { throw std::runtime_error("Deserialization failure in call:" + std::string(__PRETTY_FUNCTION__)); } @@ -107,12 +102,6 @@ inline T& read_object(Socket& socket, return object; } -template -inline T& read_object(Socket& socket, T& object) { - typename buffer_t::type buffer; - return read_object(socket, object, buffer); -} - template inline T read_object(Socket& socket) { T object; diff --git a/src/common/serialization.h b/src/common/serialization.h index 20d889e2..51bcd9d6 100644 --- a/src/common/serialization.h +++ b/src/common/serialization.h @@ -50,19 +50,6 @@ constexpr size_t max_midi_events = 32; */ constexpr size_t max_string_length = 64; -/** - * A simple constant sized buffer for smaller types that can be allocated on the - * stack. - */ -template -using ArrayBuffer = std::array; - -template -using OutputAdapter = bitsery::OutputBufferAdapter; - -template -using InputAdapter = bitsery::InputBufferAdapter; - // The cannonical overloading template for `std::visitor`, not sure why this // isn't part of the standard library template @@ -157,15 +144,6 @@ using EventPayload = * arguments sent to the `AEffect::dispatch` function. */ struct Event { - // TODO: Possibly use a vector here sicne we can't know the maximum size for - // certain - using buffer_type = ArrayBuffer; - - // Ensure that the buffer can be aligned correctly and that strings will fit - static_assert(std::tuple_size::value % 16 == 0); - static_assert(std::tuple_size::value >= - max_string_length + 32); - int opcode; int index; // TODO: This is an intptr_t, if we want to support 32 bit Wine plugins all @@ -216,8 +194,6 @@ struct Event { * AN instance of this should be sent back as a response to an incoming event. */ struct EventResult { - using buffer_type = ArrayBuffer; - /** * The result that should be returned from the dispatch function. */ @@ -241,8 +217,6 @@ struct EventResult { * whether `value` contains a value or not. */ struct Parameter { - using buffer_type = ArrayBuffer<16>; - int index; std::optional value; @@ -260,8 +234,6 @@ struct Parameter { * from the Wine VST host. */ struct ParameterResult { - using buffer_type = ArrayBuffer<16>; - std::optional value; template @@ -276,12 +248,6 @@ struct ParameterResult { * processing. The number of samples is encoded in each audio buffer's length. */ struct AudioBuffers { - // When sending data we could use a vector of the right size, but when - // receiving data we don't know how large this vector should be in advance - // (or without sending the message length first) - using buffer_type = - ArrayBuffer; - /** * An audio buffer for each of the plugin's audio channels. */ diff --git a/src/plugin/host-bridge.cpp b/src/plugin/host-bridge.cpp index 5a2c4376..0fec8cbc 100644 --- a/src/plugin/host-bridge.cpp +++ b/src/plugin/host-bridge.cpp @@ -90,8 +90,7 @@ HostBridge::HostBridge(audioMasterCallback host_callback) socket_endpoint.path(), bp::env = set_wineprefix(), bp::std_out = wine_stdout, - bp::std_err = wine_stderr), - process_buffer(std::make_unique()) { + bp::std_err = wine_stderr) { logger.log("Initializing yabridge using '" + vst_host_path.string() + "'"); logger.log("plugin: '" + vst_plugin_path.string() + "'"); logger.log("wineprefix: '" + @@ -202,12 +201,12 @@ void HostBridge::process_replacing(AEffect* /*plugin*/, } const AudioBuffers request{input_buffers, sample_frames}; - write_object(host_vst_process_replacing, request, *process_buffer); + write_object(host_vst_process_replacing, request, process_buffer); // /Write the results back to the `outputs` arrays AudioBuffers response; response = - read_object(host_vst_process_replacing, response, *process_buffer); + read_object(host_vst_process_replacing, response, process_buffer); // TODO: Doesn't quite work yet, not sure which side is causing problems assert(response.buffers.size() == static_cast(plugin.numOutputs)); diff --git a/src/plugin/host-bridge.h b/src/plugin/host-bridge.h index c4dc11f9..0ecc7b17 100644 --- a/src/plugin/host-bridge.h +++ b/src/plugin/host-bridge.h @@ -163,5 +163,5 @@ class HostBridge { * A scratch buffer for sending and receiving data during `process` and * `processReplacing` calls. */ - std::unique_ptr process_buffer; + std::vector process_buffer; }; diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp index b4931359..8b2d4a0b 100644 --- a/src/wine-host/plugin-bridge.cpp +++ b/src/wine-host/plugin-bridge.cpp @@ -61,8 +61,7 @@ PluginBridge::PluginBridge(std::string plugin_dll_path, vst_host_callback(io_context), host_vst_parameters(io_context), host_vst_process_replacing(io_context), - vst_host_aeffect(io_context), - process_buffer(std::make_unique()) { + vst_host_aeffect(io_context) { // Got to love these C APIs if (plugin_handle == nullptr) { throw std::runtime_error("Could not load a shared library at '" + @@ -154,7 +153,7 @@ PluginBridge::PluginBridge(std::string plugin_dll_path, while (true) { AudioBuffers request; request = read_object(host_vst_process_replacing, request, - *process_buffer); + process_buffer); // TODO: Check if the plugin doesn't support `processReplacing` and // call the legacy `process` function instead @@ -176,7 +175,7 @@ PluginBridge::PluginBridge(std::string plugin_dll_path, request.sample_frames); AudioBuffers response{output_buffers, request.sample_frames}; - write_object(host_vst_process_replacing, response, *process_buffer); + write_object(host_vst_process_replacing, response, process_buffer); } }); diff --git a/src/wine-host/plugin-bridge.h b/src/wine-host/plugin-bridge.h index 0ced2932..20296a6e 100644 --- a/src/wine-host/plugin-bridge.h +++ b/src/wine-host/plugin-bridge.h @@ -115,5 +115,5 @@ class PluginBridge { * A scratch buffer for sending and receiving data during `process` and * `processReplacing` calls. */ - std::unique_ptr process_buffer; + std::vector process_buffer; };