FortySevenEffects / arduino_midi_library

MIDI for Arduino
MIT License
1.58k stars 254 forks source link

[RFC] Advanced Thru #40

Open mink99 opened 8 years ago

mink99 commented 8 years ago

Hi,

I am currently testing enhanced features in the midi-thru parts. The concept is, that you can add a callback to the midi auto-thru logic.

It has the signature bool fn(message.type, &message.data &message.data2, &channel) maybe will change ..

The behaviour is : if the function returns false, the message will not be sent , otherwise it will be sent with potentially modified parameters data1, data2 and channel (if available).

This is the first stage of the required features of a hardware midi patchbay, but is also useful on other occasions.

Currently I am using it for a box, that provides complex-preset based program changes to external modules. It will be connected by its midi-in from the masterkeyboard, on its midi out to a multitimbral instrument. First I am suppressing all active sensing from the keyboard, then I trap all program changes from the keyboard, remap them to the internal presets and send those new program changes from the box. This is just one example, there could be more.

I am convinced that it is not only me who could use this feature, so I am raising this issue as a starting point for a discussion.

Thank you

Kris

mink99 commented 8 years ago

I could post my code, but it is not really conforming to the coding style, and maybe would interfere with other planned changes ? Should I ?

franky47 commented 8 years ago

Hi Kris,

It seems like a good idea ! The original intent of the thru filter was to provide a way to choose which messages were thru'd, based only on the input channel, but I have seen users having the need to remap messages and have less permissive filters, so this could be useful.

I'd be glad to review your code and we could work on it on a separate branch.

mink99 commented 8 years ago

ok here is my code : MIDI.zip diffs (against stable)

diff.zip

i did some testing, hopefully it works as expected

sample Code from my application :

bool channelMessageThruFilter(midi::MidiType inType,  midi::DataByte &inData1,  midi::DataByte &inData2,  midi::Channel &inChannel)
{
  if (inType == midi::ActiveSensing )  return false;

  if (inType == midi::ProgramChange )
  {
    currentPatchID = inData1; // --> this is internal code of the app. capture program changes from a master Keyboard
    updateScreen(VK_CHANNEL); // this is what you should never do , expensive calls within a callback
    return false; // do not forward
  }
}
mink99 commented 8 years ago

Sample code for #41 would be

bool channelMessageThruFilter(midi::MidiType inType,  midi::DataByte &inData1,  midi::DataByte &inData2,  midi::Channel &inChannel)
{
  if (inType == midi:: PitchBend   )
  {
     int bend = (int)((inData1 & 0x7f) | ((inData2 & 0x7f) << 7)) + MIDI_PITCHBEND_MIN);
     CCPitch = 1+ 127* float(
            (bend - PITCHBENDMIN)/
                float(PITCHBENDMAX-PITCHBENDMIN));

    MIDI.sendControlChange(80, CCPitch, channel);
    return false; // do not forward
  }
}

this is not tested. and shows one of the challenges of this approach : would it make sense to be able to modify message types ? I am on the strong opinion no, it would be more feasible to be able to explicitly send messages within the callback. but that would require some macro support for transforming existing parameters into new ones...

mink99 commented 8 years ago

The above example should be simplified and split into two methods:

In the callback filter suppress the pitch bend messages only. In the callback create the modified message. It seems important, that, if we are receiving in running-status mode, and suppress messages through the filter, we will have to recreate the status bytes for the next, non-suppressed message, because on a channel message, which is directed to another channel than the message before, if we suppress this message, the following message will be sent to the channel of the previous message, not to the intended.... Maybe this is also the problem on # # #41

eclab commented 7 years ago

I like this idea in general but it propagates a weakness in the code with regard to MIDI thru. So far as I can tell -- and I may be wrong -- a message must be completely parsed before it is sent. So if you have (say) a three-byte message, you'll have to wait perhaps 1 ms before you can resend it. So you have a 1ms lag. That doesn't sound like much but really is.

Ideally a thru filter mechanism would query the filter as soon as enough information about the message has been parsed to determine useful information about it. For example, as soon as we determine that the message is (say), a CC message of interest to us, we should query the filter, then if it says it's okay, we pass the message in real time as the rest of it comes in, thus minimizing possible lag.

This is of course a more complex behavior.

franky47 commented 7 years ago

@eclab, that would minimise the latency, indeed. One issue though is that some messages can be intertwined with others (ie: Clock messages can happen at any time, even in between other messages bytes or within a SysEx frame).

eclab commented 7 years ago

Ugh, yes, that's an ugly issue. :-(

franky47 commented 7 years ago

Another reason why we need to fully parse a message before thruing it is to avoid collisions. Say you have an input stream that gave you a status byte (the data byte(s) have not arrived yet), you can't send anything out until the input has finished thruing the complete message.

One potential way of solving this particular issue would be to use a send queue, where output messages would be pushed by the send methods, and popped to the UART TX when there are no more incomplete incoming messages.

franky47 commented 4 years ago

I'd like to re-open discussion on this, as it's been way too long, and this was actually a good idea.

At the moment, the Software Thru feature is limited to 4 modes:

The Different Channel modes was intended to reduce MIDI traffic for other devices in a chain (consume what you need, feed the rest back to the chain). The Same Channel mode seems like YAGNI gone wrong, it was easy to make (by negating the Different Channel mode), but in practice I don't see much use to it.

Instead, to follow up on what was proposed by @mink99, we can have two new functions:

Filter

Filter would be a callback that takes the incoming message as an argument (a const ref to avoid modification), and returns a boolean: true to forward the message, false to consume it.

bool thruFilter(const MIDI::MidiMessage& inMessage)
{
  if (inMessage.type == midi::Clock)
  {
    // Consume (don't forward) Clock messages
    return false;
  }
  return true;
}

MIDI.setThruFilter(thruFilter);

Map

Map would also be a callback that takes the incoming message as argument, but returns another message. The message returned will be forwarded to the output.

MIDI::MidiMessage thruMap(const MIDI::MidiMessage& inMessage)
{
  if (inMessage.channel == 3)
  {
    // Remap all messages from channel 3 to channel 4:
    MIDI::MidiMessage modified = inMessage;
    modified.channel = 4;
    return modified;
  }
  return inMessage;
}

MIDI.setThruMap(thruMap);

Caveats & Limitations

Complete flow diagram:

image

Note: this follows the behaviour of map and filter functions found in various other programming languages, like JavaScript or Python.

Migration from previous API

It's possible to have all four previous modes using only the filter callback:

bool thruOn(const MIDI::MidiMessage& inMessage)
{
  return true; // Forward everything
}

bool thruOff(const MIDI::MidiMessage& inMessage)
{
  return false; // Forward nothing
}

bool thruSameChannel(const MIDI::MidiMessage& inMessage)
{
  if (MIDI::isChannelMessage(inMessage.type) == false)
  {
    return true; // Always forward non-channel messages
  }
  return inMessage.channel == MIDI.getInputChannel();
}

bool thruDifferentChannel(const MIDI::MidiMessage& inMessage)
{
  if (MIDI::isChannelMessage(inMessage.type) == false)
  {
    return true; // Always forward non-channel messages
  }
  return inMessage.channel != MIDI.getInputChannel();
}
hyperbolist commented 3 years ago

I love this latest iteration on the idea. I've just started a new project where a thru filter is its primary function. I was going to naively use handlers for every incoming message type and conditionally echo them to the midi out until I stumbled upon the baked-in thru functionality and this discussion here.

Having a single thruFilter function to write would simplify my project considerably.

Looking forward to it!