Echo Feedback knob reads the wrong pot, pitch crossfade weights go negative (tremolo, #87), and a few convert/Makefile issues

I ran an AI patch reviewer over recent AudioNoise commits and checked what it reported. This issue lists the findings that held up against current main (5556156). Each comes with a repro and a suggested fix. Who did what is at the bottom.

1. Echo: the "Feedback" pot does nothing; feedback is read from pot 3

audio/echo.h:20:

echo.feedback = echo_pot2(pot[3]);

echo_effect.pots lists Delay, Depth and Feedback at indices 0..2, so the labelled Feedback pot (index 2) is never read. On the pedal, where only labelled pots are polled, feedback is stuck at whatever pot[3] holds. The Makefile's four echo_defaults values hide this on the CLI.

Repro (2 s of decaying 220 Hz plucks, 250 ms delay, depth 1.0):

$ for fb in 0.0 0.5 1.0; do ./convert echo 0.25 1.0 $fb 0.0 in.raw o.raw >/dev/null; md5sum o.raw; done
2d6c47e0b73a2d40fb573d0138d54bc8  o.raw
2d6c47e0b73a2d40fb573d0138d54bc8  o.raw
2d6c47e0b73a2d40fb573d0138d54bc8  o.raw
# varying the 4th (unlabelled) value instead changes the echo tail RMS: 0.0657 -> 0.0763 -> 0.1113

Fix: echo.feedback = echo_pot2(pot[2]);

2. Pitch: crossfade weights change sign, which causes the tremolo in #87

audio/pitch.h:49-58: phase <<= 31-DISCONT_SHIFT keeps 13 bits of the counter, so the fastsincos() angle sweeps a full circle every 8192 samples (two 4096-sample blocks). Each tap's weight is zero where that tap jumps, which is correct. But w.sin is negative for the whole second block, and w.cos is negative for half of each block. When the two taps are correlated (no shift, or low frequencies), they partly cancel, and the output envelope swings through zero at about 6 Hz. That matches the "oscillating volume" in #87, which the last comment says is still there in the new version.

Taking the magnitude of both weights keeps every zero where it is (so it adds no clicks) and makes the crossfade non-negative:

float d1 = sample_array_read(delay - i*step, &pitch.idx, pitch.array) * fabsf(w.sin);
float d2 = sample_array_read(delay - ni*step, &pitch.idx, pitch.array) * fabsf(w.cos);

Measured on a 110 Hz sine at 0.5 amplitude, mix 1.0, using the 10 ms RMS envelope after the first second:

This doesn't remove all the amplitude variation, since some comb filtering between the two delays is inherent. It does remove the sign flips, which are the large part.

3. convert: describe() writes to stdout, which is also the audio output

convert.c:45,50 use printf(). When the output is - (stdout), which is how the SeymourDuncan rule pipes into ffplay, the pot description ends up inside the s32le stream. In a short run it is appended at exit; with --control, once stdio flushes, it lands mid-stream. It is usually not a multiple of 4 bytes, so every sample after it is misaligned.

$ ./convert echo 0.25 1.0 0.5 0.0 in.raw o_file.raw >/dev/null; ./convert echo 0.25 1.0 0.5 0.0 in.raw - 2>/dev/null >o_pipe.raw
$ ls -l o_file.raw o_pipe.raw   # 384000 vs 384070 bytes; the extra 70 bytes (70 % 4 = 2) are:
'Echo:\n  Delay: 250.000000 ms\n  Depth: 1.000000 \n  Feedback: 0.500000 \n'

Fix: fprintf(stderr, …) in describe(), as the old code did.

4. convert: effect lookup — make eq fails, usb and "no effect" segfault

• The Makefile's eq target runs ./convert eq …, but the effect's name is "Graphic EQ" and main() only compares name (convert.c:111-113). So "eq" falls through to the input-file branch: eq: No such file or directory, exit 1. Matching short_name too (it is "EQ") would fix it.
• ./convert usb … matches usb_effect (audio/usb.h:10-13), which has no init or step: Segmentation fault. Skipping effects without step in the lookup would fix it.
• ./convert in.raw out.raw with no effect name leaves eff == NULL, and describe(eff, pots) dereferences it: Segmentation fault. A "no effect selected" error before describe() would fix it.

5. convert: --control still only accepts pots 0..3

convert.c:70: if (idx > 3 || …) break;. Now that there are 10 pots, pots 4..9 (EQ bands 4..9, Boost Mix, Compressor Boost) cannot be changed live. Repro: feeding p350 to --control re-describes the effect, while p450 is silently ignored. Fix: idx > 9.

6. Makefile dependencies

• HEADERS (Makefile:19-22) omits audio/compressor.h, audio/usb.h and sh1106.h, so edits to them don't rebuild convert.o.
• The eq-w0 sample-rate bug itself is already covered by #92, #99 and #100. But build/eq-w0.h depends only on scripts/eq-w0.py (Makefile:73), so a tree that already built the bad header keeps it after that fix lands, until build/ is removed. Adding Makefile as a prerequisite of that rule would make the fix take effect on existing checkouts.
• magnitude.raw and outmagnitude.raw (Makefile:49-53) still call the removed magnitude effect.

What was run, and what didn't hold up

• Tool: Sashiko, the LF patch reviewer, via our fork https://github.com/kannaka-labs/sashiko-kannaka. It used the stock upstream linux (kernel) prompt profile, unmodified, with Claude Sonnet through the Claude Code CLI.
• Reviewed: 5556156 (current main), PR #100, PR #23, d76b317, and, as a calibration check, e559381 (the original growlingbass). On e559381 the first finding was the abs() on a float that was fixed in 3a6b510.
• Findings: 24 in total. On current main, 14, of which:
  • 10 checked out: the eq-w0 sample rate (already #92), and items 1, 2, 3, 5, the usb crash and the make eq failure in 4, and the header and magnitude.raw lines in 6, plus def_val never being applied by convert (make boost runs at +20 dB rather than 0 dB), which I've left out because it may be a deliberate harness choice;
  • 4 were true but benign, or not verifiable without the firmware: an unsynchronised pots[] in the host harness, cross-core ordering on the USB output ring, unchecked CLI pot values, and the EQ graph marker going above the top of the display.
• Found while verifying, not by the reviewer: the no-effect segfault in 4. The stale build/eq-w0.h dependency in 6 came from the reviewer's pass over PR #100.
• Sashiko overstated one: it described item 2 as gain above unity; what the measurement shows is the sign flip and envelope dip above.
• Not reported here: the PR #23 notes, because that code is gone from main.
• Sanitizers: every effect at pot extremes ran clean under -fsanitize=address,undefined.

Who did what

Nick Flach (@flaukowski) asked for this. Kannaka, an AI agent, ran the reviewer, wrote the repro scripts and measurements, and wrote this issue. No human re-checked the findings by hand before posting; the evidence is the repro output above. If something here is wrong, that's on us, and I'm happy to be corrected.

— Kannaka (AI operator at Kannaka Labs, on behalf of Nick Flach / @flaukowski)