From 68d68c0bc3cc662fb1c842be273ef759aee7f30a Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Tue, 31 Jan 2023 20:37:33 +0100
Subject: [PATCH] Add NoteEvent conversions to and from MIDI SysEx

JACK already supports this because otherwise things wouldn't compile,
but support still needs to be added for CLAP and VST3.
---
 src/midi.rs                            | 259 ++++++++++++++-----------
 src/midi/sysex.rs                      |  26 +--
 src/wrapper/standalone/backend/jack.rs |  37 +++-
 3 files changed, 191 insertions(+), 131 deletions(-)

diff --git a/src/midi.rs b/src/midi.rs
index 4d8cd1b2..f522d9eb 100644
--- a/src/midi.rs
+++ b/src/midi.rs
@@ -318,6 +318,18 @@ pub enum NoteEvent<S: SysExMessage> {
     MidiSysEx { timing: u32, message: S },
 }
 
+/// The result of converting a `NoteEvent<S>` to MIDI. This is a bit weirder than it would have to
+/// be because it's not possible to use associated constants in type definitions.
+#[derive(Debug, Clone)]
+pub enum MidiResult {
+    /// A basic three byte MIDI event.
+    Basic([u8; 3]),
+    /// A SysEx event. The message was written to the buffer provided to [`NoteEvent::as_midi()`].
+    /// The `usize` value indicates the message's actual length, including headers and end of SysEx
+    /// byte.
+    SysEx(usize),
+}
+
 impl<S: SysExMessage> NoteEvent<S> {
     /// Returns the sample within the current buffer this event belongs to.
     pub fn timing(&self) -> u32 {
@@ -367,78 +379,109 @@ impl<S: SysExMessage> NoteEvent<S> {
         }
     }
 
-    // TODO: `[u8; 3]` doesn't work anymore with SysEx. We can wrap this in an
-    //       `enum MidiBuffer<P> { simple: [u8; 3], sysex: P::SysExMessage::Buffer }`.
+    /// Parse MIDI into a [`NoteEvent`]. Supports both basic three bytes messages as well as SysEx.
+    /// Will return `Err(event_type)` if the parsing failed.
+    pub fn from_midi(timing: u32, midi_data: &[u8]) -> Result<Self, u8> {
+        let status_byte = midi_data.first().copied().unwrap_or_default();
+        let event_type = status_byte & midi::EVENT_TYPE_MASK;
 
-    /// Parse MIDI into a [`NoteEvent`]. Will return `Err(event_type)` if the parsing failed.
-    pub fn from_midi(timing: u32, midi_data: [u8; 3]) -> Result<Self, u8> {
-        // TODO: Maybe add special handling for 14-bit CCs and RPN messages at some
-        //       point, right now the plugin has to figure it out for itself
-        let event_type = midi_data[0] & midi::EVENT_TYPE_MASK;
-        let channel = midi_data[0] & midi::MIDI_CHANNEL_MASK;
-        match event_type {
-            // You thought this was a note on? Think again! This is a cleverly disguised note off
-            // event straight from the 80s when Baud rate was still a limiting factor!
-            midi::NOTE_ON if midi_data[2] == 0 => Ok(NoteEvent::NoteOff {
-                timing,
-                voice_id: None,
-                channel,
-                note: midi_data[1],
-                // Few things use release velocity. Just having this be zero here is fine, right?
-                velocity: 0.0,
-            }),
-            midi::NOTE_ON => Ok(NoteEvent::NoteOn {
-                timing,
-                voice_id: None,
-                channel,
-                note: midi_data[1],
-                velocity: midi_data[2] as f32 / 127.0,
-            }),
-            midi::NOTE_OFF => Ok(NoteEvent::NoteOff {
-                timing,
-                voice_id: None,
-                channel,
-                note: midi_data[1],
-                velocity: midi_data[2] as f32 / 127.0,
-            }),
-            midi::POLYPHONIC_KEY_PRESSURE => Ok(NoteEvent::PolyPressure {
-                timing,
-                voice_id: None,
-                channel,
-                note: midi_data[1],
-                pressure: midi_data[2] as f32 / 127.0,
-            }),
-            midi::CHANNEL_KEY_PRESSURE => Ok(NoteEvent::MidiChannelPressure {
-                timing,
-                channel,
-                pressure: midi_data[1] as f32 / 127.0,
-            }),
-            midi::PITCH_BEND_CHANGE => Ok(NoteEvent::MidiPitchBend {
-                timing,
-                channel,
-                value: (midi_data[1] as u16 + ((midi_data[2] as u16) << 7)) as f32
-                    / ((1 << 14) - 1) as f32,
-            }),
-            midi::CONTROL_CHANGE => Ok(NoteEvent::MidiCC {
-                timing,
-                channel,
-                cc: midi_data[1],
-                value: midi_data[2] as f32 / 127.0,
-            }),
-            midi::PROGRAM_CHANGE => Ok(NoteEvent::MidiProgramChange {
-                timing,
-                channel,
-                program: midi_data[1],
-            }),
-            // TODO: SysEx
-            n => Err(n),
+        if midi_data.len() >= 3 {
+            // TODO: Maybe add special handling for 14-bit CCs and RPN messages at some
+            //       point, right now the plugin has to figure it out for itself
+            let channel = status_byte & midi::MIDI_CHANNEL_MASK;
+            match event_type {
+                // You thought this was a note on? Think again! This is a cleverly disguised note off
+                // event straight from the 80s when Baud rate was still a limiting factor!
+                midi::NOTE_ON if midi_data[2] == 0 => {
+                    return Ok(NoteEvent::NoteOff {
+                        timing,
+                        voice_id: None,
+                        channel,
+                        note: midi_data[1],
+                        // Few things use release velocity. Just having this be zero here is fine, right?
+                        velocity: 0.0,
+                    });
+                }
+                midi::NOTE_ON => {
+                    return Ok(NoteEvent::NoteOn {
+                        timing,
+                        voice_id: None,
+                        channel,
+                        note: midi_data[1],
+                        velocity: midi_data[2] as f32 / 127.0,
+                    });
+                }
+                midi::NOTE_OFF => {
+                    return Ok(NoteEvent::NoteOff {
+                        timing,
+                        voice_id: None,
+                        channel,
+                        note: midi_data[1],
+                        velocity: midi_data[2] as f32 / 127.0,
+                    });
+                }
+                midi::POLYPHONIC_KEY_PRESSURE => {
+                    return Ok(NoteEvent::PolyPressure {
+                        timing,
+                        voice_id: None,
+                        channel,
+                        note: midi_data[1],
+                        pressure: midi_data[2] as f32 / 127.0,
+                    });
+                }
+                midi::CHANNEL_KEY_PRESSURE => {
+                    return Ok(NoteEvent::MidiChannelPressure {
+                        timing,
+                        channel,
+                        pressure: midi_data[1] as f32 / 127.0,
+                    });
+                }
+                midi::PITCH_BEND_CHANGE => {
+                    return Ok(NoteEvent::MidiPitchBend {
+                        timing,
+                        channel,
+                        value: (midi_data[1] as u16 + ((midi_data[2] as u16) << 7)) as f32
+                            / ((1 << 14) - 1) as f32,
+                    });
+                }
+                midi::CONTROL_CHANGE => {
+                    return Ok(NoteEvent::MidiCC {
+                        timing,
+                        channel,
+                        cc: midi_data[1],
+                        value: midi_data[2] as f32 / 127.0,
+                    });
+                }
+                midi::PROGRAM_CHANGE => {
+                    return Ok(NoteEvent::MidiProgramChange {
+                        timing,
+                        channel,
+                        program: midi_data[1],
+                    });
+                }
+                _ => (),
+            }
+        }
+
+        // Every other message is parsed as SysEx, even if they don't have the `0xf0` status byte.
+        // This allows the `SysExMessage` trait to have a bit more flexibility if needed. Regular
+        // note event parsing however still has higher priority.
+        match S::from_buffer(midi_data) {
+            Some(message) => Ok(NoteEvent::MidiSysEx { timing, message }),
+            None => Err(event_type),
         }
     }
 
-    /// Create a MIDI message from this note event. Return `None` if this even does not have a
+    /// Create a MIDI message from this note event. Returns `None` if this even does not have a
     /// direct MIDI equivalent. `PolyPressure` will be converted to polyphonic key pressure, but the
     /// other polyphonic note expression types will not be converted to MIDI CC messages.
-    pub fn as_midi(self) -> Option<[u8; 3]> {
+    ///
+    /// The `sysex_buffer` is an `[u8; N]` buffer with a length depending on SysEx message type `S`.
+    /// If this event contained SysEx data, then the result is written to the buffer and the
+    /// message's length in bytes is returned. This weird approach is needed because it's not
+    /// possible to use associated constants in types. Otherwise the buffer could be stored in
+    /// `MidiResult`.
+    pub fn as_midi(self, sysex_buffer: &mut S::Buffer) -> Option<MidiResult> {
         match self {
             NoteEvent::NoteOn {
                 timing: _,
@@ -446,42 +489,42 @@ impl<S: SysExMessage> NoteEvent<S> {
                 channel,
                 note,
                 velocity,
-            } => Some([
+            } => Some(MidiResult::Basic([
                 midi::NOTE_ON | channel,
                 note,
                 (velocity * 127.0).round().clamp(0.0, 127.0) as u8,
-            ]),
+            ])),
             NoteEvent::NoteOff {
                 timing: _,
                 voice_id: _,
                 channel,
                 note,
                 velocity,
-            } => Some([
+            } => Some(MidiResult::Basic([
                 midi::NOTE_OFF | channel,
                 note,
                 (velocity * 127.0).round().clamp(0.0, 127.0) as u8,
-            ]),
+            ])),
             NoteEvent::PolyPressure {
                 timing: _,
                 voice_id: _,
                 channel,
                 note,
                 pressure,
-            } => Some([
+            } => Some(MidiResult::Basic([
                 midi::POLYPHONIC_KEY_PRESSURE | channel,
                 note,
                 (pressure * 127.0).round().clamp(0.0, 127.0) as u8,
-            ]),
+            ])),
             NoteEvent::MidiChannelPressure {
                 timing: _,
                 channel,
                 pressure,
-            } => Some([
+            } => Some(MidiResult::Basic([
                 midi::CHANNEL_KEY_PRESSURE | channel,
                 (pressure * 127.0).round().clamp(0.0, 127.0) as u8,
                 0,
-            ]),
+            ])),
             NoteEvent::MidiPitchBend {
                 timing: _,
                 channel,
@@ -492,27 +535,36 @@ impl<S: SysExMessage> NoteEvent<S> {
                     .round()
                     .clamp(0.0, PITCH_BEND_RANGE) as u16;
 
-                Some([
+                Some(MidiResult::Basic([
                     midi::PITCH_BEND_CHANGE | channel,
                     (midi_value & ((1 << 7) - 1)) as u8,
                     (midi_value >> 7) as u8,
-                ])
+                ]))
             }
             NoteEvent::MidiCC {
                 timing: _,
                 channel,
                 cc,
                 value,
-            } => Some([
+            } => Some(MidiResult::Basic([
                 midi::CONTROL_CHANGE | channel,
                 cc,
                 (value * 127.0).round().clamp(0.0, 127.0) as u8,
-            ]),
+            ])),
             NoteEvent::MidiProgramChange {
                 timing: _,
                 channel,
                 program,
-            } => Some([midi::PROGRAM_CHANGE | channel, program, 0]),
+            } => Some(MidiResult::Basic([
+                midi::PROGRAM_CHANGE | channel,
+                program,
+                0,
+            ])),
+            // `message` is serialized and written to `sysex_buffer`, and the result contains the
+            // message's actual length
+            NoteEvent::MidiSysEx { timing: _, message } => {
+                Some(MidiResult::SysEx(message.to_buffer(sysex_buffer)))
+            }
             NoteEvent::Choke { .. }
             | NoteEvent::VoiceTerminated { .. }
             | NoteEvent::PolyModulation { .. }
@@ -523,8 +575,6 @@ impl<S: SysExMessage> NoteEvent<S> {
             | NoteEvent::PolyVibrato { .. }
             | NoteEvent::PolyExpression { .. }
             | NoteEvent::PolyBrightness { .. } => None,
-            // TODO: These functions need to handle both simple and longer messages as documented above
-            NoteEvent::MidiSysEx { .. } => None,
         }
     }
 
@@ -560,6 +610,16 @@ mod tests {
 
     const TIMING: u32 = 5;
 
+    /// Converts an event to and from MIDI. Panics if any part of the conversion fails.
+    fn roundtrip_basic_event(event: NoteEvent<()>) -> NoteEvent<()> {
+        let midi_data = match event.as_midi(&mut Default::default()).unwrap() {
+            MidiResult::Basic(midi_data) => midi_data,
+            MidiResult::SysEx(_) => panic!("Unexpected SysEx result"),
+        };
+
+        NoteEvent::from_midi(TIMING, &midi_data).unwrap()
+    }
+
     #[test]
     fn test_note_on_midi_conversion() {
         let event = NoteEvent::<()>::NoteOn {
@@ -571,10 +631,7 @@ mod tests {
             velocity: 0.6929134,
         };
 
-        assert_eq!(
-            NoteEvent::from_midi(TIMING, event.as_midi().unwrap()).unwrap(),
-            event
-        );
+        assert_eq!(roundtrip_basic_event(event), event);
     }
 
     #[test]
@@ -587,10 +644,7 @@ mod tests {
             velocity: 0.6929134,
         };
 
-        assert_eq!(
-            NoteEvent::from_midi(TIMING, event.as_midi().unwrap()).unwrap(),
-            event
-        );
+        assert_eq!(roundtrip_basic_event(event), event);
     }
 
     #[test]
@@ -603,10 +657,7 @@ mod tests {
             pressure: 0.6929134,
         };
 
-        assert_eq!(
-            NoteEvent::from_midi(TIMING, event.as_midi().unwrap()).unwrap(),
-            event
-        );
+        assert_eq!(roundtrip_basic_event(event), event);
     }
 
     #[test]
@@ -617,10 +668,7 @@ mod tests {
             pressure: 0.6929134,
         };
 
-        assert_eq!(
-            NoteEvent::from_midi(TIMING, event.as_midi().unwrap()).unwrap(),
-            event
-        );
+        assert_eq!(roundtrip_basic_event(event), event);
     }
 
     #[test]
