midi2 icon indicating copy to clipboard operation
midi2 copied to clipboard

how to return runtime defined amount and type of midi2 messages.

Open laenzlinger opened this issue 1 year ago • 23 comments

How would one use this trait for an application that might create multiple messages triggered by an event (e.g. a button)

Is there some support for an iterator approach available where the caller would get an iterator of messages produces by the event?

laenzlinger avatar Jan 04 '25 03:01 laenzlinger

I am struggling with my rust knowledge. Sorry for this. Any help is very much appreciated. In addition to the question above, I am struggling already with returning a single generic message from a function. The specific returned message can be pre-configured on the device at runtime -> it is not yet known at compile time. Which type of this create would such a function return?

laenzlinger avatar Jan 04 '25 13:01 laenzlinger

Hello!

How would one use this trait for an application that might create multiple messages triggered by an event?

TLDR: UmpMessage<[u32; 4]> is a pretty good choice.

Which trait are you referring to? There are some traits which capture common functionality, but in this case you probably want to have such events generate one of the aggregate message types. Either UmpMessage or perhaps channel_voice2::ChannelVoice2 or something, if you don't need all the different message types.

All messages are generic over their backing buffer. This means you'll need choose the buffer type for your generated messages. [u32; 4] will cover most cases. For the fancier messages (e.i. data messages or text) you'll need more space - a Vec<u32> would always work if you need such things.

BenLeadbetter avatar Jan 04 '25 21:01 BenLeadbetter

The specific returned message can be pre-configured on the device at runtime -> it is not yet known at compile time.

I think this is also solved with the aggregate message types, e.g. UmpMessage or one of the sub aggregate messages e.g. channel_voice2::ChannelVoice2

BenLeadbetter avatar Jan 04 '25 22:01 BenLeadbetter

Thank you for your answer. At the moment I want to send ChannelVoice1 or MMC (Sysex7?) types of messages. However, in future also other (MIDI2) types of messages might also be implemented. But this has no priority. I just want to develop using a midi crate which has a future with MIDI 2.0. I just found this crate a few days ago and still trying to learn how its built and can be used.

I forgot to mention, that I am developing for a micro controller (no_std environment)

how would you recommend to define a function that generates such messages triggered by user actions (concrete message to be generated is based on the internal state of the device)

Would you pass a mut buffer into the function?

How can I keep the design flexible for future extensions (more message types)

laenzlinger avatar Jan 05 '25 00:01 laenzlinger

my current attempt looks like this:

