Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

midly integration for comprehensive message parsing #35

Open
masonium opened this issue Nov 14, 2023 · 3 comments
Open

midly integration for comprehensive message parsing #35

masonium opened this issue Nov 14, 2023 · 3 comments

Comments

@masonium
Copy link
Contributor

Currently, bevy_midi only parses Note On and Note Off messages, leaving it to the user to parse most messages. Because messages are consistently presented as 3 bytes, some messages are bound to be incorrectly parsed as well (this is the root cause of #29 , I think).

What do you think about using an external library, like midly, to do the messaging parsing? It has MidiStream specifically meant for parsing undelimitted byte streams of MIDI data into discrete events.

It does add an external dependency, and it would be a breaking change to the interface if bevy_midi directly uses its LiveEvent as the output. (midly itself has only one optional dependency in rayon, which bevy_midi wouldn't need for streaming data).

If it sounds reasonable, I'd be happy to take a crack at it.

@BlackPhlox
Copy link
Owner

It does sound very reasonable! midly have been on my radar, in regards to midi file playback (See here and here), but it is a bit of work that I myself don't have capacity right now. LiveEvent would also be a better place to start from, so you have my full support on this! 🚀

midly dosn't mention anything about WASM support as mentioned in #1, but that might be an non-issue and is just a question about implementing the integration via js/wasm bindgen

While we are talking about parsing underlying Midi messages, I wonder if it also would resolve #33, as it looks like some midi controllers have different ways of handling, in this case, the NOTE_OFF event. Looking at midly at a glance, it dosn't look like it handles it. I was thinking to create a lookup table for certain midi-devices and their name, to then handle odd cases as 33 and otherwise use the default, I would like to hear if you have any opinion on this in regards to midly.

Regarding #29, you might be right but I think it has more to do with it being Ableton Push 2 Live Port which doesn't follow the midi spec.

@masonium
Copy link
Contributor Author

Some updates:

midly itself seems to build fine with WASM, so I don't foresee an issue there.

The main thing I've run into is that LiveEvents are parameterized by lifetime, between they share storage with the buffer used in MidiStream. There is an to_static function which converts any LiveEvent<'a> to a LiveEvent<'static>, but this drops the data from SysEx messages. One approach is to accept that limitation; another approach is to create an OwnedLiveEvent and OwnedSystemCommand that mirrors the respective midly implementations but uses Vec<u8> (or a stack-allocated version) to make the events usable.

It could be nice to add some convenience functions for certain transformations (e.g. NOTE_ON with velocity 0 is converted to NOTE_OFF), or a comprehensive MessageTranslator that has a bunch of options for transformations. Personally I would lean against the lookup table approach; it strikes me as a maintenance headache (user reports with requests for changes but no real way of testing, etc.) but I don't have a strong opinion.

Anyway, I'm happy to proceed with the mirrored OwnedLiveEvent , etc. if you don't have a strong preference otherwise.

@BlackPhlox
Copy link
Owner

Nice, that's good news! I'm all for the mirrored OwnedLiveEvent.
Yeah, lookup will be painful, I think some DAWs have midi read certain transformations if they are non-destructive to other midi controllers, such as NOTE_ON with vel 0. I'll take a look at an impl. for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants