[pulseaudio-discuss] [PATCH] Remove module-equalizer-sink

Mon Mar 10 07:14:37 PDT 2014

Author here.

Wow, I didn't realize there was so many problems with it (somewhat
sarcastic here).  But I have been using it every day for hours since before
I submitted it and I've stayed up to date with pulseaudio (as a user) with
each successive version as it's been realized on openSUSE.  The only real
issue I've had is that tsched and flash and the equalizer don't really work
together and video games don't like the latency - I never noticed a delay
with mpv/mplayer and videos or anything iritating with music on mpd.  For
those 2 trouble cases of flash/games I just move them to the hardware
sink.  Yea rewinding/seeks of audio cause a small audio glitch but its not
something that bothers me much as it automatically recovers within a
second.  It's worked pretty well for me.  I've seen a few gentoo people
using it but I know it never really reached mass usage due to the
(unfortunate) timing and naming of pulseaudio equalizer (the gtk ladspa
script with non-real-time tuning).  I've liked the large spectrum of the
filter to also have make shift notch filters for when I'm to lazy to
actually load things up in matlab or audacity.

I'll go through this a little bit to answer what I can at the moment (I
just sort of stumbled on to this thread, I'm neck deep in other stuff these
days).

On Fri, Mar 7, 2014 at 11:43 AM, Alexander E. Patrakov
<patrakov at gmail.com>wrote:

> 1. The FFT size and the window size for overlap-add are chosen
> inconsistently. The window size is always 16000 samples. The FFT size
> depends on the sample rate. Thus, with sample rates of 16 kHz or below,
> there is a buffer overflow (window becomes larger than FFT).
>

