rust-embedded-community / usbd-midi

MIT License
46 stars 19 forks source link

How does this crate fit into the usb-audio rust ecosystem #5

Open btrepp opened 3 years ago

btrepp commented 3 years ago

From conversations in https://github.com/btrepp/rust-midi-stomp/pull/1 with @perlindgren

Figured it might be better to discuss this in it's own issue.

Ultimately the summary is that to me it makes sense that this crate, or future versions of it, fit in nicely with the rest of usb audio devices. With most of the usb spec, this really should just be an extra 'feature' as part of the audio class, that you can choose to use or not.

What I'm unclear of at the moment, is the current state of audio devices in rust. I can't seem to find any groups about this in the embedded rust pages, but I think perhaps one exists. Does anyone have any links to repositories or groups?. From there I would also be curious as to whether there should be a 'reference' platform for boards. This was developed against the bluepill, but targeting a common board would help with investigating issues/development

In regards to aligning with other midi-implementations, I am for this. The implementation of codes here exists because others didn't at the time. It seems the normal rust community has published a few nifty crates since then.

x37v commented 1 year ago

In regards to aligning with other midi-implementations, I am for this. The implementation of codes here exists because others didn't at the time. It seems the normal rust community has published a few nifty crates since then.

11 updates this library to use shared types from https://github.com/rust-midi/

sourcebox commented 4 months ago

USB audio is one of the more complicated things to implement. There are even two standards: UAC1 and UAC2. For UAC2, a high speed device is required, which most MCUs today don't feature without an external PHY. I'm also not aware of a mature implementation in Rust for these classes, especially with having a working synchronisation endpoint recognized by all major host platforms.

What I can tell from having a look at the descriptors of some devices in my studio setup is that most manufacturers keep MIDI and audio as separate class interfaces, even if the device can do both. This is understandable as both are very different and having them in the same USB specs is somewhat unfortunate fact.

Regarding the types: I would really keep them of this crate and let the user choose an external one he likes. Imagine you have a synthesizer that features not only USB MIDI but also regular DIN MIDI (or maybe even Bluetooth MIDI). In that case, you likely want to have the same types for all transport mechanisms because they essentially carry the same messages (for MIDI 2.0, it's a different topic). Having the types in one of those libaries would make the other ones depend on exact the same version on it, a similar situation of what we currently have with usb-device.

With an external crate like wmidi, it's a one liner to convert from raw data:

let message = wmidi::MidiMessage::try_from(bytes)?;

I'm quite sure, you could do the same with the midi-types crate, avoiding all the dependency hassle.

btrepp commented 4 months ago

Setting aside types.

One of the reasons I made this issue was https://github.com/rust-embedded-community/usbd-midi/blob/b433d0f4a5d223b11c5f06c837132df2d39ce46e/src/midi_device.rs#L107.

You can see ultimately while we are 'just doing midi', even as it's own device (and you can easily have multiple devices in a usb device), it always felt a little dumb to make an 'audio class' device, and then not really do anything audio-ish, nor even be related to other classes that do audio things.

It probably does have relation to other crates, maybe ultimately a sub or library crate of some bigger tool-kit. This also probably highlights that the MidiDevice code isn't the best, it's workable, but it didn't really have a great 'design' haha.

sourcebox commented 4 months ago

You can see ultimately while we are 'just doing midi', even as it's own device (and you can easily have multiple devices in a usb device), it always felt a little dumb to make an 'audio class' device, and then not really do anything audio-ish, nor even be related to other classes that do audio things.

This is a personal view, but I think at the time USB MIDI came into the game, the USB implementors forum had to decide how to handle it. They essentially had 2 two options: create an own class for it or integrate it into another. They decided for the latter, probably because MIDI is kind of niche compared to the other things in USB universe. But I admit that I also was a little bit surprised when I first saw it under the audio class. With that in mind, I don't think there's something wrong with not doing audio in the audio class device.

btrepp commented 4 months ago

Ah I don't have any issues with how the spec is implemented, it's probably just around user expectations :). With midi being a subclass of audio, when looking at libraries you might expect to see say usb-audio, and the various sub classes, eg looking for midi, but you won't find it. Currently it's all different crates.

In practical matters, it doesn't really matter though, just that if you were implementing something that did both, it would appear as maybe more devices that it needed to :).

Pretty minor in the scheme of things though.