Fix MIDI pickup crossing detection never firing

[mcfnord]

Note

📡 STAND BY FOR AN LLM-AUTHORED MESSAGE.

MIDI pickup mode has a second acceptance path beside the tolerance test, for when a fast fader move makes consecutive MIDI values skip straight over the software value:

// If buffer has at least 2 values, check if we're crossing the current value
// Handles the case where rapid movement causes MIDI values to "skip over" the software value
if ( recentMidiValues.size() >= 2 )
{
    int prevMidi = recentMidiValues.back();
    ...
}

It has never run. There are two independent defects, and either one alone is enough to disable it.

1. The history is never recorded while waiting. midiPickupTryApply builds a temporary copy, tests it, and on failure returns early — jumping over the // Update the pickup buffer block at the end of the function. So while waitingForPickup is true, which is exactly the state the history exists to serve, pickupBuffer stays empty. The largest deque ever passed to midiPickupShouldApply is 1, so size() >= 2 is never satisfied.

2. prevMidi is not the previous value. recentMidiValues.back() is the element midiValue was just push_back'd as, so prevMidi == midiValue and the bracket test reduces to midiValue == currentValue — already covered by the tolerance test three lines above. Sweeping (prev, current, new) over 0..100 outside tolerance, the current code detects 0 of 323400 genuine crossings.

The size() >= 2 guard is itself the evidence of intent: you only need two elements if you mean to look past the last one.

What it costs a user

CSoundBase::ParseMIDIMessage maps CC 0..127 onto fader 0..AUD_MIX_FADER_MAX (100), so one CC step is 0.79 fader units against a MIDI_PICKUP_TOLERANCE of 2. Slow moves are picked up by the tolerance test and the bug stays invisible. Fast moves are where it shows: sweeping a hardware fader past a software fader sitting at 50, the fader stays dead at several speeds until the user slows down and lands within ±2 units.

CC step	before	fix 1 only	fix 2 only	this PR
8	picks up (9 msgs)	picks up (9)	picks up (9)	picks up (9)
10	never picks up	never	never	picks up (8)
13	picks up (6)	picks up (6)	picks up (6)	picks up (6)
20	never picks up	never	never	picks up (5)
24	never picks up	never	never	picks up (4)
32	picks up (3)	picks up (3)	picks up (3)	picks up (3)

The two middle columns are the reason this PR changes two things rather than one: fixing either defect on its own changes nothing measurable.

Checks

Harness compiles the two patched functions verbatim out of audiomixerboard.cpp rather than a re-typed copy, so what is measured is what is shipped.

• All previously stuck sweep speeds now pick up.
• Regression on the property the feature exists for: pickup must never engage on the first message when the hardware fader is far from the software fader. Across all 12928 combinations of software position 0..100 and CC 0..127 outside tolerance, first-message jumps: 0.
• clang-format 14.0.6 reports no change; compiles clean under -Wall -Wextra.

Runtime, through the shipped client. A JACK MIDI source — not physical hardware, but real MIDI arriving on the port CSound::CreateMIDIPort opens — feeds a GUI client with one channel fader parked at 50 and Pick-up Mode on, and the observable is the CHANNEL_GAIN message the client puts on the wire. Two control-change messages 298.12 ms apart carrying CC 51 and CC 76 — fader 40 and 59, bracketing the parked 50, both outside tolerance — produce no gain message and no fader movement on main at 72e856af. The same two messages on that tree with this PR's two edits applied pick up and send the gain for 59. Sweeping that same span one CC step at a time on main instead picks up at 48, the tolerance edge, and tracks from there: 12 gain messages. Reproducers, including one that needs no MIDI hardware at all: this gist.

What I did not do

One related thing I noticed and deliberately left alone: g_midiPickupWaitingForPickup is a single flag per channel shared by the fader and the pan, while the history deques and timestamps beside it in MidiPickupState are correctly per-control (recentFader/recentPan, lastMidiTimeFader/lastMidiTimePan). That means picking up the pan also clears the fader's waiting state, and either control's inactivity timeout re-arms both. It needs two flags rather than one, which is a different change from this one, so I have not folded it in. I can open it separately if you would like.

CHANGELOG: Bugfix: MIDI pickup mode now correctly detects a fader crossing the software value during fast moves.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: QUIET

Plan: Advanced

Run ID: e4f682e2-f349-4e1f-aacd-c5fe9f2dd912

📥 Commits

Reviewing files that changed from the base of the PR and between 9779918 and 938faf0.

📒 Files selected for processing (1)

• src/audiomixerboard.cpp

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The MIDI pickup logic now preserves pending MIDI values and uses the second-to-last history entry for crossing detection.

Changes

MIDI pickup behavior

Layer / File(s)	Summary
Preserve pickup history and detect crossings src/audiomixerboard.cpp	midiPickupShouldApply compares against the second-to-last MIDI value. midiPickupTryApply stores the updated history buffer while pickup remains pending.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 938fa

The pickup crossing fix is ready to merge; normal build and runtime checks remain sufficient.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: fixing MIDI pickup crossing detection.
Description check	✅ Passed	The description provides a detailed change summary, explains the two defects, documents user impact, records validation results, includes a CHANGELOG entry, and identifies an out-of-scope issue. It do…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}