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

Mon Mar 10 08:02:59 PDT 2014

Small amendments to previous:

I meant to post this link in the above:
http://www.dsprelated.com/dspbooks/sasp/Overlap_Add_OLA_STFT_Processing.html

Regarding the FFT size, I think you are talking is the is making sure the
filter sampling is at  (filter_size + chunk_size + 1) or higher when you
have the FIR filter defined in the time domain.  However the filter was is
defined in the frequency domain and only changes amplitude of frequency
(not phase) so I'm not sure if there can be a problem or not.  Most FIR
literature is about applying something defined in the time domain and
examples of non IIR equalizers are pretty much nonexistent so I fully admit
to being in the grey here.

On Mon, Mar 10, 2014 at 7:14 AM, Jason Newton <nevion at gmail.com> wrote:

> 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/3c42ce33/attachment.html>