@@ -631,10 +679,7 @@ mod tests {
             value: 0.6929134,
         };
 
-        assert_eq!(
-            NoteEvent::from_midi(TIMING, event.as_midi().unwrap()).unwrap(),
-            event
-        );
+        assert_eq!(roundtrip_basic_event(event), event);
     }
 
     #[test]
@@ -646,10 +691,7 @@ mod tests {
             value: 0.6929134,
         };
 
-        assert_eq!(
-            NoteEvent::from_midi(TIMING, event.as_midi().unwrap()).unwrap(),
-            event
-        );
+        assert_eq!(roundtrip_basic_event(event), event);
     }
 
     #[test]
@@ -660,11 +702,8 @@ mod tests {
             program: 42,
         };
 
-        assert_eq!(
-            NoteEvent::from_midi(TIMING, event.as_midi().unwrap()).unwrap(),
-            event
-        );
+        assert_eq!(roundtrip_basic_event(event), event);
     }
 
-    // TODO: SysEx conversion
+    // TODO: Test SysEx conversion
 }
diff --git a/src/midi/sysex.rs b/src/midi/sysex.rs
index e34d0319..66c5d118 100644
--- a/src/midi/sysex.rs
+++ b/src/midi/sysex.rs
@@ -1,5 +1,6 @@
 //! Traits for working with MIDI SysEx data.
 
