From 8d2d293b49b76610e48ee1ea31084edb5654f125 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 1 Feb 2023 17:13:03 +0100 Subject: [PATCH] Clamp event timings for CLAP plugins --- src/midi.rs | 4 +-- src/wrapper/clap/wrapper.rs | 64 ++++++++++++++++++++++++++++--------- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/midi.rs b/src/midi.rs index d27392ad..219ba324 100644 --- a/src/midi.rs +++ b/src/midi.rs @@ -39,8 +39,8 @@ pub enum MidiConfig { /// [`Plugin::MIDI_INPUT`][crate::prelude::Plugin::MIDI_INPUT]. Also check out the /// [`util`][crate::util] module for convenient conversion functions. /// -/// All of the timings are sample offsets within the current buffer. All sample, channel and note -/// numbers are zero-indexed. +/// All of the timings are sample offsets within the current buffer. Out of bounds timings are +/// clamped to the current buffer's length. All sample, channel and note numbers are zero-indexed. #[derive(Debug, Clone, Copy, PartialEq)] #[non_exhaustive] pub enum NoteEvent { diff --git a/src/wrapper/clap/wrapper.rs b/src/wrapper/clap/wrapper.rs index 7cd7a673..24528980 100644 --- a/src/wrapper/clap/wrapper.rs +++ b/src/wrapper/clap/wrapper.rs @@ -914,14 +914,25 @@ impl Wrapper

{ } /// Handle all incoming events from an event queue. This will clear `self.input_events` first. - pub unsafe fn handle_in_events(&self, in_: &clap_input_events, current_sample_idx: usize) { + pub unsafe fn handle_in_events( + &self, + in_: &clap_input_events, + current_sample_idx: usize, + total_buffer_len: usize, + ) { let mut input_events = self.input_events.borrow_mut(); input_events.clear(); let num_events = clap_call! { in_=>size(in_) }; for event_idx in 0..num_events { let event = clap_call! { in_=>get(in_, event_idx) }; - self.handle_in_event(event, &mut input_events, None, current_sample_idx); + self.handle_in_event( + event, + &mut input_events, + None, + current_sample_idx, + total_buffer_len, + ); } } @@ -939,6 +950,7 @@ impl Wrapper

{ in_: &clap_input_events, transport_info: &mut *const clap_event_transport, current_sample_idx: usize, + total_buffer_len: usize, resume_from_event_idx: usize, stop_predicate: impl Fn(*const clap_event_header) -> bool, ) -> Option<(usize, usize)> { @@ -959,6 +971,7 @@ impl Wrapper

{ &mut input_events, Some(transport_info), current_sample_idx, + total_buffer_len, ); // Stop just before the next parameter change or transport information event at a sample @@ -977,6 +990,7 @@ impl Wrapper

{ &mut input_events, Some(transport_info), current_sample_idx, + total_buffer_len, ); None @@ -986,7 +1000,14 @@ impl Wrapper

{ /// used as part of splitting up the input buffer for sample accurate automation changes. This /// will also modify the actual parameter values, since we should only do that while the wrapped /// plugin is not actually processing audio. - pub unsafe fn handle_out_events(&self, out: &clap_output_events, current_sample_idx: usize) { + /// + /// The `total_buffer_len` argument is used to clamp out of bounds events to the buffer's length. + pub unsafe fn handle_out_events( + &self, + out: &clap_output_events, + current_sample_idx: usize, + total_buffer_len: usize, + ) { // We'll always write these events to the first sample, so even when we add note output we // shouldn't have to think about interleaving events here let sample_rate = self.current_buffer_config.load().map(|c| c.sample_rate); @@ -1057,7 +1078,13 @@ impl Wrapper

{ // Also send all note events generated by the plugin let mut output_events = self.output_events.borrow_mut(); while let Some(event) = output_events.pop_front() { + // Out of bounds events are clamped to the buffer's size let time = event.timing() + current_sample_idx as u32; + nih_debug_assert!( + time < total_buffer_len as u32, + "Output event is out of bounds, will be clamped to the buffer's size" + ); + let time = time.min(total_buffer_len as u32 - 1); let push_successful = match event { NoteEvent::NoteOn { @@ -1391,9 +1418,17 @@ impl Wrapper

{ input_events: &mut AtomicRefMut>>, transport_info: Option<&mut *const clap_event_transport>, current_sample_idx: usize, + total_buffer_len: usize, ) { let raw_event = &*event; + + // Out of bounds events are clamped to the buffer's size let timing = raw_event.time - current_sample_idx as u32; + nih_debug_assert!( + timing < total_buffer_len as u32, + "Input event is out of bounds, will be clamped to the buffer's size" + ); + let timing = timing.min(total_buffer_len as u32 - 1); match (raw_event.space_id, raw_event.type_) { (CLAP_CORE_EVENT_SPACE_ID, CLAP_EVENT_PARAM_VALUE) => { @@ -1622,8 +1657,7 @@ impl Wrapper

{ // messages to stay consistent with the VST3 wrapper. let event = &*(event as *const clap_event_midi); - match NoteEvent::from_midi(raw_event.time - current_sample_idx as u32, &event.data) - { + match NoteEvent::from_midi(timing, &event.data) { Ok( note_event @ (NoteEvent::NoteOn { .. } | NoteEvent::NoteOff { .. } @@ -1647,9 +1681,7 @@ impl Wrapper

{ // necessarily an error assert!(!event.buffer.is_null()); let sysex_buffer = std::slice::from_raw_parts(event.buffer, event.size as usize); - if let Ok(note_event) = - NoteEvent::from_midi(raw_event.time - current_sample_idx as u32, sysex_buffer) - { + if let Ok(note_event) = NoteEvent::from_midi(timing, sysex_buffer) { input_events.push_back(note_event); }; } @@ -1946,6 +1978,7 @@ impl Wrapper

{ // accuration automation yet and there's no way to get the last event for a parameter, // we'll process every incoming event. let process = &*process; + let total_buffer_len = process.frames_count as usize; // Before doing anything, clear out any auxiliary outputs since they may contain // uninitialized data when the host assumes that we'll always write something there @@ -1962,7 +1995,7 @@ impl Wrapper

{ ptr::write_bytes( *((*host_output).data32.offset(channel_idx)) as *mut f32, 0, - process.frames_count as usize, + total_buffer_len, ); } } @@ -1972,7 +2005,7 @@ impl Wrapper

{ // If `P::SAMPLE_ACCURATE_AUTOMATION` is set, then we'll split up the audio buffer into // chunks whenever a parameter change occurs let mut block_start = 0; - let mut block_end = process.frames_count as usize; + let mut block_end = total_buffer_len; let mut event_start_idx = 0; // The host may send new transport information as an event. In that case we'll also @@ -1985,6 +2018,7 @@ impl Wrapper

{ &*process.in_events, &mut transport_info, block_start, + total_buffer_len, event_start_idx, |next_event| { // Always split the buffer on transport information changes (tempo, time @@ -2026,7 +2060,7 @@ impl Wrapper

{ block_end = next_param_change_sample_idx; event_start_idx = next_param_change_event_idx; } - None => block_end = process.frames_count as usize, + None => block_end = total_buffer_len, } } @@ -2336,13 +2370,13 @@ impl Wrapper

{ // After processing audio, send all spooled events to the host. This include note // events. if !process.out_events.is_null() { - wrapper.handle_out_events(&*process.out_events, block_start); + wrapper.handle_out_events(&*process.out_events, block_start, total_buffer_len); } // If our block ends at the end of the buffer then that means there are no more // unprocessed (parameter) events. If there are more events, we'll just keep going // through this process until we've processed the entire buffer. - if block_end as u32 == process.frames_count { + if block_end == total_buffer_len { break clap_result; } else { block_start = block_end; @@ -3153,11 +3187,11 @@ impl Wrapper

{ let wrapper = &*((*plugin).plugin_data as *const Self); if !in_.is_null() { - wrapper.handle_in_events(&*in_, 0); + wrapper.handle_in_events(&*in_, 0, 0); } if !out.is_null() { - wrapper.handle_out_events(&*out, 0); + wrapper.handle_out_events(&*out, 0, 0); } }