fix: prioritize MIDI realtime output - #631
Conversation
4372e07 to
d56e82f
Compare
ArthurGibert
left a comment
There was a problem hiding this comment.
Splits the shared MIDI output channel into three (notes/CC, clock, transport) so note/CC backlog can no longer starve clock ticks or Stop/Start. This is a genuine, well-targeted fix for real head-of-line blocking.
Hardware validation: flashed on real hardware and stress-tested well beyond this PR's original scope — 16 concurrent LFO apps (several in NRPN mode) continuously flooding the app-level MIDI queue (millions of intentional, by-design drops per send_cc's own drop-on-full-queue policy, confirmed not a bug). Across 1.2M+ log lines under this load, the clock priority channel recorded zero drops (verified via temporary instrumentation), the physical tempo LED stayed visually steady, and external MIDI monitoring confirmed clean clock signal on both USB and DIN outputs throughout. Confirms the PR's core mechanism works: clock delivery isn't starved by note/CC congestion, even at a load far beyond realistic use.
Residual gaps, suggest tracking as follow-up rather than blocking:
midi.rs:127—MIDI_CLOCK_CHANNELdepth 16 lets stale ticks back up during a USB stall specifically, then burst-fire on recovery — contradicts its own doc comment. (Not exercised by this hardware test, which didn't stall the USB host.)midi.rs:393— sustained clock/transport traffic can starveMIDI_CHANNEL, backpressuringmidi_distributorand potentially freezing an app task mid-send_note_on/send_cc.midi.rs:360— nothing flushesMIDI_CLOCK_CHANNELon Stop/Start/Reset, so stale ticks can play out after Stop.clock.rs:463— the transport channel's blocking send reproduces the same stall-propagation bug this PR fixes, just relocated — a full transport channel now stalls CV clock output too.- Minor:
midi.rs:357's manualtry_receivepre-check is redundant withselect4's own fixed poll order. - Minor, pre-existing:
write_msg_to_uart0has no timeout, unlike the USB path.
A related investigation (an unconfirmed user jitter report) additionally found that once the dispatcher commits to a write it isn't preemptible — up to ~28ms worst case for an NRPN write under a USB stall — and produced a fix for that plus #2 above, now on branch fix/midi-jitter-on-realtime-priority (pushed, not yet a PR).
Recommend merging as-is given the hardware validation above, and tracking the remaining gaps as documented follow-up.
d56e82f to
8862ec6
Compare
Summary
Supersedes #599.
Verification
cargo fmt --all -- --check, firmware clippy, and./build-uf2.shpass.Hardware test