HarbourMasters / HarbourMasters/Shipwright
aEnvMixerImpl: where do the -4 / -2 wet-path XOR masks come from?
- Dominant language
- C
- Stars
- 5.4k
- Forks
- 837
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 72
Description
While looking at the audio mixer I could not find any reference implementation that matches our wet-path negation, and I would rather ask than guess before anyone touches it.
`soh/soh/mixer.c`, `aEnvMixerImpl`:
```c
int16_t negs[4] = { neg_left ? -1 : 0, neg_right ? -1 : 0, neg_3 ? -4 : 0, neg_2 ? -2 : 0 };
...
*dry[j] = clamp16(*dry[j] + samples[j]);
*wet[j] = clamp16(*wet[j] + ((samples[swapped[j]] * vol_wet >> 16) ^ negs[2 + j]));
```
Three implementations, none of which has the wet masks:
| implementation | dry | wet |
| --- | --- | --- |
| mupen64plus-rsp-hle `alist_envmix_exp` | no negation | no negation |
| sm64-port `src/pc/mixer.c` (what this file descends from) | XOR -1 | no negation |
| here | XOR -1 | **XOR -4 / -2** |
mupen64plus's `ENVMIXER` reads only `A_INIT` and `A_AUX` out of the flag byte and ignores the four low bits that `aEnvMixer` packs `x0..x3` into. sm64-port has `negs[2]`, not `negs[4]`, and no negation on the wet path at all.
The history does not settle it: the file arrives in libultraship already in this form (Kenix3/libultraship@e42bd5a3, 2022-02-22, @AloXado320), and the only parent present in that history does not contain the file, so there is no earlier state to diff against. It moved here in #2361 / Kenix3/libultraship#99. blawar/ooot has byte-identical code, but got it from here (`added SoH mixer.c to fix audio issues`, 2022-03-23), so it is not independent corroboration.
Two questions:
1. Is the wet-path negation modelled on something concrete -- microcode disassembly, a hardware comparison, a specific bug it fixed -- or is it an artefact?
2. XOR with `-4` (`0xFFFC`) and `-2` (`0xFFFE`) is not a negation of anything: it flips every bit except the low two and the low one respectively. If the intent was "negate", `~x` (XOR `-1`) is the near miss and `-x` the exact one. For what it is worth, `4` and `2` are exactly the bit values of `x1` and `x2` in the command-word packing, which is the shape a flag/mask mix-up would leave -- but that is speculation and I would not act on it.
I am deliberately **not** proposing a patch. With mupen64plus doing no negation at all and sm64-port doing none on the wet path, "correct" has at least two candidate answers, and switching to a plain `* -1` would just be a third guess. Measured against the current behaviour, a plain negate differs by up to 4 LSB (mean 2.67) for the `-4` mask and up to 2 LSB for `-2`, so this is inaudible either way -- it is a correctness question, not an audible bug.
Flagging it because the same code is in 2ship2harkinian, Starship, SpaghettiKart, Ghostship and Lighthouse: if it is wrong, it is wrong in all of them.
@Kenix3 @AloXado320 -- do either of you remember where this came from?
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.