-
Notifications
You must be signed in to change notification settings - Fork 258
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
[RFC] Advanced Thru #40
Comments
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 ? |
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. |
ok here is my code : i did some testing, hopefully it works as expected sample Code from my application :
|
Sample code for #41 would be
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... |
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. |
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. |
@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). |
Ugh, yes, that's an ugly issue. :-( |
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. |
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:
FilterFilter would be a callback that takes the incoming message as an argument (a const ref to avoid modification), bool thruFilter(const MIDI::MidiMessage& inMessage)
{
if (inMessage.type == midi::Clock)
{
// Consume (don't forward) Clock messages
return false;
}
return true;
}
MIDI.setThruFilter(thruFilter); MapMap 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
Migration from previous APIIt'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();
} |
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 Looking forward to it! |
* resolves FortySevenEffects#40 with franky47's proposed thru filter overhaul * removes thru filter modes * adds thru filter callback * adds thru map callback * old thru filter unit tests have been replicated with filter callbacks * does not yet include documentation changes I believe this implements the latest proposal for FortySevenEffects#40 and exercises everything necessary in the unit tests, including the immutability of `mMessage` after a thru map callback has modified the outgoing message. The thru filter callbacks in the unit tests are not suitable for copying and pasting by end-users due to the difference in the MIDI namespace when setup via the unit tests vs via `MIDI_CREATE_DEFAULT_INSTANCE()`. If the changes here are deemed suitable, I'll work on documentation.
* resolves #40 with franky47's proposed thru filter overhaul * removes thru filter modes * adds thru filter callback * adds thru map callback * old thru filter unit tests have been replicated with filter callbacks * does not yet include documentation changes I believe this implements the latest proposal for #40 and exercises everything necessary in the unit tests, including the immutability of `mMessage` after a thru map callback has modified the outgoing message. The thru filter callbacks in the unit tests are not suitable for copying and pasting by end-users due to the difference in the MIDI namespace when setup via the unit tests vs via `MIDI_CREATE_DEFAULT_INSTANCE()`. If the changes here are deemed suitable, I'll work on documentation.
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
The text was updated successfully, but these errors were encountered: