From 5dbc76ef6942464283af834eaaa5412d39593c6f Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 31 Jan 2023 22:03:40 +0100 Subject: [PATCH] Simplify conversion from SysExMessage to buffers There's no need to provide this as an argument anymore. --- src/midi.rs | 39 ++++++++++---------------- src/midi/sysex.rs | 13 +++++---- src/wrapper/clap/wrapper.rs | 14 ++++----- src/wrapper/standalone/backend/jack.rs | 15 ++++------ src/wrapper/vst3/wrapper.rs | 10 +++---- 5 files changed, 38 insertions(+), 53 deletions(-) diff --git a/src/midi.rs b/src/midi.rs index 097ae66e..83e9fbcc 100644 --- a/src/midi.rs +++ b/src/midi.rs @@ -321,13 +321,13 @@ pub enum NoteEvent { /// The result of converting a `NoteEvent` 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 { +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), + /// A SysEx event. The message was written to the `S::Buffer` and may include padding at the + /// end. The `usize` value indicates the message's actual length, including headers and end of + /// SysEx byte. + SysEx(S::Buffer, usize), } impl NoteEvent { @@ -487,13 +487,7 @@ impl NoteEvent { /// 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. - /// - /// 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 { + pub fn as_midi(self) -> Option> { match self { NoteEvent::NoteOn { timing: _, @@ -575,7 +569,8 @@ impl NoteEvent { // `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))) + let (padded_sysex_buffer, length) = message.to_buffer(); + Some(MidiResult::SysEx(padded_sysex_buffer, length)) } NoteEvent::Choke { .. } | NoteEvent::VoiceTerminated { .. } @@ -624,9 +619,9 @@ mod tests { /// 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() { + let midi_data = match event.as_midi().unwrap() { MidiResult::Basic(midi_data) => midi_data, - MidiResult::SysEx(_) => panic!("Unexpected SysEx result"), + MidiResult::SysEx(_, _) => panic!("Unexpected SysEx result"), }; NoteEvent::from_midi(TIMING, &midi_data).unwrap() @@ -735,12 +730,9 @@ mod tests { } } - fn to_buffer(self, buffer: &mut Self::Buffer) -> usize { + fn to_buffer(self) -> (Self::Buffer, usize) { match self { - MessageType::Foo(x) => { - *buffer = [0xf0, 0x69, (x * 127.0).round() as u8, 0xf7]; - 4 - } + MessageType::Foo(x) => ([0xf0, 0x69, (x * 127.0).round() as u8, 0xf7], 4), } } } @@ -767,10 +759,9 @@ mod tests { message, }; - let mut sysex_buffer = [0; 4]; - match event.as_midi(&mut sysex_buffer) { - Some(MidiResult::SysEx(length)) => { - assert_eq!(sysex_buffer[..length], [0xf0, 0x69, 127, 0xf7]) + match event.as_midi() { + Some(MidiResult::SysEx(padded_sysex_buffer, length)) => { + assert_eq!(padded_sysex_buffer[..length], [0xf0, 0x69, 127, 0xf7]) } result => panic!("Unexpected result: {result:?}"), } diff --git a/src/midi/sysex.rs b/src/midi/sysex.rs index 66c5d118..dfd39c1e 100644 --- a/src/midi/sysex.rs +++ b/src/midi/sysex.rs @@ -30,10 +30,11 @@ pub trait SysExMessage: Debug + Clone + PartialEq + Send + Sync { /// matches the received message. It is not padded to match [`Buffer`][Self::Buffer]. fn from_buffer(buffer: &[u8]) -> Option; - /// 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. - fn to_buffer(self, buffer: &mut Self::Buffer) -> usize; + /// Serialize this message object as a SysEx message in a byte buffer. This returns a buffer + /// alongside the message's length in bytes. The buffer may contain padding at the end. This + /// should contain the full message including headers and the EOX byte, see the trait's + /// docstring for more information. + fn to_buffer(self) -> (Self::Buffer, usize); } /// A default implementation plugins that don't need SysEx support can use. @@ -44,7 +45,7 @@ impl SysExMessage for () { None } - fn to_buffer(self, _buffer: &mut [u8; 0]) -> usize { - 0 + fn to_buffer(self) -> (Self::Buffer, usize) { + ([], 0) } } diff --git a/src/wrapper/clap/wrapper.rs b/src/wrapper/clap/wrapper.rs index 4a814aae..a5b598a3 100644 --- a/src/wrapper/clap/wrapper.rs +++ b/src/wrapper/clap/wrapper.rs @@ -1317,9 +1317,9 @@ impl Wrapper

