On Mon Mar 2 18:30:28 2026 +0000, Matteo Bruni wrote:
I find the variable naming a bit confusing here. Before the previous MR this was probably called `rem` as in "remainder", as it effectively was the fractional part of `(freqAcc_start + i * dsb->freqAdjustNum) * dsbfirstep / dsb->freqAdjustDen`. Now it's a bit of a misnomer since it's `1.0f - <remainder>` of the new division, while `rem_inv` is the actual remainder. I'd make use of the fact that this is also the `t` parameter of the linear interpolation to rename the variable and get rid of the "rem" confusion altogether. Or at least that's how I understood it. Maybe you have a better explanation for the (preexisting) name. :sweat_smile: I was thinking of `rem` in terms of calculating the value of a continuous FIR function `f(x)`: we split `x` like this: `x = x * fir_step / fir_step = (floor(x * fir_step) + fmod(x * fir_step), 1.0)) / fir_step` so `idx = floor(x * fir_step)` and `rem = fmod(x * fir_step, 1.0)`.
Note that the filter coefficients are accessed in reverse order compared to what a convolution would give. This is where `fir_step - 1 - ...` and `1.0 - ...` come from. It doesn't affect the resulting value because the FIR is symmetric, and it allows the FIR and the input array indices to advance in the same direction, which will be very useful for SIMD. It was already like this in the original code. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10217#note_131052