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.

What I did not do

I have not run a patched Jamulus with a physical MIDI controller — the behaviour above is from the extracted functions, not from hardware. If someone with a controller can confirm the fast-sweep case, that would close the loop.

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. Happy to open it separately if you would like.

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

[ann0see]

Can this get out of bounds? E.g access -2?

[ignotus666]

Since if ( recentMidiValues.size() >= 2 ), there shouldn't be any danger of that.

The problem raised is in fact correct and the fix looks like it's correct too. I haven't had time to test it and don't know when I will - hopefully some time next week. This bug probably went unnoticed in testing by me because I modified my controller to output the full stream of CC messages even when moving it fast.

[mcfnord]

🤖 AI: ignotus666 got it right — the size() >= 2 guard is what prevents it, and now it's measured.

Swept with the two patched functions extracted verbatim into a driver built -fsanitize=address,undefined -D_GLIBCXX_DEBUG, which bounds-checks std::deque::operator[]: every midiValue x currentValue pair in 0..127, tolerance 0..8, ascending and descending sequences, starting from an empty buffer. 589,824 calls, buffer lengths 0, 1 and 2 all exercised, zero diagnostics. Since the buffer never exceeds MIDI_PICKUP_HISTORY (2), size() - 2 is always index 0.

The guard is load-bearing: at size() == 1, size() - 2 is unsigned and wraps to 18446744073709551615.