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

@@ -23,171 +23,84 @@ std::optional<Event> Event::parse(const clap_event_header_t& generic_event) {
     std::optional<decltype(Event::payload)> payload;
     if (generic_event.space_id == CLAP_CORE_EVENT_SPACE_ID) {
         switch (generic_event.type) {
-            case CLAP_EVENT_NOTE_ON: {
+            case CLAP_EVENT_NOTE_ON:
-                const auto& event =
+            case CLAP_EVENT_NOTE_OFF:
-                    reinterpret_cast<const clap_event_note_t&>(generic_event);
+            case CLAP_EVENT_NOTE_CHOKE:
-                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<const clap_event_note_t&>(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<const clap_event_note_t&>(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_END: {
                 const auto& event =
                     reinterpret_cast<const clap_event_note_t&>(generic_event);
-                payload = payload::Note{
+
-                    .event_type = payload::NoteEventType::End,
+                // The original event type can be restored from the header
-                    .note_id = event.note_id,
+                payload = payload::Note{.event = event};
-                    .port_index = event.port_index,
-                    .channel = event.channel,
-                    .key = event.key,
-                    .velocity = event.velocity,
-                };
             } break;
             case CLAP_EVENT_NOTE_EXPRESSION: {
                 const auto& event =
                     reinterpret_cast<const clap_event_note_expression_t&>(
                         generic_event);
-                payload = payload::NoteExpression{
+                payload = payload::NoteExpression{.event = event};
-                    .expression_id = event.expression_id,
-                    .note_id = event.note_id,
-                    .port_index = event.port_index,
-                    .channel = event.channel,
-                    .key = event.key,
-                    .value = event.value,
-                };
             } break;
             case CLAP_EVENT_PARAM_VALUE: {
                 const auto& event =
                     reinterpret_cast<const clap_event_param_value_t&>(
                         generic_event);
-                payload = payload::ParamValue{
+                payload = payload::ParamValue{.event = event};
-                    .param_id = event.param_id,
-                    .cookie = static_cast<native_size_t>(
-                        reinterpret_cast<size_t>(event.cookie)),
-                    .note_id = event.note_id,
-                    .port_index = event.port_index,
-                    .channel = event.channel,
-                    .key = event.key,
-                    .value = event.value,
-                };
             } break;
             case CLAP_EVENT_PARAM_MOD: {
                 const auto& event =
                     reinterpret_cast<const clap_event_param_mod_t&>(
                         generic_event);
-                payload = payload::ParamMod{
+                payload = payload::ParamMod{.event = event};
-                    .param_id = event.param_id,
-                    .cookie = static_cast<native_size_t>(
-                        reinterpret_cast<size_t>(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<const clap_event_param_gesture_t&>(
-                        generic_event);
-                payload = payload::ParamGesture{
-                    .gesture_type = payload::ParamGestureType::Begin,
-                    .param_id = event.param_id,
-                };
             } break;
+            case CLAP_EVENT_PARAM_GESTURE_BEGIN:
             case CLAP_EVENT_PARAM_GESTURE_END: {
                 const auto& event =
                     reinterpret_cast<const clap_event_param_gesture_t&>(
                         generic_event);
-                payload = payload::ParamGesture{
+                payload = payload::ParamGesture{.event = event};
-                    .gesture_type = payload::ParamGestureType::End,
-                    .param_id = event.param_id,
-                };
             } break;
             case CLAP_EVENT_TRANSPORT: {
                 const auto& event =
                     reinterpret_cast<const clap_event_transport_t&>(
                         generic_event);
-                payload = payload::Transport{
+                payload = payload::Transport{.event = event};
-                    .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,
-                };
             } break;
             case CLAP_EVENT_MIDI: {
                 const auto& event =
                     reinterpret_cast<const clap_event_midi_t&>(generic_event);
-                payload = payload::Midi{
+                payload = payload::Midi{.event = event};
-                    .port_index = event.port_index,
-                    .data{event.data[0], event.data[1], event.data[2]},
-                };
             } break;
             case CLAP_EVENT_MIDI_SYSEX: {
                 const auto& event =
                     reinterpret_cast<const clap_event_midi_sysex_t&>(
                         generic_event);
+
                 assert(event.buffer);
-                payload = payload::MidiSysex{
+                const auto sysex_payload = payload::MidiSysex{
-                    .port_index = event.port_index,
+                    .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<const char*>(event.buffer),
-                                    event.size),
+                                    event.size)};
-                };
             } break;
             case CLAP_EVENT_MIDI2: {
                 const auto& event =
                     reinterpret_cast<const clap_event_midi2_t&>(generic_event);
-                payload = payload::Midi2{
+                payload = payload::Midi2{.event = event};
-                    .port_index = event.port_index,
-                    .data{event.data[0], event.data[1], event.data[2],
-                          event.data[3]},
-                };
             } break;
         }
     }
 
     if (payload) {
-        return Event{.time = generic_event.time,
+        return Event{.payload = std::move(*payload)};
-                     .flags = generic_event.flags,
-                     .payload = std::move(*payload)};
     } else {
         return std::nullopt;
     }

src/common/serialization/clap/events.h

@@ -23,48 +23,36 @@
 #include <clap/events.h>
 
 #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
+ * The actual event data. `clap::events::Event` stores these as a variant.
- * alongside the timestamp and flags for the original event header.
+ * 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.
+ * The payload for `clap_event_note`. This is used for multiple event types,
- */
+ * which are encoded through `event.header.type`.
-enum class NoteEventType : uint8_t {
-    On,
-    Off,
-    Choke,
-    End,
-};
-
-/**
- * The payload for `clap_event_note`.
  */
 struct Note {
-    NoteEventType event_type;
+    clap_event_note_t event;
-
-    int32_t note_id;
-    int16_t port_index;
-    int16_t channel;
-    int16_t key;
-
-    double velocity;
 
     template <typename S>
     void serialize(S& s) {
-        s.value1b(event_type);
+        s.object(event.header);
-        s.value4b(note_id);
+        s.value4b(event.note_id);
-        s.value2b(port_index);
+        s.value2b(event.port_index);
-        s.value2b(channel);
+        s.value2b(event.channel);
-        s.value2b(key);
+        s.value2b(event.key);
-        s.value8b(velocity);
+        s.value8b(event.velocity);
     }
 };
 
@@ -72,23 +60,17 @@ struct Note {
  * The payload for `clap_event_note_expression`.
  */
 struct NoteExpression {
-    clap_note_expression expression_id;
+    clap_event_note_expression_t event;
-
-    int32_t note_id;
-    int16_t port_index;
-    int16_t channel;
-    int16_t key;
-
-    double value;
 
     template <typename S>
     void serialize(S& s) {
-        s.value4b(expression_id);
+        s.object(event.header);
-        s.value4b(note_id);
+        s.value4b(event.expression_id);
-        s.value2b(port_index);
+        s.value4b(event.note_id);
-        s.value2b(channel);
+        s.value2b(event.port_index);
-        s.value2b(key);
+        s.value2b(event.channel);
-        s.value8b(value);
+        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;
+    clap_event_param_value_t event;
-    // 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;
 
     template <typename S>
     void serialize(S& s) {
-        s.value4b(param_id);
+        s.object(event.header);
-        s.value8b(cookie);
+        s.value4b(event.param_id);
-        s.value4b(note_id);
+        // The cookie is a pointer. Using `native_size_t`/the host system's
-        s.value2b(port_index);
+        // pointer size here will allow bridged 32-bit plugins to work
-        s.value2b(channel);
+        // correctly.
-        s.value2b(key);
+        // XXX: This will silently blow up when using 32-bit yabridge on a
-        s.value8b(value);
+        //      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;
+    clap_event_param_mod_t event;
-    // Same as above
-    native_size_t cookie;
-
-    int32_t note_id;
-    int16_t port_index;
-    int16_t channel;
-    int16_t key;
-
-    double amount;
 
     template <typename S>
     void serialize(S& s) {
-        s.value4b(param_id);
+        s.object(event.header);
-        s.value8b(cookie);
+        s.value4b(event.param_id);
-        s.value4b(note_id);
+        // Same as the above
-        s.value2b(port_index);
+        s.ext(event.cookie, bitsery::ext::NativePointer{});
-        s.value2b(channel);
+        s.value4b(event.note_id);
-        s.value2b(key);
+        s.value2b(event.port_index);
-        s.value8b(amount);
+        s.value2b(event.channel);
+        s.value2b(event.key);
+        s.value8b(event.amount);
     }
 };
 
 /**
- * `ParamGesture` can both be a `CLAP_EVENT_PARAM_GESTURE_BEGIN` and a
+ * The payload for `clap_event_param_gesture`. This is used for multiple event
- * `CLAP_EVENT_PARAM_GESTURE_END`.
+ * types, which are encoded through `event.header.type`.
- */
-enum class ParamGestureType : uint8_t {
-    Begin,
-    End,
-};
-
-/**
- * The payload for `clap_event_param_gesture`.
  */
 struct ParamGesture {
-    ParamGestureType gesture_type;
+    clap_event_param_gesture_t event;
-    clap_id param_id;
 
     template <typename S>
     void serialize(S& s) {
-        s.value1b(gesture_type);
+        s.object(event.header);
-        s.value4b(param_id);
+        s.value4b(event.param_id);
     }
 };
 
@@ -177,40 +139,24 @@ struct ParamGesture {
  * The payload for `clap_event_transport`.
  */
 struct Transport {
-    uint32_t flags;
+    clap_event_transport_t event;
-
-    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;
 
     template <typename S>
     void serialize(S& s) {
-        s.value4b(flags);
+        s.object(event.header);
-        s.value8b(song_pos_beats);
+        s.value4b(event.flags);
-        s.value8b(song_pos_seconds);
+        s.value8b(event.song_pos_beats);
-        s.value8b(tempo);
+        s.value8b(event.song_pos_seconds);
-        s.value8b(tempo_inc);
+        s.value8b(event.tempo);
-        s.value8b(loop_start_beats);
+        s.value8b(event.tempo_inc);
-        s.value8b(loop_end_beats);
+        s.value8b(event.loop_start_beats);
-        s.value8b(loop_start_seconds);
+        s.value8b(event.loop_end_beats);
-        s.value8b(loop_end_seconds);
+        s.value8b(event.loop_start_seconds);
-        s.value8b(bar_start);
+        s.value8b(event.loop_end_seconds);
-        s.value4b(bar_number);
+        s.value8b(event.bar_start);
-        s.value2b(tsig_num);
+        s.value4b(event.bar_number);
-        s.value2b(tsig_denom);
+        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;
+    clap_event_midi_t event;
-    uint8_t data[3];
 
     template <typename S>
     void serialize(S& s) {
-        s.value2b(port_index);
+        s.object(event.header);
-        s.container4b(data);
+        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;
+    clap_event_midi_sysex_t event;
-    // 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
+     * The actual SysEx event data. The pointer in `event` is set to the string
-    // be to store the data in a vector and to only store a tag here, but I
+     * data after the event has been created. As long as this event is not moved
-    // don't think it's necessary at the moment.
+     * 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 <typename S>
     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;
+    clap_event_midi2_t event;
-    uint32_t data[4];
 
     template <typename S>
     void serialize(S& s) {
-        s.value2b(port_index);
+        s.object(event.header);
-        s.container4b(data);
+        s.value2b(event.port_index);
+        s.container4b(event.data);
     }
 };
 
@@ -275,16 +239,13 @@ struct alignas(16) Event {
     static std::optional<Event> parse(const clap_event_header_t& generic_event);
 
     /**
-     * The time from the event header.
+     * 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
-    uint32_t time;
+     * 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
-     * The flags from the event header.
+     * write the event in the proper format to a buffer before returning it from
-     */
+     * `clap_input_events::get()`, but that would cause unexpected lifetime
-    uint32_t flags;
+     * issues.
-
-    /**
-     * The actual event data. This also encodes the type, size, and space ID.
      */
     std::variant<payload::Note,
                  payload::NoteExpression,
@@ -303,11 +264,21 @@ struct alignas(16) Event {
 
     template <typename S>
     void serialize(S& s) {
-        s.value4b(time);
-        s.value4b(flags);
         s.ext(payload, bitsery::ext::InPlaceVariant{});
     }
 };
 
 }  // namespace events
 }  // namespace clap
+
+template <typename S>
+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);
+}