{ { // NIH-plug already includes MIDI conversion functions, so we'll reuse those for // the MIDI events - let midi_data = match midi_event.as_midi(&mut Default::default()) { + let midi_data = match midi_event.as_midi() { Some(MidiResult::Basic(midi_data)) => midi_data, - Some(MidiResult::SysEx(_)) => unreachable!( + Some(MidiResult::SysEx(_, _)) => unreachable!( "Basic MIDI event read as SysEx, something's gone horribly wrong" ), None => unreachable!("Missing MIDI conversion for MIDI event"), @@ -1343,12 +1343,10 @@ impl Wrapper

{ if P::MIDI_OUTPUT >= MidiConfig::Basic => { // SysEx is supported on the basic MIDI config so this is separate - let mut sysex_buffer = Default::default(); - - let length = message.to_buffer(&mut sysex_buffer); - let sysex_buffer = sysex_buffer.borrow(); - nih_debug_assert!(sysex_buffer.len() >= length); - let sysex_buffer = &sysex_buffer[..length]; + let (padded_sysex_buffer, length) = message.to_buffer(); + let padded_sysex_buffer = padded_sysex_buffer.borrow(); + nih_debug_assert!(padded_sysex_buffer.len() >= length); + let sysex_buffer = &padded_sysex_buffer[..length]; let event = clap_event_midi_sysex { header: clap_event_header { diff --git a/src/wrapper/standalone/backend/jack.rs b/src/wrapper/standalone/backend/jack.rs index 09050d07..eb47f2fc 100644 --- a/src/wrapper/standalone/backend/jack.rs +++ b/src/wrapper/standalone/backend/jack.rs @@ -136,8 +136,7 @@ impl Backend

for Jack { for event in output_events.drain(..) { let timing = event.timing(); - let mut sysex_buffer = Default::default(); - match event.as_midi(&mut sysex_buffer) { + match event.as_midi() { Some(MidiResult::Basic(midi_data)) => { let write_result = midi_writer.write(&jack::RawMidi { time: timing, @@ -146,15 +145,13 @@ impl Backend

for Jack { 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()); + Some(MidiResult::SysEx(padded_sysex_buffer, length)) => { + // `sysex_buffer` may contain padding + let padded_sysex_buffer = padded_sysex_buffer.borrow(); + nih_debug_assert!(length <= padded_sysex_buffer.len()); let write_result = midi_writer.write(&jack::RawMidi { time: timing, - bytes: &sysex_buffer[..length], + bytes: &padded_sysex_buffer[..length], }); nih_debug_assert!(write_result.is_ok(), "The MIDI buffer is full"); diff --git a/src/wrapper/vst3/wrapper.rs b/src/wrapper/vst3/wrapper.rs index 2d395b41..01eff8bc 100644 --- a/src/wrapper/vst3/wrapper.rs +++ b/src/wrapper/vst3/wrapper.rs @@ -1740,12 +1740,10 @@ impl IAudioProcessor for Wrapper

{ NoteEvent::MidiSysEx { timing: _, message } if P::MIDI_OUTPUT >= MidiConfig::Basic => { - let mut sysex_buffer = Default::default(); - - let length = message.to_buffer(&mut sysex_buffer); - let sysex_buffer = sysex_buffer.borrow(); - nih_debug_assert!(sysex_buffer.len() >= length); - let sysex_buffer = &sysex_buffer[..length]; + let (padded_sysex_buffer, length) = message.to_buffer(); + let padded_sysex_buffer = padded_sysex_buffer.borrow(); + nih_debug_assert!(padded_sysex_buffer.len() >= length); + let sysex_buffer = &padded_sysex_buffer[..length]; vst3_event.type_ = EventTypes::kDataEvent as u16; vst3_event.event.data = DataEvent {