impl Button {
    pub fn handle<B>(&mut self, action: Action) -> Option<BytesMessage<B>>
    where
        B: Buffer<Unit = u8> + BufferMut + BufferDefault + BufferResize,
    {
         // depending on the configuration of the Button and the action input, 
         // a different message is created. Can be Sysex or ChannelVoice message
    }

This compiles but I can not use this with a [u8; 3] buffer because it does not implement the BufferResize trait.

How would you suggest to change the fn interface?

I am sorry for this question I know I still have to learn a lot about Rust.

laenzlinger avatar Jan 05 '25 17:01 laenzlinger

You could use the BufferTryResize trait instead of BufferResize. This will allow you to do sysex operations and also support the full feature set for the channel voice messages.

BenLeadbetter avatar Jan 12 '25 11:01 BenLeadbetter

Thanks a lot for this hint with BufferTryResize This really helped.

I am now trying an interface like the following:

impl Button {
    pub fn handle<B>(&mut self, action: Action) -> Result<Option<BytesMessage<B>>, BufferOverflow>
    where
        B: Buffer<Unit = u8> + BufferMut + BufferDefault + BufferTryResize,
    {
         // depending on the state of the button, create new messages
         // e.g.:   let mut m = NoteOn::<B>::try_new()?;
    }
}

Then on the caller side i can extract the data with data() function.

This seems to work and also allows different handling of messages (in my case I need to handle sysex messages slighly different than ChannelVoice1 messages

I am not sure if this makes sense? If you have better suggestions, please let me know

laenzlinger avatar Jan 12 '25 19:01 laenzlinger

What is not clear to me: How do I find out how large the resulting buffer should be that has to be sent over the wire?

For example a TimingClock message would be one byte, but the buffer returned by data() seems to be two bytes long.

laenzlinger avatar Jan 12 '25 20:01 laenzlinger

data() should always return the size of the message buffer. TimingClock::data should therefore return a buffer size 1.

It seems there's a bug in this area - thanks for flagging it! I've created a pull request to fix the issue. I'll put out a hotfix release today hopefully. Please do let me know if there are any other issues like this?

BenLeadbetter avatar Jan 13 '25 07:01 BenLeadbetter

Ok, this makes the above approach valid again. I was scared that this this information (length) would have to be retreived from somewhere else which would have been a bit more inconvenient. But anyways did not find it. So fixing it, so that the client can rely on data to have the right size sounds great.

laenzlinger avatar Jan 13 '25 15:01 laenzlinger

If you pin to version 0.6.4 the issue should be fixed.

BenLeadbetter avatar Jan 13 '25 19:01 BenLeadbetter

Yes I can confirm that upgrading to 0.6.4 fixes the length issue with realtime messages.

Could you maybe provide some feedback to the API that was proposed above. Does this make sense for you? With my current knowledge, this would allow to dynamically extend the buffer, if the underlying implementation allows. The try_new method would do that, if I understood the code correctly.

To also support MIDI 2 messages the interface would have to be changed. Is my understanding correct?

laenzlinger avatar Jan 13 '25 20:01 laenzlinger

Sorry about the delayed response. I wanted to put out a feature release (0.7.0) before getting back to you.

It's a little tricky to critique your implementation without knowing more of the context of your application, but I can give it a go.

You say that this handler on the button needs to dynamically create midi messages of different types? In that case a generic api on the handler function might not be the best since it will force you to decide a backing buffer at compile time. So in order to remove the generic we need to choose an appropriate backing buffer. We need something which can accommodate large messages and also something no_std friendly (otherwise a Vec would do nicely). [u8; 3] is not going to be much good if you want to create sysex messages too.

With the new_with_buffer constructors (new in 0.7.0) we can use a &mut [u8], so long as your application memory model will allow it:

use midi2::{channel_voice1::*, prelude::*, sysex7::*};

enum Action {
    Foo,
    Bar,
}

fn handle(
    action: Action,
    buffer: &mut [u8],
) -> Result<BytesMessage<&mut [u8]>, midi2::error::BufferOverflow> {
    match action {
        Action::Foo => {
            let mut message = NoteOn::try_new_with_buffer(buffer)?;
            message.set_note_number(u7::new(0x3));
            message.set_velocity(u7::new(0x5));
            Ok(message.into())
        }
        Action::Bar => {
            let mut message = Sysex7::try_new_with_buffer(buffer)?;
            message.try_set_payload((0x0..0x20).map(u7::new))?;
            Ok(message.into())
        }
    }
}

fn main() {
    let mut message_buffer = [0x00u8; 128];
    println!("{:?}", handle(Action::Foo, &mut message_buffer[..]).unwrap().data());
    println!("{:?}", handle(Action::Bar, &mut message_buffer[..]).unwrap().data());
}

Alternatively you could using a simple array for backing buffer (BytesMessage<[u8; 64]>), but you'd be wasting memory for the CV messages.

BenLeadbetter avatar Jan 18 '25 20:01 BenLeadbetter

Regarding your question about using the newer midi2 protocols. Yes, you'd have to use Ump buffers instead to do things the modern way. You could always use the Ump buffer now and simply convert them to the old format with into_bytes before you need to transmit them. This way you wont need to change any of your types when you decide to start transmitting the ump data instead.

e.g.

use midi2::{channel_voice1::*, prelude::*};

fn main() {
    let mut ump_note_on = NoteOn::<[u32; 2]>::new();
    ump_note_on.set_note_number(u7::new(0x14));
    ump_note_on.set_velocity(u7::new(0x19));
    println!("{:?}", ump_note_on.data());
    
    let bytes_message: NoteOn<[u8; 3]> = ump_note_on.into_bytes();
    println!("{:?}", bytes_message.data());
}

BenLeadbetter avatar Jan 18 '25 20:01 BenLeadbetter

Thanks for you reply and also for try_new_with_buffer.

I have changed the code now to use a non-generic API as you suggested.

Since the state of the button is stored in the Button struct I had to declare the lifetime.

pub struct Button {
  // state of the button (incl. configuration) 
}

pub enum Action {
    Pressed,
    Released,
}

impl Button {
    pub fn handle<'a>(
        &mut self,
        action: Action,
        buffer: &'a mut [u8],
    ) -> Result<Option<BytesMessage<&'a mut [u8]>>, BufferOverflow> {
     // use try_new_with_buffer to create messages.
   }
}
 

I am building a library, which should have enough flexibility to be used by different applications.

Example: The number of controllers might differ on different applications, but the library should be reusable. But the targets are no_std environments.

