Updates to filtering work. Review requested.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2013/4/20 Justin Chudgar <justin at justinzane.com>:
> https://github.com/justinzane/pulseaudio

Sorry for the delay with the answer. I was really busy at work, and
will be very busy for at least two more weeks due to a new employee
who needs guidance duing his initial work period. Still, here is the
review.

1. I don't like the fact that the interface to construct a biquad
filter (i.e. to calculate the proper coefficients) and the interface
to do the filtering given the coefficients are in the same file. The
current interface is good only for crossover filters and similar
tasks. It is too limited for tasks that may arise in the future (e.g.
replacement of the current 5.1-to-headphones filter with IIR would
probably result in a good speedup, but a completely custom filter, not
Butterworth, and not constructed according to any common cookbook
formula). So let's not try to guess what the general interface would
be, but let's move the current pa_biquad_calc_factors function to some
less generic location, relevant only to Butterworth filters and LR
crossovers.

2. Please remove options to create a high-shelf or other types of
filters not relevant to your current work. They can be introduced when
needed.

3. Please remove code that calculates coefficients for non-LR4
filters. It cannot be tested (because pa_biquad_chunk_4 assumes two
stages) => it is useless now. Again, this can be introduced together
with the first user.

4. My previous comment about an allpass filter being an invalid
optimization has been ignored. Once again, the output of an allpass
filter is not in phase with the output of a lowpass or highpass filter
of the same order. So you cannot use it. For allpass channels, please
use the sum of outputs of lowpass and highpass filters instead.

I have not otherwise tested the correctness of the code and have not
investigated what has been plotted.

-- 
Alexander E. Patrakov


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux