[pulseaudio-discuss] [PATCH 1-3/8] Summary for easier reviewing

[Alexander E. Patrakov]

24.03.2015 14:29, David Henningsson wrote:
> This is a summary of patch 1, 2, 3 for easier reviewing, as requested by Alexander.

This time, there is even less dead code, and the formulas still produce 
the same result as those published at 
http://www.musicdsp.org/files/Audio-EQ-Cookbook.txt (referenced below as 
"their formula", "their version" and so on).

> +	} else if (cutoff > 0) {
> +		/* Compute biquad coefficients for lowpass filter */
> +		double d = sqrt(2);
Looks like their 1/Q

> +
> +		double theta = M_PI * cutoff;
Same as their w0

> +		double sn = 0.5 * d * sin(theta);
Same as their alpha

> +		double beta = 0.5 * (1 - sn) / (1 + sn);
> +		double gamma = (0.5 + beta) * cos(theta);
> +		double alpha = 0.25 * (0.5 + beta - gamma);
No exact equivalents in their terms. For the purpose of comparing the 
formulas, it is useful to introduce K = 0.5 + beta, which obviously 
simplifies down to 1 / (1 + sn). Then:

beta = K - 0.5
gamma = K * cos(theta)
alpha = 0.25 * K * (1 - cos(theta))

> +
> +		double b0 = 2 * alpha;
What matters is b0 / a0, with our a0 = 1.

Our version: 0.5 * K * (1 - cos(theta))

Their version: 0.5 * (1 - cos(theta)) / (1 + sn)

...which is the same.

> +		double b1 = 2 * 2 * alpha;
The comparison can be saved here, because this is the same as 2 * b0, 
and b0 is already known to match.

> +		double b2 = 2 * alpha;
This is the same as b0, which is already known to match.

> +		double a1 = 2 * -gamma;
What matters is a1 / a0.

Our version: -2 * K * cos(theta), which, after substituting K = 1 / (1 + 
sn), is exactly their result.

> +		double a2 = 2 * beta;
What matters is a2 / a0. In both our code and their cookbook, this is 
quite obviously (1 - sn) / (1 + sn). Match!


> +
> +		set_coefficient(bq, b0, b1, b2, 1, a1, a2);
> +	} else {
> +		/* When cutoff is zero, nothing gets through the filter, so set
> +		 * coefficients up correctly.
> +		 */
> +		set_coefficient(bq, 0, 0, 0, 1, 0, 0);
> +	}
> +}
> +
> +static void biquad_highpass(struct biquad *bq, double cutoff)
> +{
> +	/* Limit cutoff to 0 to 1. */
> +	cutoff = PA_MIN(cutoff, 1.0);
> +	cutoff = PA_MAX(0.0, cutoff);
> +
> +	if (cutoff >= 1.0) {
> +		/* The z-transform is 0. */
> +		set_coefficient(bq, 0, 0, 0, 1, 0, 0);
> +	} else if (cutoff > 0) {
> +		/* Compute biquad coefficients for highpass filter */

And now compare with the lowpass filter. You'll see the differences for 
edge cases (which are invalid for our purposes, are already eliminated 
in lfe-filter.c and thus can be either dropped here or turned into 
asserts), in the sign before gamma in the definition of alpha, and in 
the sign of b1. So it may be a good idea to consider merging the two 
functions together. But, this is just a nitpick.

> +void lr4_process_float32(struct lr4 *lr4, int samples, int channels, float *src, float *dest)

Yes, it was a good idea to remove in-place processing.

-- 
Alexander E. Patrakov