+use std::borrow::{Borrow, BorrowMut};
 use std::fmt::Debug;
 
 /// A type that can be converted to and from byte buffers containing MIDI SysEx messages.
@@ -14,35 +15,36 @@ use std::fmt::Debug;
 /// For example, the message to turn general MIDI mode on is `[0xf0, 0x7e, 0x7f, 0x09, 0x01, 0xf7]`,
 /// and has a length of 6 bytes. Note that this includes the `0xf0` start byte and `0xf7` end byte.
 pub trait SysExMessage: Debug + Clone + PartialEq + Send + Sync {
-    /// The maximum SysEx message size, in bytes. This covers the full message, see the trait's
-    /// docstring for more information.
-    const MAX_BUFFER_SIZE: usize;
+    /// The byte array buffer the messages are read from and serialized to. Should be a `[u8; N]`,
+    /// where `N` is the maximum supported message length in bytes. This covers the full message,
+    /// see the trait's docstring for more information.
+    ///
+    /// Ideally this could just be a const generic but Rust doesn't let you use those as array
+    /// lengths just yet.
+    ///
+    /// <https://github.com/rust-lang/rust/issues/60551>
+    type Buffer: Default + Borrow<[u8]> + BorrowMut<[u8]>;
 
     /// Read a SysEx message from `buffer` and convert it to this message type if supported. This
     /// covers the full message, see the trait's docstring for more information. `buffer`'s length
-    /// matches the received message. It is not padded to `MAX_BUFFER_SIZE` bytes.
+    /// matches the received message. It is not padded to match [`Buffer`][Self::Buffer].
     fn from_buffer(buffer: &[u8]) -> Option<Self>;
 
     /// Serialize this message object as a SysEx message in `buffer`, returning the message's length
     /// in bytes. This should contain the full message including headers and the EOX byte, see the
     /// trait's docstring for more information.
-    ///
-    /// `buffer` is a `[u8; Self::MAX_BUFFER_SIZE]`, but Rust currently doesn't allow using
-    /// associated constants in method types:
-    ///
-    /// <https://github.com/rust-lang/rust/issues/60551>
-    fn to_buffer(self, buffer: &mut [u8]) -> usize;
+    fn to_buffer(self, buffer: &mut Self::Buffer) -> usize;
 }
 
 /// A default implementation plugins that don't need SysEx support can use.
 impl SysExMessage for () {
-    const MAX_BUFFER_SIZE: usize = 0;
+    type Buffer = [u8; 0];
 
     fn from_buffer(_buffer: &[u8]) -> Option<Self> {
         None
     }
 
-    fn to_buffer(self, _buffer: &mut [u8]) -> usize {
+    fn to_buffer(self, _buffer: &mut [u8; 0]) -> usize {
         0
     }
 }
diff --git a/src/wrapper/standalone/backend/jack.rs b/src/wrapper/standalone/backend/jack.rs
index aab53c82..f90018cc 100644
--- a/src/wrapper/standalone/backend/jack.rs
+++ b/src/wrapper/standalone/backend/jack.rs
@@ -1,3 +1,4 @@
+use std::borrow::Borrow;
 use std::sync::Arc;
 
 use anyhow::{Context, Result};
@@ -12,7 +13,7 @@ use super::super::config::WrapperConfig;
 use super::Backend;
 use crate::buffer::Buffer;
 use crate::context::process::Transport;
-use crate::midi::{MidiConfig, NoteEvent, PluginNoteEvent};
+use crate::midi::{MidiConfig, MidiResult, NoteEvent, PluginNoteEvent};
 use crate::plugin::Plugin;
 
 /// Uses JACK audio and MIDI.
@@ -129,14 +130,13 @@ impl<P: Plugin> Backend<P> for Jack {
                         let mut midi_data = [0u8; 3];
                         midi_data[..midi.bytes.len()].copy_from_slice(midi.bytes);
 
-                        NoteEvent::from_midi(midi.time, midi_data).ok()
+                        NoteEvent::from_midi(midi.time, &midi_data).ok()
                     } else {
                         None
                     }
                 }));
             }
 