Do you think such an API makes sense? Do you have suggestions for improvments?

laenzlinger avatar Jan 26 '25 14:01 laenzlinger

One nest problem, where I am now struggling, is the initial question that I had: There are some actions, which trigger multiple messages to be returned. For example. I want to send multiple CC messages when the user presses a certain button.

I was thinking to return an Iterator of BytesMessages from my handle method.

But I am struggling with defining the right lifetime anotations for such an iterator.

Can you help me to define such an iterator? I am sorry that my Rust knowlege is currently so limitted. I am trying to learn as fast as possbile. Any hint in which direction I should investage would be very much appreciated.

laenzlinger avatar Feb 02 '25 19:02 laenzlinger

Here's an example of how such an iterator might be implemented. Being no_std compatible means you have to jump through a lot of hoops with statically sized buffers, but it's still possible to achieve what you want, I think 😊 .

use midi2::{channel_voice1::*, prelude::*, sysex7::*};

struct MessageIterator<'a, 'b> {
    messages: &'b [Option<BytesMessage<&'a mut [u8]>>],
    index: usize,
}

impl<'a, 'b> Iterator for MessageIterator<'a, 'b> {
    type Item = &'b BytesMessage<&'a mut [u8]>;

    fn next(&mut self) -> Option<Self::Item> {
        if self.index < self.messages.len() {
            let index = self.index;
            self.index += 1;
            Some(self.messages[index].as_ref().unwrap())
        } else {
            None
        }
    }
}

const MAX_MESSAGES: usize = 16;

struct Button<'a> {
    messages: [Option<BytesMessage<&'a mut [u8]>>; MAX_MESSAGES],
}

impl<'a> Button<'a> {
    fn new() -> Self {
        Self {
            messages: [
                None, None, None, None, None, None, None, None, None, None, None, None, None, None,
                None, None,
            ],
        }
    }

    fn handle<'b>(
        &'b mut self,
        buffer: &'a mut [u8],
    ) -> Result<MessageIterator<'a, 'b>, midi2::error::BufferOverflow> {
        let mut number_of_messages = 0;

        // create some channel voice messages

        let (message_buffer, buffer) = buffer.split_at_mut(2);
        let mut channel_pressure = ChannelPressure::try_new_with_buffer(message_buffer)?;
        channel_pressure.set_pressure(u7::new(0x50));
        self.messages[number_of_messages] = Some(channel_pressure.into());
        number_of_messages += 1;

        let (message_buffer, buffer) = buffer.split_at_mut(3);
        let mut note_on = NoteOn::try_new_with_buffer(message_buffer)?;
        note_on.set_note_number(u7::new(0x38));
        note_on.set_velocity(u7::new(0x20));
        self.messages[number_of_messages] = Some(note_on.into());
        number_of_messages += 1;

        let (message_buffer, buffer) = buffer.split_at_mut(3);
        let mut control_change = ControlChange::try_new_with_buffer(message_buffer)?;
        control_change.set_control(u7::new(0x34));
        control_change.set_control_data(u7::new(0x2A));
        self.messages[number_of_messages] = Some(control_change.into());
        number_of_messages += 1;

        // and a sysex

        let (message_buffer, _) = buffer.split_at_mut(22);
        let mut sysex = Sysex7::try_new_with_buffer(message_buffer)?;
        sysex.try_set_payload((0..20).map(u7::new))?;
        self.messages[number_of_messages] = Some(sysex.into());
        number_of_messages += 1;

        Ok(MessageIterator {
            messages: &self.messages[0..number_of_messages],
            index: 0,
        })
    }
}

fn main() {
    let mut button = Button::new();
    for message in button.handle(&mut [0; 100]).unwrap() {
        println!("{:?}", message);
    }
}

As for whether the API makes sense, it's really hard for me to say without more context about the intention of your library and which clients it's supposed to cater for.

BenLeadbetter avatar Feb 05 '25 21:02 BenLeadbetter

@BenLeadbetter Thank you so much for your continiued support and help for exploring these ideas!

If I understand your idea correctly, then you split the buffer into multiple junks with a size depending on the needs for the specific message (by using split_at_mut).

I think this could work out but would require that I need to create a buffer size based on the worst case scenario (largest message * number of possible messages). This is exactly what I would like to avoid by using an iterator approach.

I would like to defer using the buffer by defering the use of the buffer until the time when the iterator.next() is called. That way, the requirements for the buffer is just the max size of all possible messages, but is independent of the number of messages that will be generated.

However this makes everthing so much more complicated. I am struggling to find the right answer on how the API should look like with the right lifetimes.

