From 72e2a9c9f7d0c1ebc7ea6ff367a4d319f87751ea Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 1 Oct 2022 15:48:49 +0200 Subject: [PATCH] Store the entire clap_event_*_t structs This removes both time and computational overhead. Otherwise we'd have to create a second event list in the proper format to avoid lifetime issues. --- src/common/serialization/clap/events.cpp | 145 +++--------- src/common/serialization/clap/events.h | 273 ++++++++++------------- 2 files changed, 151 insertions(+), 267 deletions(-) diff --git a/src/common/serialization/clap/events.cpp b/src/common/serialization/clap/events.cpp index 2dca513e..465e0099 100644 --- a/src/common/serialization/clap/events.cpp +++ b/src/common/serialization/clap/events.cpp @@ -23,171 +23,84 @@ std::optional Event::parse(const clap_event_header_t& generic_event) { std::optional payload; if (generic_event.space_id == CLAP_CORE_EVENT_SPACE_ID) { switch (generic_event.type) { - case CLAP_EVENT_NOTE_ON: { - const auto& event = - reinterpret_cast(generic_event); - payload = payload::Note{ - .event_type = payload::NoteEventType::On, - .note_id = event.note_id, - .port_index = event.port_index, - .channel = event.channel, - .key = event.key, - .velocity = event.velocity, - }; - } break; - case CLAP_EVENT_NOTE_OFF: { - const auto& event = - reinterpret_cast(generic_event); - payload = payload::Note{ - .event_type = payload::NoteEventType::Off, - .note_id = event.note_id, - .port_index = event.port_index, - .channel = event.channel, - .key = event.key, - .velocity = event.velocity, - }; - } break; - case CLAP_EVENT_NOTE_CHOKE: { - const auto& event = - reinterpret_cast(generic_event); - payload = payload::Note{ - .event_type = payload::NoteEventType::Choke, - .note_id = event.note_id, - .port_index = event.port_index, - .channel = event.channel, - .key = event.key, - .velocity = event.velocity, - }; - } break; + case CLAP_EVENT_NOTE_ON: + case CLAP_EVENT_NOTE_OFF: + case CLAP_EVENT_NOTE_CHOKE: case CLAP_EVENT_NOTE_END: { const auto& event = reinterpret_cast(generic_event); - payload = payload::Note{ - .event_type = payload::NoteEventType::End, - .note_id = event.note_id, - .port_index = event.port_index, - .channel = event.channel, - .key = event.key, - .velocity = event.velocity, - }; + + // The original event type can be restored from the header + payload = payload::Note{.event = event}; } break; case CLAP_EVENT_NOTE_EXPRESSION: { const auto& event = reinterpret_cast( generic_event); - payload = payload::NoteExpression{ - .expression_id = event.expression_id, - .note_id = event.note_id, - .port_index = event.port_index, - .channel = event.channel, - .key = event.key, - .value = event.value, - }; + payload = payload::NoteExpression{.event = event}; } break; case CLAP_EVENT_PARAM_VALUE: { const auto& event = reinterpret_cast( generic_event); - payload = payload::ParamValue{ - .param_id = event.param_id, - .cookie = static_cast( - reinterpret_cast(event.cookie)), - .note_id = event.note_id, - .port_index = event.port_index, - .channel = event.channel, - .key = event.key, - .value = event.value, - }; + payload = payload::ParamValue{.event = event}; } break; case CLAP_EVENT_PARAM_MOD: { const auto& event = reinterpret_cast( generic_event); - payload = payload::ParamMod{ - .param_id = event.param_id, - .cookie = static_cast( - reinterpret_cast(event.cookie)), - .note_id = event.note_id, - .port_index = event.port_index, - .channel = event.channel, - .key = event.key, - .amount = event.amount, - }; - } break; - case CLAP_EVENT_PARAM_GESTURE_BEGIN: { - const auto& event = - reinterpret_cast( - generic_event); - payload = payload::ParamGesture{ - .gesture_type = payload::ParamGestureType::Begin, - .param_id = event.param_id, - }; + payload = payload::ParamMod{.event = event}; } break; + case CLAP_EVENT_PARAM_GESTURE_BEGIN: case CLAP_EVENT_PARAM_GESTURE_END: { const auto& event = reinterpret_cast( generic_event); - payload = payload::ParamGesture{ - .gesture_type = payload::ParamGestureType::End, - .param_id = event.param_id, - }; + payload = payload::ParamGesture{.event = event}; } break; case CLAP_EVENT_TRANSPORT: { const auto& event = reinterpret_cast( generic_event); - payload = payload::Transport{ - .flags = event.flags, - .song_pos_beats = event.song_pos_beats, - .song_pos_seconds = event.song_pos_seconds, - .tempo = event.tempo, - .tempo_inc = event.tempo_inc, - .loop_start_beats = event.loop_start_beats, - .loop_end_beats = event.loop_end_beats, - .loop_start_seconds = event.loop_start_seconds, - .loop_end_seconds = event.loop_end_seconds, - .bar_start = event.bar_start, - .bar_number = event.bar_number, - .tsig_num = event.tsig_num, - .tsig_denom = event.tsig_denom, - }; + payload = payload::Transport{.event = event}; } break; case CLAP_EVENT_MIDI: { const auto& event = reinterpret_cast(generic_event); - payload = payload::Midi{ - .port_index = event.port_index, - .data{event.data[0], event.data[1], event.data[2]}, - }; + payload = payload::Midi{.event = event}; } break; case CLAP_EVENT_MIDI_SYSEX: { const auto& event = reinterpret_cast( generic_event); + assert(event.buffer); - payload = payload::MidiSysex{ - .port_index = event.port_index, + const auto sysex_payload = payload::MidiSysex{ + .event = + clap_event_midi_sysex_t{ + .header = event.header, + .port_index = event.port_index, + // The buffer and size fields will be restored + // during the `get_header()` call. Nulling the + // pointer and zeroing the size should make + // incorrect usage much easier to spot than leaving + // them dangling. + .buffer = nullptr, + .size = 0}, .buffer = std::string(reinterpret_cast(event.buffer), - event.size), - }; + event.size)}; } break; case CLAP_EVENT_MIDI2: { const auto& event = reinterpret_cast(generic_event); - payload = payload::Midi2{ - .port_index = event.port_index, - .data{event.data[0], event.data[1], event.data[2], - event.data[3]}, - }; + payload = payload::Midi2{.event = event}; } break; } } if (payload) { - return Event{.time = generic_event.time, - .flags = generic_event.flags, - .payload = std::move(*payload)}; + return Event{.payload = std::move(*payload)}; } else { return std::nullopt; } diff --git a/src/common/serialization/clap/events.h b/src/common/serialization/clap/events.h index 91a396bd..c0e33389 100644 --- a/src/common/serialization/clap/events.h +++ b/src/common/serialization/clap/events.h @@ -23,48 +23,36 @@ #include #include "../bitsery/ext/in-place-variant.h" +#include "../bitsery/ext/native-pointer.h" #include "../common.h" namespace clap { namespace events { /** - * The actual event data. `clap::events::Event` stores these as a variant - * alongside the timestamp and flags for the original event header. + * The actual event data. `clap::events::Event` stores these as a variant. + * Ideally we'd store only the non-header payload data, but the + * `clap_input_events::get()` function requires us to return a pointer to the + * header, so if we did that then we'd need to create a second buffer containing + * the serialzed events. */ namespace payload { /** - * `Note` can be a variety of note events. - */ -enum class NoteEventType : uint8_t { - On, - Off, - Choke, - End, -}; - -/** - * The payload for `clap_event_note`. + * The payload for `clap_event_note`. This is used for multiple event types, + * which are encoded through `event.header.type`. */ struct Note { - NoteEventType event_type; - - int32_t note_id; - int16_t port_index; - int16_t channel; - int16_t key; - - double velocity; + clap_event_note_t event; template void serialize(S& s) { - s.value1b(event_type); - s.value4b(note_id); - s.value2b(port_index); - s.value2b(channel); - s.value2b(key); - s.value8b(velocity); + s.object(event.header); + s.value4b(event.note_id); + s.value2b(event.port_index); + s.value2b(event.channel); + s.value2b(event.key); + s.value8b(event.velocity); } }; @@ -72,23 +60,17 @@ struct Note { * The payload for `clap_event_note_expression`. */ struct NoteExpression { - clap_note_expression expression_id; - - int32_t note_id; - int16_t port_index; - int16_t channel; - int16_t key; - - double value; + clap_event_note_expression_t event; template void serialize(S& s) { - s.value4b(expression_id); - s.value4b(note_id); - s.value2b(port_index); - s.value2b(channel); - s.value2b(key); - s.value8b(value); + s.object(event.header); + s.value4b(event.expression_id); + s.value4b(event.note_id); + s.value2b(event.port_index); + s.value2b(event.channel); + s.value2b(event.key); + s.value8b(event.value); } }; @@ -96,30 +78,26 @@ struct NoteExpression { * The payload for `clap_event_param_value`. */ struct ParamValue { - clap_id param_id; - // This is a pointer. Using `native_size_t`/the host system's pointer size - // here will allow bridged 32-bit plugins to work correctly. - // XXX: This will silently blow up when using 32-bit yabridge on a 64-bit - // system with 64-bit plugins, but that's such a specific use case that - // we won't even bother. - native_size_t cookie; - - int32_t note_id; - int16_t port_index; - int16_t channel; - int16_t key; - - double value; + clap_event_param_value_t event; template void serialize(S& s) { - s.value4b(param_id); - s.value8b(cookie); - s.value4b(note_id); - s.value2b(port_index); - s.value2b(channel); - s.value2b(key); - s.value8b(value); + s.object(event.header); + s.value4b(event.param_id); + // The cookie is a pointer. Using `native_size_t`/the host system's + // pointer size here will allow bridged 32-bit plugins to work + // correctly. + // XXX: This will silently blow up when using 32-bit yabridge on a + // 64-bit system with 64-bit plugins, but that's such a specific + // use case that we won't even bother. Building 32-bit yabridge + // with CLAP support on 64-bit symbols has been disabled to prevent + // this from being an issue. + s.ext(event.cookie, bitsery::ext::NativePointer{}); + s.value4b(event.note_id); + s.value2b(event.port_index); + s.value2b(event.channel); + s.value2b(event.key); + s.value8b(event.value); } }; @@ -127,49 +105,33 @@ struct ParamValue { * The payload for `clap_event_param_mod`. */ struct ParamMod { - clap_id param_id; - // Same as above - native_size_t cookie; - - int32_t note_id; - int16_t port_index; - int16_t channel; - int16_t key; - - double amount; + clap_event_param_mod_t event; template void serialize(S& s) { - s.value4b(param_id); - s.value8b(cookie); - s.value4b(note_id); - s.value2b(port_index); - s.value2b(channel); - s.value2b(key); - s.value8b(amount); + s.object(event.header); + s.value4b(event.param_id); + // Same as the above + s.ext(event.cookie, bitsery::ext::NativePointer{}); + s.value4b(event.note_id); + s.value2b(event.port_index); + s.value2b(event.channel); + s.value2b(event.key); + s.value8b(event.amount); } }; /** - * `ParamGesture` can both be a `CLAP_EVENT_PARAM_GESTURE_BEGIN` and a - * `CLAP_EVENT_PARAM_GESTURE_END`. - */ -enum class ParamGestureType : uint8_t { - Begin, - End, -}; - -/** - * The payload for `clap_event_param_gesture`. + * The payload for `clap_event_param_gesture`. This is used for multiple event + * types, which are encoded through `event.header.type`. */ struct ParamGesture { - ParamGestureType gesture_type; - clap_id param_id; + clap_event_param_gesture_t event; template void serialize(S& s) { - s.value1b(gesture_type); - s.value4b(param_id); + s.object(event.header); + s.value4b(event.param_id); } }; @@ -177,40 +139,24 @@ struct ParamGesture { * The payload for `clap_event_transport`. */ struct Transport { - uint32_t flags; - - clap_beattime song_pos_beats; - clap_sectime song_pos_seconds; - - double tempo; - double tempo_inc; - - clap_beattime loop_start_beats; - clap_beattime loop_end_beats; - clap_sectime loop_start_seconds; - clap_sectime loop_end_seconds; - - clap_beattime bar_start; - int32_t bar_number; - - uint16_t tsig_num; - uint16_t tsig_denom; + clap_event_transport_t event; template void serialize(S& s) { - s.value4b(flags); - s.value8b(song_pos_beats); - s.value8b(song_pos_seconds); - s.value8b(tempo); - s.value8b(tempo_inc); - s.value8b(loop_start_beats); - s.value8b(loop_end_beats); - s.value8b(loop_start_seconds); - s.value8b(loop_end_seconds); - s.value8b(bar_start); - s.value4b(bar_number); - s.value2b(tsig_num); - s.value2b(tsig_denom); + s.object(event.header); + s.value4b(event.flags); + s.value8b(event.song_pos_beats); + s.value8b(event.song_pos_seconds); + s.value8b(event.tempo); + s.value8b(event.tempo_inc); + s.value8b(event.loop_start_beats); + s.value8b(event.loop_end_beats); + s.value8b(event.loop_start_seconds); + s.value8b(event.loop_end_seconds); + s.value8b(event.bar_start); + s.value4b(event.bar_number); + s.value2b(event.tsig_num); + s.value2b(event.tsig_denom); } }; @@ -218,13 +164,13 @@ struct Transport { * The payload for `clap_event_midi`. */ struct Midi { - uint16_t port_index; - uint8_t data[3]; + clap_event_midi_t event; template void serialize(S& s) { - s.value2b(port_index); - s.container4b(data); + s.object(event.header); + s.value2b(event.port_index); + s.container1b(event.data); } }; @@ -232,18 +178,36 @@ struct Midi { * The payload for `clap_event_midi_sysex`. */ struct MidiSysex { - uint16_t port_index; - // We're not expecting a lot of SysEx events, and `std::string`'s small - // string optimization should make it possible to send small sysex events - // without allocations. An alternative that won't allocate as quickly would - // be to store the data in a vector and to only store a tag here, but I - // don't think it's necessary at the moment. + clap_event_midi_sysex_t event; + + /** + * The actual SysEx event data. The pointer in `event` is set to the string + * data after the event has been created. As long as this event is not moved + * that pointer will remain valid. + * + * We're not expecting a lot of SysEx events, and `std::string`'s small + * string optimization should make it possible to send small sysex events + * without allocations. An alternative that won't allocate as quickly would + * be to store the data in a vector and to only store a tag here, but I + * don't think it's necessary at the moment. + */ std::string buffer; template void serialize(S& s) { - s.value2b(port_index); + s.object(event.header); + s.value2b(event.port_index); + s.text1b(buffer, 1 << 16); + + // NOTE: These will need to be set when retrieving the event using + // `clap_input_events::get()`. We could set the pointer here, but + // in the off chance that there are a lot more events than we can + // handle and the vector is reallocated to avoid dropping events, + // then these pointers would become dangling. Making sure these + // are null until the event is retrieved is probably for the best. + event.buffer = nullptr; + event.size = 0; } }; @@ -251,13 +215,13 @@ struct MidiSysex { * The payload for `clap_event_midi2`. */ struct Midi2 { - uint16_t port_index; - uint32_t data[4]; + clap_event_midi2_t event; template void serialize(S& s) { - s.value2b(port_index); - s.container4b(data); + s.object(event.header); + s.value2b(event.port_index); + s.container4b(event.data); } }; @@ -275,16 +239,13 @@ struct alignas(16) Event { static std::optional parse(const clap_event_header_t& generic_event); /** - * The time from the event header. - */ - uint32_t time; - /** - * The flags from the event header. - */ - uint32_t flags; - - /** - * The actual event data. This also encodes the type, size, and space ID. + * The actual event data. These also contain the header because storing the + * entire `clap_event_*_t` struct is the only way to serialize the event + * list in a way that doesn't require us to create a second event list in + * that format after deserializing the events. An alternative would be to + * write the event in the proper format to a buffer before returning it from + * `clap_input_events::get()`, but that would cause unexpected lifetime + * issues. */ std::variant void serialize(S& s) { - s.value4b(time); - s.value4b(flags); s.ext(payload, bitsery::ext::InPlaceVariant{}); } }; } // namespace events } // namespace clap + +template +void serialize(S& s, clap_event_header_t& event_header) { + // Feels a bit weird serializing this, but assuming the host/plugin set it + // correctly it will be fine. And this is kind of a host implementation + // detail for storing the events in a packed list anyways. + s.value4b(event_header.size); + s.value4b(event_header.time); + s.value2b(event_header.space_id); + s.value2b(event_header.type); + s.value4b(event_header.flags); +}