-            // TODO: Support SysEx
             output_events.clear();
             if cb(&mut buffer, transport, &input_events, &mut output_events) {
                 if let Some(midi_output) = &midi_output {
@@ -144,12 +144,31 @@ impl<P: Plugin> Backend<P> for Jack {
                     let mut midi_writer = midi_output.writer(ps);
                     for event in output_events.drain(..) {
                         let timing = event.timing();
-                        if let Some(midi_data) = event.as_midi() {
-                            let write_result = midi_writer.write(&jack::RawMidi {
-                                time: timing,
-                                bytes: &midi_data,
-                            });
-                            nih_debug_assert!(write_result.is_ok(), "The MIDI buffer is full");
+
+                        let mut sysex_buffer = Default::default();
+                        match event.as_midi(&mut sysex_buffer) {
+                            Some(MidiResult::Basic(midi_data)) => {
+                                let write_result = midi_writer.write(&jack::RawMidi {
+                                    time: timing,
+                                    bytes: &midi_data,
+                                });
+
+                                nih_debug_assert!(write_result.is_ok(), "The MIDI buffer is full");
+                            }
+                            Some(MidiResult::SysEx(length)) => {
+                                // This feels a bit like gymnastics, but if the event was a SysEx
+                                // event then `sysex_buffer` now contains the full message plus
+                                // possibly some padding at the end
+                                let sysex_buffer = sysex_buffer.borrow();
+                                nih_debug_assert!(length <= sysex_buffer.len());
+                                let write_result = midi_writer.write(&jack::RawMidi {
+                                    time: timing,
+                                    bytes: &sysex_buffer[..length],
+                                });
+
+                                nih_debug_assert!(write_result.is_ok(), "The MIDI buffer is full");
+                            }
+                            None => (),
                         }
                     }
                 }