Skip to content

Commit

Permalink
Medium-level API: Fix MidiEvent
Browse files Browse the repository at this point in the history
There was a misunderstanding. The frame offset of a MIDI
event in REAPER is only given in 1/1024000 of a second when
read directly from the MIDI input device. In other cases,
e.g. when returning them from a PCM source, it should
refer to the current sample rate.
  • Loading branch information
helgoboss committed Feb 29, 2024
1 parent 6c0d9d7 commit 1b4ca83
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 53 deletions.
26 changes: 11 additions & 15 deletions main/medium/src/midi.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use helgoboss_midi::{ShortMessage, U7};
use reaper_low::raw;

use crate::{MidiFrameOffset, SendMidiTime};
use crate::SendMidiTime;
use reaper_low::raw::MIDI_event_t;
use ref_cast::RefCast;
use std::os::raw::c_int;
Expand All @@ -24,6 +24,9 @@ impl MidiInput {
///
/// This must only be called in the real-time audio thread! See [`get_midi_input()`].
///
/// This event list returned from this function has frame offsets that are in units of 1/1024000 of a second,
/// **not** sample frames. This frame rate is also available as constant [`crate::MIDI_INPUT_FRAME_RATE`].
///
/// # Design
///
/// In the past this function was unsafe and expected a closure which let the consumer do
Expand Down Expand Up @@ -215,13 +218,13 @@ impl MidiEvent {
}

/// Returns the frame offset.
pub fn frame_offset(&self) -> MidiFrameOffset {
MidiFrameOffset::new(self.0.frame_offset as u32)
pub fn frame_offset(&self) -> u32 {
self.0.frame_offset as _
}

/// Sets the frame offset.
pub fn set_frame_offset(&mut self, offset: MidiFrameOffset) {
self.0.frame_offset = offset.to_raw();
pub fn set_frame_offset(&mut self, frame_offset: u32) {
self.0.frame_offset = frame_offset as _;
}

/// Returns the actual message.
Expand Down Expand Up @@ -265,13 +268,9 @@ impl LongMidiEvent {
///
/// Size needs to be given because the actual message length is probably lower than the maximum
/// size of a long message.
pub fn new(
frame_offset: MidiFrameOffset,
midi_message: [u8; Self::MAX_LENGTH],
size: u32,
) -> Self {
pub fn new(frame_offset: u32, midi_message: [u8; Self::MAX_LENGTH], size: u32) -> Self {
Self {
frame_offset: frame_offset.to_raw(),
frame_offset: frame_offset as _,
size: size as _,
midi_message,
}
Expand All @@ -284,10 +283,7 @@ impl LongMidiEvent {
/// # Errors
///
/// Returns an error if the given slice is longer than the supported maximum.
pub fn try_from_slice(
frame_offset: MidiFrameOffset,
midi_message: &[u8],
) -> Result<Self, &'static str> {
pub fn try_from_slice(frame_offset: u32, midi_message: &[u8]) -> Result<Self, &'static str> {
if midi_message.len() > Self::MAX_LENGTH {
return Err("given MIDI message too long");
}
Expand Down
7 changes: 3 additions & 4 deletions main/medium/src/misc_enums.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::{
BookmarkId, CommandId, Handle, Hidden, Hwnd, InsertMediaFlag, KbdSectionInfo, MediaTrack,
MidiFrameOffset, MidiOutputDeviceId, ReaProject, ReaperPanValue, ReaperStr, ReaperStringArg,
ReaperWidthValue,
MidiOutputDeviceId, ReaProject, ReaperPanValue, ReaperStr, ReaperStringArg, ReaperWidthValue,
};

use crate::util::concat_reaper_strs;
Expand Down Expand Up @@ -1641,7 +1640,7 @@ pub enum SendMidiTime {
/// MIDI message will be sent instantly.
Instantly,
/// MIDI messages will be sent at the given frame offset.
AtFrameOffset(MidiFrameOffset),
AtFrameOffset(u32),
}

impl SendMidiTime {
Expand All @@ -1650,7 +1649,7 @@ impl SendMidiTime {
use SendMidiTime::*;
match self {
Instantly => -1,
AtFrameOffset(o) => o.to_raw(),
AtFrameOffset(o) => o as i32,
}
}
}
Expand Down
31 changes: 2 additions & 29 deletions main/medium/src/misc_newtypes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -862,35 +862,8 @@ impl<'a> Display for ReaperVersion<'a> {
}
}

/// A MIDI frame offset.
///
/// This is a 1/1024000 of a second, *not* a sample frame!
#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Default, Display)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
pub struct MidiFrameOffset(pub(crate) u32);

impl MidiFrameOffset {
/// Minimum frame offset (zero).
pub const MIN: MidiFrameOffset = MidiFrameOffset(0);

/// The frame rate to which this unit relates.
pub const REFERENCE_FRAME_RATE: Hz = unsafe { Hz::new_unchecked(1_024_000.0) };

/// Creates a MIDI frame offset.
pub fn new(value: u32) -> MidiFrameOffset {
MidiFrameOffset(value)
}

/// Returns the wrapped value.
pub const fn get(self) -> u32 {
self.0
}

/// Converts this value to an integer as expected by the low-level API.
pub fn to_raw(self) -> i32 {
self.0 as i32
}
}
/// The frame rate used for MIDI events in [`crate::MidiInput::get_read_buf`] in Hertz.
pub const MIDI_INPUT_FRAME_RATE: Hz = unsafe { Hz::new_unchecked(1_024_000.0) };

// TODO-medium This is debatable. Yes, we don't want information loss. But hiding the value?
// Too idealistic.
Expand Down
8 changes: 3 additions & 5 deletions main/rx/src/midi.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use crate::ReactiveEvent;
use helgoboss_midi::{RawShortMessage, ShortMessage, ShortMessageType};
use reaper_medium::{
MidiFrameOffset, MidiInputDeviceId, OnAudioBufferArgs, RealTimeAudioThreadScope,
};
use reaper_medium::{MidiInputDeviceId, OnAudioBufferArgs, RealTimeAudioThreadScope};
use rxrust::prelude::*;

pub struct MidiRxMiddleware {
Expand Down Expand Up @@ -57,12 +55,12 @@ impl MidiRx {

#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)]
pub struct MidiEvent<M> {
frame_offset: MidiFrameOffset,
frame_offset: u32,
msg: M,
}

impl<M> MidiEvent<M> {
pub fn new(frame_offset: MidiFrameOffset, msg: M) -> MidiEvent<M> {
pub fn new(frame_offset: u32, msg: M) -> MidiEvent<M> {
MidiEvent { frame_offset, msg }
}
}

0 comments on commit 1b4ca83

Please sign in to comment.