Midi functions draft #66

Levitanus · 2022-11-12T23:54:43Z

I've just built an iterator, that returns arbitrary "generic" midi events with PPQ position and all flags unpacked.

Then the decision is needed what to do:

stop on this point and just implement midi_set_all_evts and make everything clean
implement the rest of the midi_* functions as pure wrap on top of low-level
implement iterators over various type of midi-events. This breaks a bit medium-level API philosophy, but it should be more safe, than leave not very optimized raw functions instead.

midi_get_ppq_pos_start_of_measure midi_get_ppq_pos_end_of_measure midi_get_proj_qn_from_ppq_pos midi_get_ppq_pos_from_proj_qn midi_get_proj_time_from_ppq_pos midi_get_ppq_pos_from_proj_time every PPQ function is unsafe, because of take requirement Fixing naming convencions Co-authored-by: helgoboss <[email protected]> rebasing to master Replaced all mut to ptr

also introduced the new struct `TimeMapQNToMeasuresResult` for storing bar start\end positions renamed to time_map (without 2) and again

This reverts commit 59f2716cfcdf7efe543aaac796d2242b8ad8981b.

Levitanus · 2022-11-13T00:04:50Z

Example:

fn test_midi_events() {
    let rpr = Reaper::get().medium_reaper();
    let pr = reaper_medium::ProjectContext::CurrentProject;
    let track = rpr.get_track(pr, 2).unwrap();
    let item = get_track_items(track).expect("should be item")[0];
    unsafe {
        let take = rpr.get_active_take(item).expect("should be take");
        assert_true(
            rpr.take_is_midi(&take),
            "Take is MIDI",
            "Take should be MIDI",
        );
        let events = rpr
            .midi_get_all_events_iter(take, 500)
            .expect("should be iterator");
        println!(
            "Events from iterator: {:#?}",
            events.collect::<Vec<SourceMidiEvent>>()
        );
    }
}

Out:

Events from iterator: [
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            240.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: Square,
        message: [
            144,
            60,
            96,
        ],
    },
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            480.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: Square,
        message: [
            128,
            60,
            0,
        ],
    },
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            720.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: Square,
        message: [
            144,
            62,
            96,
        ],
    },
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            960.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: Square,
        message: [
            128,
            62,
            0,
        ],
    },
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            1200.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: Square,
        message: [
            144,
            60,
            103,
        ],
    },
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            1440.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: Square,
        message: [
            128,
            60,
            0,
        ],
    },
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            1680.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: Square,
        message: [
            144,
            62,
            96,
        ],
    },
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            1680.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: Square,
        message: [
            255,
            15,
            78,
            79,
            84,
            69,
            32,
            48,
            32,
            54,
            50,
            32,
            97,
            114,
            116,
            105,
            99,
            117,
            108,
            97,
            116,
            105,
            111,
            110,
            32,
            115,
            116,
            97,
            99,
            99,
            97,
            116,
            105,
            115,
            115,
            105,
            109,
            111,
        ],
    },
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            1920.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: Square,
        message: [
            128,
            62,
            0,
        ],
    },
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            2400.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: SlowStartEnd,
        message: [
            176,
            1,
            108,
        ],
    },
    SourceMidiEvent {
        position_in_ppq: PositionInPpq(
            3840.0,
        ),
        is_selected: false,
        is_muted: false,
        cc_shape_kind: Square,
        message: [
            176,
            123,
            0,
        ],
    },
]

Levitanus · 2022-11-13T18:13:33Z

Now, I'm pretty sure in iterators. And again thinking of adding original single-event functions, as additional tool for self-check, and because more complex iterators through raw midi lays out of medium-level tasks.

Considering the naming: I've thought at first to name the original functions as they are, and functions with iterators like *_iter. But it seemed not expressive and not very clear for me. And I didn't find anything better, than call original uncomfortable to use functions midi_set_all_evts_raw and midi_get_all_evts_raw. But, since, anywhere, there is almost no case, when someone would prefer raw function over iterator — I don't think it is a big problem.

Now it can hold various messages and still can represent raw bytes with `RawMidiMessage`

So, the main message types from `helgoboss_midi` now covered

Levitanus · 2022-11-13T21:28:39Z

OK. Now the idea looks fine for me. Please, check if you like this API and realization, and I'll finish other MIDI_* functions.

I've checked, that everything works OK with real REAPER, just need to fix this in reaper-rs integration test.

helgoboss

I didn't have the time to review all of your code yet but here's a start.

helgoboss · 2022-11-20T20:50:26Z

main/medium/src/reaper.rs

+        track: MediaTrack,
+        starttime: f64,
+        endtime: f64,
+        time_in_qn: bool,


Taking a bool is not very self-explaining in this case (see here for design rationale):

reaper.create_midi_item_in_proj(track, 0.0, 10.0, true);

I think I would prefer this one:

reaper.create_midi_item_in_proj(track, PositionInQuarterNotes(0.0)..PositionInQuarterNotes(10.0));

This would also make sense because we already have newtypes PositionInQuarterNotes and PositionInSeconds which are used consistently across the medium-level API. In real-world use cases, chances are you already have PositionInQuarterNotes values ready, so this would get quite terse.

It should be achievable by having:

pub unsafe fn create_midi_item_in_proj(&self, track: MediaTrack, range: R) -> ReaperFunctionResult<MediaItem> where R: Into<TimeRange>, UsageScope: MainThreadOnly, { ... } pub enum TimeRange { Seconds(Range<PositionInSeconds>), QuarterNotes(Range<PositionInQuarterNotes>), } impl From<Range<PositionInSeconds>> for TimeRange { ... } impl From<Range<PositionInQuarterNotes>> for TimeRange { ... }

