Skip to content

fix: prioritize MIDI realtime output - #631

Merged
ArthurGibert merged 2 commits into
mainfrom
fix/midi-realtime-priority
Aug 14, 2026
Merged

fix: prioritize MIDI realtime output#631
ArthurGibert merged 2 commits into
mainfrom
fix/midi-realtime-priority

Conversation

@chmanie

@chmanie chmanie commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

  • isolate MIDI timing-clock ticks from app note/CC traffic
  • deliver transport through a reliable queue ahead of clock ticks
  • bound stale tick backlog while keeping clock generation non-blocking

Supersedes #599.

Verification

cargo fmt --all -- --check, firmware clippy, and ./build-uf2.sh pass.

Hardware test

  • Generate heavy note/CC traffic while sending internal clock; confirm stable BPM externally
  • Confirm Start, Stop, Continue, and Reset reach each configured MIDI output
  • Confirm normal note/CC output under light load

@ArthurGibert ArthurGibert left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. midi.rs:127MIDI_CLOCK_CHANNEL depth 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.)
  2. midi.rs:393 — sustained clock/transport traffic can starve MIDI_CHANNEL, backpressuring midi_distributor and potentially freezing an app task mid-send_note_on/send_cc.
  3. midi.rs:360 — nothing flushes MIDI_CLOCK_CHANNEL on Stop/Start/Reset, so stale ticks can play out after Stop.
  4. 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.
  5. Minor: midi.rs:357's manual try_receive pre-check is redundant with select4's own fixed poll order.
  6. Minor, pre-existing: write_msg_to_uart0 has 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.

@chmanie
chmanie force-pushed the fix/midi-realtime-priority branch from d56e82f to 8862ec6 Compare August 14, 2026 09:38
@ArthurGibert
ArthurGibert merged commit 4004b17 into main Aug 14, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants