I was quite surprised finding out that MIDO cannot handle running status. Such a basic MIDI feature…

I am writing a new back-end and was feeding the parser with midi bytes using running status and wondered why only the first event is accepted.

@Jajcus did you test if the fixes from this PR work for you? Might be good to get some feedback.

@m-vo No, I have not. I wanted to make my code work with the released version of mido, so I implemented a workaround even before I found this merge request.

Per the official midi.org specification, page 5, running status is "For Voice and Mode messages only" aka Channel Messages.
System messages (Common, Real-Time and Exclusive) are not Running Status enabled and should exit running status mode.
Same goes for undefined Status bytes.

I don’t see such filtering in your code.

I made a mistake in my previous comment. Real-Time messages are a special case and the only ones that should not reset/exit the Running status.

Real-Time messages are a special case and the only ones that should not reset/exit the Running status.

The code only runs in _feed_data_byte, so IMHO this should not affect SysEx and other messages at all. We could in theory reset self._last_status in certain cases, but I didn't find anything in the spec about this. Do you see any downsides in the current implementation that we should address?

The specs says the reverse.
Quote from MIDI 1.0 specification page 5 (10 in PDF form): "For Voice and Mode messages only" and "Running Status will be stopped when any other Status byte intervenes."

Common messages can have data bytes (F1, F2, F3) and SysEx has data bytes but are not Running Status enabled.

How does this test perform?

def test_running_status_common():
    """The last known status byte should not be reused with common or exclusive message"""
    assert tokenize([0xf3, 1, 2]) != [
        [0xf3, 1], [0xf3, 2]
    ]

To be spec compliant it should pass and from just reading the code, I believe it fails.
The second data byte should be considered invalid and not yield another Song Select message.

FWIW I have a spec compliant Running Status implementation part of a work in progress to fix #363. I can extract/PR it since it seems to work fine while I continue working on the EOX issue which is more hairy.

Thanks for the explanation. I must have jumped over the "Running Status will be stopped …" part each time. 🙈

FWIW I have a spec compliant Running Status implementation part of a work in progress to fix #363. I can extract/PR it since it seems to work fine while I continue working on the EOX issue which is more hairy.

Sounds good. 👍 Closing this PR in favor of #363 then.

If anyone is willing to review my code, I just pushed my work in progress: https://github.com/EMATech/mido/tree/eox

The relevant running status implementation is there: EMATech@c6e13e5#diff-657b1756ea262acda7e2095a923fc3f18e64343552164609e0b5badd451151b4

If anyone is willing to review my code, I just pushed my work in progress: https://github.com/EMATech/mido/tree/eox

The relevant running status implementation is there: EMATech@c6e13e5#diff-657b1756ea262acda7e2095a923fc3f18e64343552164609e0b5badd451151b4

Thanks. Please consider opening a (draft) PR, because IMHO it's way easier to review this way.

@m-vo Thanks for the suggestion, done! I didn't know "Draft PR" were a thing.