Each call to iterator.next could reuse the same backing buffer, create the next BytesMessage which would then be sent to the output. But I am not sure if that is possible to achieve with the standard Iterator API.

In addition, what makes the matter even more complicated: There is not only Buttons that should be configured, there are also other types of input elements (analog inputs or encoders) which have a different handle method. But all should return the same type of Iterator, so that all input events can return the same response. This response is then handled by the application.

Essentially I am trying to build a crate that implements the OpenDeck Midi Controller 'specification'. The current status of my development can be found here. Its very much WIP.

laenzlinger avatar Feb 09 '25 15:02 laenzlinger

As far as I can see from the OpenDeck spec, these buttons should never need to send SesEx messages. Seems like SysEx is only used during a configuration stage, not during ordinary functional use of the Buttons / Encoders / etc. If we could remove the SysEx requirement from the design then this API become much more simple. You can use BytesMessage<[u8; 3]> and cover all the bases with absolutely no lifetime issues.

BenLeadbetter avatar Feb 10 '25 14:02 BenLeadbetter

There are the MMC messages, which are SysEx based. When using a DAW this is a very useful feature for a Midi controller. DAWs (although they can of course be reconfigured) are acting on MMC messages. Therefore, it would hurt a bit to not support SysEx.

When looking into the Analog inputs. Its used to send out multiple CC messages to support Ableton's "High resolution MIDI" protocol. (Of course this will be replaced hopefully soon with MIDI2 capabilties)

What is IMO an edge case and could be skipped is the "Omni Channel" mode. This would IMO be better configured on the receiver end. Not sure if there is a real use case (need) for sending the same message on all 16 channels.

laenzlinger avatar Feb 10 '25 15:02 laenzlinger

Ok, cool.

Looking into the schema for these MMC messages it seems that the longest possible message would be:

F0 7F <Device-ID> 06 <length>=44goto <length>=06 01 <hours> <minutes> <seconds> <frames> <subframes> F7

Which is 13 bytes long.

Suppose you used a simple backing buffer of [u8; 16]. Then all the lifetime issues go away and you can represent all CV messages along with any sysex messages which fit into 16 bytes (including all MMC messages). This would actually be the same size as a midi2 "Universal Message Packet" - definitely not too expensive for copy semantics, etc.

BenLeadbetter avatar Mar 02 '25 09:03 BenLeadbetter

In addition, what makes the matter even more complicated: There is not only Buttons that should be configured, there are also other types of input elements (analog inputs or encoders) which have a different handle method. But all should return the same type of Iterator, so that all input events can return the same response. This response is then handled by the application.

Why not make a MessageBuffer type which owns the buffer of messages and is able to return the iterator type. Then any control which needs to return messages simply owns one of these things and interfaces with it to add the correct messages to the buffer? You could use const generics to add some flexibility with the number of messages allowed in the buffer.

type Message = midi2::BytesMessage<[u8; 16]>;

struct MessageBuffer<const MAX_MESSAGES: usize> {
    messages: [Option<Message>; MAX_MESSAGES],
}

impl<const MAX_MESSAGES: usize> MessageBuffer<MAX_MESSAGES> {
    fn messages() -> MessageIterator { ... }
    fn add_message(Message) { ... }
    fn clear() { ... }
}

struct Button {
    MessageBuffer<32>,
}
struct Encoder {
    MessageBuffer<32>,
}
// etc
```I

BenLeadbetter avatar Mar 02 '25 09:03 BenLeadbetter

Thanks @BenLeadbetter for all your suggestions. I am currently working with the following API:

Each controller type's handle() function returns a ControllerMessages struct which has a similar interface than the std iterator. It wraps the Controller struct and the next() function takes the buffer as input paramter and returns an Result<Optional<BytesMessages>>

For example the Button it is implemented like this: https://github.com/pedalboard/opendeck/blob/main/src/button/handler.rs#L29

Then I implemented an common Messages enum, which also has the same next() function and then wraps all the different ControllerMessages struct.

see https://github.com/pedalboard/opendeck/blob/main/src/handler.rs#L9-L28

With this i compromised on implementing the std::Iterator trait which is not so nice for the API client, but I did not compromise on the buffer size flexibility and the maximum buffer size is the maximum expected message size.

I think this is an acceptable solution for this problem, since there will not be many such handler clients.

Maybe, in future , as I learn more about Rust I will discover a better solution with using some std iterator features that I am currently not aware of.

laenzlinger avatar Mar 18 '25 18:03 laenzlinger