[pulseaudio-discuss] Updated module-lfe-lp, requesting review/recommendations

[Alexander E. Patrakov]

2013/4/4 Alexander E. Patrakov <patrakov at gmail.com>:
> This e-mail contains only things found by looking at your code. When I
> test it, there will be another e-mail.

OK, tested outside of pulseaudio (so no review of rewinding code).
Good news: if you set Q = M_SQRT1_2, then the output of your lowpass
filter matches the expected output of a 2nd order Butterworth filter
with the given corner frequency (i.e. I compared the output with a
known-good Butterworth filter implementation made for a certain choice
of the corner frequency). And you need Q = M_SQRT1_2 in both stages.
Same for highpass.

However, your use of an allpass filter does not look like a valid
optimization for allpass channels. Let's consider some stereo sound
with the same content in left and right channels. The upmixer in
remix.c will just duplicate them again, so your filter would receive
essentially the same input in all channels. Thus, with this particular
input, your module (with Q = M_SQRT_1_2) implements a proper LR4
crossover filter if the center channel and subwoofer are concerned.

Assuming that all speakers have the same distance to the listener (as
that's the ideal configuration), we would expect no phase difference
between the filtered right channel and the sum of the filtered center
and LFE channels. As the inputs are just the same and the resulting
amplitude gain is 1 in both cases, this means that the allpass channel
output should be equal to the sum of lowpass and highpass channel
outputs. A quick blackbox test shows that it is not the case.

In other words, a filter formed by summing the two outputs or an LR4
crossover filter is a 4th order allpass filter different from the one
obtained by cascading two 2nd order Butterworth allpass filters with
the same corner frequency. So the output of an allpass channel, in
your case, should not be a twice-allpass-filtered input. It should be
the sum of twice-lowpass-filtered and twice-highpass-filtered copies
of input.

And I will repeat (just for the archives) the additional notes that I
have already said on IRC. Yes, you have already answered (1).

1. I think that you are storing duplicate data in the history, because
the output from the first stage is the input to the second stage. This
may or may not be OK.

2. In filter_init_bqdt, you are zeroing the first channel N times.

Again, I must say that I have not reviewed and have not tested the
rewind-related code.

-- 
Alexander E. Patrakov