Alternatively, we could also start by taking a TimeRange directly, without the magic Into conversions (and add the magic later if it's desired):

reaper.create_midi_item_in_proj(track, TimeRange::QuarterNotes(PositionInQuarterNotes(0.0)..PositionInQuarterNotes(10.0)));

But a bit too much repetition for my taste.

BTW, Rust has inclusive and exclusive ranges. Although it wouldn't make any different for us because we don't want to iterate the range, I think it would be good to communicate the intention. If I understand REAPER correctly, the end time always represents something exclusive. The item actually ends before that end position. What do you think?

Well, in this case we again face the problem of what medium-level should do, and what not...

For my personal tase, I found, that std already has awesome type, that solves 90% of ambiguity and boiler-plate: std:time::Duration. While one think of a Position in the therm of «Duration from the start», it removes any leaks in terminology. Alongside, it leaves interpretation of low-level API f64 arguments not on the type system, but on concrete wrapper-function realization. And, if, occasionally, someone made a mistake in interpreting float time as seconds, while it is not — the change will not affect next-level users.

More than, it implements accurate arithmetic operations as position comparison and holding the length of events. It naturally converts from\into f64, it is fast and supports all the derive traits, like Copy, Hash, Ord, Eq etc.

P.S. So, For my personal use I just wrapped it in my struct for the case, and implemented a couple of from\into traits.

The same thing I thought, when used a Color. NativeColor and RGB color is just a leaking abstraction, that not really helps in code usage. So, again, for myself I've made just Color with from_native and into_native methods, that take and return f64.

helgoboss · 2022-11-20T20:51:32Z

main/medium/src/reaper.rs

+        })
+    }
+
+    /// returns false if there are no plugins on the track that support MIDI programs,


The docs are not ready for prime-time yet but this is something that we can polish at the end, more important things right now.

helgoboss · 2022-11-20T20:54:04Z

main/medium/src/reaper.rs

+    /// or if all programs have been enumerated
+    pub fn enum_track_midi_program_names(
+        &self,
+        track: i32,


As a first time reader of this function, I don't know what these parameters mean, e.g. why i32 instead of u32? What does a negative value mean? This is something we should clarify by using newtypes or enums.

helgoboss · 2022-11-20T20:56:40Z

main/medium/src/reaper.rs

+        track_index: u32,
+        pitch: u32,
+        channel: u32,
+    ) -> Option<&'a ReaperStr>


Whenever you return a string that's owned by REAPER and can lose validity, use continuation-passing style. See enum_project_markers_3 for example.

Levitanus added 9 commits November 12, 2022 08:57

added reaper_medium::Reaper::time_map_2_qn_to_measure

4dee3b4

also introduced the new struct `TimeMapQNToMeasuresResult` for storing bar start\end positions renamed to time_map (without 2) and again

basics arithmetic for DurationInQuarterNotes

2274dca

started MIDI-functions (for issue)

c16fca4

Medium-level API: get_media_item_track

bb25dbb

Revert "Merge branch 'ppqfunctions' into midi_functions"

b72c902

This reverts commit 59f2716cfcdf7efe543aaac796d2242b8ad8981b.

resolve conflicts

de34208

midi_get_evt for consult

4ee5df4

skeleton of SourceMidiEvent iterator

4cfbe38

Levitanus marked this pull request as draft November 12, 2022 23:55

changed U7 to u8 as status bytes and etc uses 8 bits

a6358ba

Levitanus added 2 commits November 13, 2022 19:02

stabilized *all_evts iterators and functions

a303845

removed unused type in misc_enums

b9b88fa

Levitanus added 4 commits November 13, 2022 19:50

channel can be negative

1ef9e44

refactored SourceMidiEvent to be generic

f377932

Now it can hold various messages and still can represent raw bytes with `RawMidiMessage`

Adopted SourceMidiEvent to RawShortMessage

17e2524

So, the main message types from `helgoboss_midi` now covered

insert_* get_* set_* *cc functions

0f28db4

Levitanus mentioned this pull request Nov 15, 2022

What is state of MIDI take? State of API and what to use right now? #64

Closed

helgoboss requested changes Nov 20, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Midi functions draft #66

Midi functions draft #66

Levitanus commented Nov 12, 2022

Levitanus commented Nov 13, 2022

Levitanus commented Nov 13, 2022 •

edited

Loading

Levitanus commented Nov 13, 2022 •

edited

Loading

helgoboss left a comment

helgoboss Nov 20, 2022

Levitanus Nov 23, 2022 •

edited

Loading

Levitanus Nov 23, 2022 •

edited

Loading

helgoboss Nov 20, 2022

helgoboss Nov 20, 2022

helgoboss Nov 20, 2022

Midi functions draft #66

Are you sure you want to change the base?

Midi functions draft #66

Conversation

Levitanus commented Nov 12, 2022

Levitanus commented Nov 13, 2022

Levitanus commented Nov 13, 2022 • edited Loading

Levitanus commented Nov 13, 2022 • edited Loading

helgoboss left a comment

Choose a reason for hiding this comment

helgoboss Nov 20, 2022

Choose a reason for hiding this comment

Levitanus Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

Levitanus Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

helgoboss Nov 20, 2022

Choose a reason for hiding this comment

helgoboss Nov 20, 2022

Choose a reason for hiding this comment

helgoboss Nov 20, 2022

Choose a reason for hiding this comment

Levitanus commented Nov 13, 2022 •

edited

Loading

Levitanus commented Nov 13, 2022 •

edited

Loading

Levitanus Nov 23, 2022 •

edited

Loading

Levitanus Nov 23, 2022 •

edited

Loading