This was done to allow more detail to be preserved from the signal, or at
least that was the thought.  FFT size  would be the
2^(ceil(log2(sample_rate)) so the spectrum covered all theoretical
frequencies the audio signal at that sample rate could contain.
Granularity of latency would be something that isn't too bad for most
people and still efficient as those FFTs are still expensive.   Perhaps
this should have been based on the sample rate as well, but the latency
wasn't bad for me and cpu usage is overall 1-2% at 44.1khz on an i7.  I
don't recall it being bad (2-5%?) at 96khz back on an i7 920.  I didn't
think one would really would have a sample rate lower than 16khz .

>
> 2. There is no attempt to ensure that the impulse response of the
> equalizer is short enough to fit in the difference between the FFT size
> and the window size (a requirement for the correct operation of the
> overlap-add technique). See [1] for the FFT size consideration and [2]
> why it is important to regularize the desired frequency response.
>

Yes, I recall someone mentioned that theoretically there is "ringing" that
occurs due to this... it's been below the noise floor for me if it
happens.  Has it been a problem for anyone? By the way, maybe I'm
remembering wrong but I believe that tails from the "ringing" cancel out
with COLA method when the filter is constant, that's why I used it and
allowed an arbitrary magnitude based filter (no phase adjustments though).
I'm not an audio engineer though, I mainly due spatial signal processing on
images which is a whole different ball-game.

> 3. The large window size (16000 samples) is unjustified. It only leads
> to excessively high latency. A "33 ms of audio" window and "66 ms of
> audio" FFT size would be sufficient for any practical implementation
> of the equalizer and would still allow one to control the attenuation
> at 30 Hz and 60 Hz separately (as commonly found in music players), all
> with only 33 ms of latency.
>

OK you can tune that if you think it's a problem... I was just balancing
something that worked ok for me while trying not to do too many FFTs, a
respectable default for the general case.

>
> 4. This latency is not properly reported (fdo bug 41465).
>

Some of the harder to understand pulseaudio details like latency escaped me
- I know the concepts very well but I tried for hours to tune it and asked
Lennart countless times for help regarding this (this was in the days when
he was starting the hand off).  Perhaps other bugs or something in PA at
the time made that harder to understand and work with.  If you understand
how to make that part work better, please experiment with it and push a
patch cause it was lost on me at the time.

> 5. There are numerous issues with code style. A lot of commented-out
> code, and a "FIXME: Please clean this up" that was never fixed in 4
> years.
>

I'll see what I can do about that, some of those were from
module-virtual-sink when Lennart was updating it, most of them were
completely unclear as to what should change or if changed resulted in
crashes (the unref one).  Again, Lennart got hard to access in those days
and I couldn't figure out what to do with them as he put them there.   The
rewind stuff was also very confusing overall, I recall this (outside of
module-equalizer-sink) coming up before and no clear answers on the
horizon.  You got me on the dead code one though.  I argue a few of those
lines should be left as they were frequently useful in developing the
filter or diagnosing something - perhaps I should guard them in a debug
macro instead, rather than delete or leave them commented?

>
> 6. There are memory management issues. E.g. the already-pointed-out leak
> at the end of sink_input_pop_cb(), and the fact that a buffer for the
> FFT is allocated by fftw_malloc() but freed by free().
>

Interesting, rather than complain I would've just made a patch but point
taken.  I'll try to submit a patch for that soon but my time for PA efforts
is pretty small.

>
> 7. The use of the Hanning window is unjustified. A rectangular window
> works just as well for the purpose of overlap-add based convolution, but
> allows one to do less FFTs, i.e. go faster. In fact, the DSP Guide book
> does not even mention the variant of overlap-add with a non-rectangular
> window!
>

Here's a few links that might answer it,  I'll also mention most of my work
was derived from these:
http://www.dsprelated.com/dspbooks/sasp/COLA_Examples.html
http://www.dsprelated.com/dspbooks/sasp/Frequency_Domain_COLA_Constraints.html
http://www.dspguide.com/

I don't think it's so broken as you instigate it to be, but maybe I'm
wrong... it does work as desired (as a user) however.

> 8. The module does not use any benefits (such as a chance to handle
> rewinds properly) of being a native PulseAudio module.
>

Please explain how to handle it and I might add it... again there was alot
of black magic in pulseaudio in those days wrt stuff like this.  I suspect
any of the module-virtual-sink inheritors might benefit in exactly the same
way with potentially duplicated code. But I don't know how numerous they
are...

>
> 9. None of the known alternative "system-wide equalizer GUI" projects
> work on top of module-equalizer-sink. Google has no external hits on the
> [EqualizedSinks dbus] search query that would find any alternative GUIs
> that use module-equalizer-sink DBus interface. Both veromix [3] and
> pulseaudio-equalizer [4] work on top of module-ladspa-sink. And they are
> usable with video players, too, even though both are unmaintained.
>

OK...?  is qpaeq a problem for you or did you miss it all together?  I
don't see why this was listed.

>
> 10. It's more an optimization question, but I have a gut feeling that
> overlap-save [5] would be a better fit for PulseAudio buffering model
> than overlap-add. Namely, it would allow to abandon the need to store
> overlap_accum. Instead, the only buffer needed would be for looking back
> at the past portions of the input signal, and we already have that
> anyway due to the need to react to rewinds. In other words, from a
> qualified DSP engineer, I would rather expect a rewrite of the algorithm
> instead of any attempts to improve it gradually.
>
> In short, let's not pretend that PulseAudio has a working native
> equalizer module. It should have never been accepted in the current form
> in the first place. A better replacement already exists in the form of
> module-ladspa-sink + mbeq + veromix.
>

Regarding overlap-save, I evaluated this and specifically show overlap-add
instead, however having been in 2009 or so I can't really remember it so
well anymore.  Maybe it'll come back to me, I suspect will come back when I
look over the dspguide book again.

>
> [1] http://www.dspguide.com/ch18/2.htm
> [2] http://www.dspguide.com/ch17/1.htm
> [3] https://code.google.com/p/veromix-plasmoid/
> [4] http://www.webupd8.org/2013/10/system-wide-pulseaudio-equalizer.html
> [5] http://en.wikipedia.org/wiki/Overlap-save_method
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20140310/0a7e64de/attachment-0001.html>