Ignore clock and active sensing on serial MIDI

[JennToo]

Fixes #416

I'm not entirely sure this is the correct way to fix it. But it does seem to fix the problem I was seeing. I can now completely mash the keyboard and no notes will get stuck on.

[BobanSpasic]

Dunno how MiniDexed process the active sensing, but active sensing should be there to detect if the MIDI signal gets lost (cable disconnected. the sending devices turned off etc.) - if there is no active sensing for some predefined time, all notes should turn off.

[diyelectromusic]

Dunno how MiniDexed process the active sensing, but active sensing should be there...

I think the operative word there is should :) I only seem to have one device that uses ActiveSensing - my Roland UM-ONE PC USB to MIDI interface! Nothing else I have seems to bother with it, and I regularly filter it out myself.

So having configuration options to ignore real-time messages I'm sure would be useful for situations like this. But it would have to be a configuration option I believe - and what granularity - just AS/Clock or all real-time for example - I don't know - up for discussion I guess?

And should they still be forwarded on via software THRU I wonder - that may negate any savings from being able to ignore them...

With regards to your fix @JennToo that filters it out at the serial MIDI layer, which catches it pretty early in the processing chain, so we'd probably need to consider it in the USB MIDI driver too (and as I say make it configurable).

It looks like both messages are ignored by the time they get to the MIDI message handling layer anyway, so it is interesting that this solves your issue... it implies the MIDI message handling still has quite an overhead which is curious! Although I also note it will have hit the software THRU at this point, even though it isn't handled further... so maybe that is part of the performance optimisation going on?

Kevin

[JennToo]

it implies the MIDI message handling still has quite an overhead which is curious!

I think it's actually not a throughput / load issue, but instead something is wrong with the state machine just below this change. I suspect what's happening is these messages are getting interspersed with other messages and it's causing the parsing to break down. It's hard to tell though, the log traces scroll by way too quickly when this stuff is enabled.

I think passing this message up to the semantic layer above wouldn't be a problem. I'll give that a try.

[Banana71]

The good old Roland devices: My Roland RD-300 and Roland MKB-300 also send "Active Sensing" at short intervals and repeatedly caused hanging notes. #356
Your version completely solves this problem for me.
Many many thanks @JennToo

[JennToo]

Ah! I think I've confirmed what the issue is. From the MIDI spec:

Real-Time messages may be sent at any time — even between bytes of a message
which has a different status. In such cases the Real-Time message is either acted
upon or ignored, after which the receiving process resumes under the previous
status

I suspect what's happening is we're hitting this case in the state machine:

MiniDexed/src/serialmididevice.cpp

Line 128 in 24e10c3

if (uchData & 0x80) // got status when parameter expected

Which throws everything off the rails.

I'm going to adjust the filter I've added to catch all the Real-Time messages, not just these two. And I'll also retest with adding the message passing up to the MIDIMessageHandler.

[JennToo]

I retested with handling all the system real-time messages (though I suspect my system is only using active sense and clock), and also passing the messages to the MIDIMessageHandler seemed to work fine as well.

I can't seem to find where the equivalent of this framing logic lives in the USB stuff. It might be buried down in the circle code. Regardless, I don't feel comfortable making a fix for that myself since I don't have a USB device that can reproduce the problem (assuming it even is a problem on the USB side).

[diyelectromusic]

Oh well spotted :)

Yes, I don't really know why there is pseudo-parsing going on in there. You'd think it would be fairly "dumb" at this level.

The USB handling is done in midikeyboard.cpp and it just registers a series of packet handlers against the USB MIDI device and pretty much just sends them straight off to the MIDI message handler...

[probonopd]

Thanks @JennToo.
So, before this patch, does the issue exist for USB as well? Does anyone have the means to test?

[diyelectromusic]

Thanks @JennToo. So, before this patch, does the issue exist for USB as well? Does anyone have the means to test?

I can't test it, but I don't think it does. The USB MIDI code in circle receives complete packets from the USB stack and passes them directly off to the MIDI parsing. In fact the USB MIDI standard puts single 1,2,3 byte MIDI messages into single 32-bit transfers, so is much more "atomic" anyway - it has no need for "running status" for example (I think SysEx can be split... not sure...)

"MIDI data is transferred over USB using 32-bit USB-MIDI Event Packets. These packets provide an efficient method to transfer multiple MIDI streams with fixed length messages. The 32-bit USB-MIDI Event Packet allows multiple "virtual MIDI cables" routed over the same USB endpoint. This approach minimizes the number of required endpoints. It also makes parsing MIDI events easier by packetizing the separate bytes of a MIDI event into one parsed USB-MIDI event." (USB MIDI v1.0)

The serial MIDI code has to construct "complete packets" (i.e. complete MIDI messages) itself and send them off to the MIDI parser - and was getting it wrong :)

[probonopd]

Thank you very much!

[JennToo]

Sure thing! And thanks for the great project, it's been really awesome!