Saturate the IMA ADPCM predictor instead of wrapping it

[dqIndieGames]

Fixes #19.

Note: this description was edited after merge to correct two mistakes in the original text — a wrong sample count backed by a tautological test, and a false claim that the repo has no test project. Details at the bottom. The code change itself is unaffected.

The bug

FmodImaAdPcmRebuilder.ExpandNibble casts the decoded predictor to short before clamping it:

hist = Utils.Clamp((short)sampleDecoded, short.MinValue, short.MaxValue);

Utils.Clamp is declared as Clamp(short val, short min, short max), so the caller is forced to cast the int down to short first. In C# an unchecked (short) cast truncates — it wraps an out-of-range value around instead of saturating it. And once the wrap has happened, the Clamp is dead code, because a short is always within short range.

So a sample that should have saturated to +32767 comes out as unclamped - 65536: a full-scale jump to the opposite rail, audible as a loud click/pop.

ExpandNibble is shared by DecodeSamplesFsbIma and DecodeSamplesXboxIma, so both mono and stereo were affected.

The fix

The repo already contains the correct helper — Utils.ClampToShort(int), which range-checks before casting — and FmodFadPcmRebuilder already uses it. The IMA ADPCM rebuilder just didn't:

- hist = Utils.Clamp((short)sampleDecoded, short.MinValue, short.MaxValue);
+ hist = Utils.ClampToShort(sampleDecoded);

This also matches the IMA ADPCM spec, which requires the predictor to be clamped to int16 range, never wrapped.

Verification

Corpus: 157 mono IMA ADPCM FSBs (32 / 44.1 / 48 kHz) from a shipped game. Ground truth: the real FMOD library.

1. The patched decoder matches FMOD exactly.
All 157 FSBs through the patched FmodImaAdPcmRebuilder.Rebuild, compared sample-by-sample against FMOD:

59,997,056 samples — 0 differences, 0 clips differing.

2. The bug is exactly what breaks the current output.
A faithful reimplementation of master's (pre-fix) decoder reproduces the corrupted output bit-for-bit (0 differences) — confirming the damage comes from this cast and nothing else. Against FMOD, that same pre-fix decoder differs on:

62,029 samples across 107 of the 157 clips.

3. Causal attribution is clean.
Of the 1,692 frames containing any difference, the first differing sample lands on a saturation point (predictor pushed out of int16 range) in 1,692 / 1,692 cases — zero unexplained. Saturation events in the corpus: 10,767.

The corrupted-sample count is ~6× the number of saturation events because IMA ADPCM is differential: once hist is wrong, every subsequent sample in the frame inherits the error until the next frame header resets the predictor. That's why one overflow is audible as a distinct "pop" rather than a single-sample tick.

Notes

• No behaviour change for samples that were already in range. The only samples affected are the ones that previously wrapped — which were broken.
• Tests: the repo does have a test project (Fmod5Sharp.Tests) with IMA ADPCM tests and fixtures. I checked all three existing IMA fixtures (imaadpcm_short.fsb, imaadpcm_long.fsb, xbox_imaad.fsb): none of them ever drives the predictor out of int16 range, so they decode identically before and after this change and cannot catch this bug. A regression test would need a synthetic FSB crafted to overflow the predictor. Happy to add one if you'd like it.
• I deliberately left the three stepIndex = Utils.Clamp((short)stepIndex, 0, 88); calls alone. They use the same suspicious pattern, but stepIndex stays within [-1, 91] in practice (the -1 comes from IMA_IndexTable and is immediately clamped to 0), so the cast is harmless there — just redundant. Happy to clean those up too.
• Builds clean on netstandard2.0 locally (0 warnings, 0 errors); I only have .NET SDK 7 here so the net8.0/net9.0/net10.0 targets were left to CI.

---

Corrections to the original description

Two things in the first version of this description were wrong. Correcting them here rather than quietly leaving them:

1. "the repo has no test project" — false. Fmod5Sharp.Tests is right there in the solution, with IMA ADPCM tests and fixtures, and CI runs dotnet test. What I should have said is that the existing IMA fixtures don't trigger the overflow, so they can't catch this — which is what I verified and stated above.

2. "11005 samples must saturate / 100.00% of them wrapped / zero exceptions" — the 100% was a tautology. The test I used was wrapped_value + 65536 > 32767, which is true for every possible int16 (v ≥ -32768 ⟹ v + 65536 ≥ 32768). It therefore reported "100.00%" no matter what the data was — running it on correctly decoded audio also prints 100.00%. The count 11,005 was also the wrong quantity (samples whose output sits on the int16 rail, not samples whose predictor overflowed).

   The numbers above (10,767 saturation events; 62,029 corrupted samples across 107/157 clips; 1692/1692 frames whose first divergence lands on a saturation point) come from direct comparison against the real FMOD library and do not rely on that test. The 62,029 / 107-clip figures were unaffected by the bad test and reproduce exactly.

The root cause, the fix, and the conclusion that the patched decoder is sample-identical to FMOD all stand — verified independently above.