Gr_firdes.cc/firdes.cc - window functions - flawed code found and fixed

(If you just want to cut to the chase, the diff against 3.6.5.1 is
attached)

How i got here: Contemplating some filters using gnuradio-companion
with a
simple flowgraph (simple enough to describe in words alone). Noticed
the
frequency response with a Rectangular filter was exactly the same as
with a
Hamming filter and also the response with a Kaiser filter (while varying
Beta) seemed quite wrong.

The flowgraph: noise source → throttle → filter → FFT
really basic. used the “convenience” blocks which are wrappers for
firdes.

After quite a while of scratching my head at the odd results observed,
then
checking (and double-checking) Oppenheim[1999] and others, I wrote a
little
python to have a direct look at the window function coefficients:

#!/usr/bin/env python
from gnuradio import gr, audio
from math import pi

sample_rate = 192000
ntaps = 16

#channel_coeffs =
gr.firdes.low_pass(1.0,sample_rate,50e3,4e3,gr.firdes.WIN_HAMMING,beta)

#print channel_coeffs

#channel_coeffs =
gr.firdes.low_pass(1.0,10,1,1,gr.firdes.WIN_HAMMING,beta)

#print channel_coeffs

print “\n\nRectangular window function for {} samples\n”.format(ntaps)

win_coeffs = gr.firdes.window(gr.firdes.WIN_RECTANGULAR,ntaps,0)

print win_coeffs

print “\n\nHamming window function for {} samples\n”.format(ntaps)

win_coeffs = gr.firdes.window(gr.firdes.WIN_HAMMING,ntaps,0)

print win_coeffs

print “\n\nKaiser window function for {} samples\n”.format(ntaps)

alpha = 1.0
print “Alpha = {}\n”.format(alpha)
win_coeffs = gr.firdes.window(gr.firdes.WIN_KAISER,ntaps,alpha*pi)

print win_coeffs

alpha = 2.5
print “\nAlpha = {}\n”.format(alpha)
win_coeffs = gr.firdes.window(gr.firdes.WIN_KAISER,ntaps,alpha*pi)

print win_coeffs

alpha = 8.0
print “\nAlpha = {}\n”.format(alpha)
win_coeffs = gr.firdes.window(gr.firdes.WIN_KAISER,ntaps,alpha*pi)

print win_coeffs

alpha = 20.0
print “\nAlpha = {}\n”.format(alpha)
win_coeffs = gr.firdes.window(gr.firdes.WIN_KAISER,ntaps,alpha*pi)

print win_coeffs

print “\nDone\n”

… and here’s the essential extract of output from unmodified v3.6.5.1
source:

Rectangular window function for 16 samples

(0.07999999821186066, 0.11976908892393112, 0.23219992220401764,
0.39785218238830566, 0.5880830883979797, 0.7699999809265137,
0.9121478199958801, 0.9899479150772095, 0.9899479150772095,
0.9121478199958801, 0.7699999809265137, 0.5880830883979797,
0.39785218238830566, 0.23219992220401764, 0.11976908892393112,
0.07999999821186066)

Hamming window function for 16 samples

(0.07999999821186066, 0.11976908892393112, 0.23219992220401764,
0.39785218238830566, 0.5880830883979797, 0.7699999809265137,
0.9121478199958801, 0.9899479150772095, 0.9899479150772095,
0.9121478199958801, 0.7699999809265137, 0.5880830883979797,
0.39785218238830566, 0.23219992220401764, 0.11976908892393112,
0.07999999821186066)

Kaiser window function for 16 samples

Alpha = 1.0

(1.0, 0.9949779510498047, 0.9800193309783936, 0.9554436802864075,
0.9217740297317505, 0.8797227740287781, 0.8301725387573242,
0.7741525173187256, 0.7128111124038696, 0.6473857760429382,
0.5791705250740051, 0.5094824433326721, 0.43962839245796204,
0.3708721101284027, 0.3044034540653229, 0.241310253739357)

… the Rectangular coefficients aren’t right. And sure, it’s really
weird
the coefficients are the same as for the Hamming window. But look at
the
Kaiser coefficients! (this was giving me an awful headache and
bothering
me to no end).

With a little help from octave and some quick cut-n-pastes, I was now
contemplating graphs of window functions. The Kaiser window function
didn’t look right at all. It started a 1.0 and tapered toward zero. No
starting near zero and tapering -up- toward 1.0 present. That can’t be
right, can it? Well, no, it can’t. Hmm!

Glossing over the many hours it took from the start of this journey to
its
conclusion, I present the essential extract of output from v3.6.5.1
source
modified by the diff attached to this message:

Rectangular window function for 16 samples

(1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0,
1.0,
1.0)

Hamming window function for 16 samples

(0.07999999821186066, 0.11976908892393112, 0.23219992220401764,
0.39785218238830566, 0.5880830883979797, 0.7699999809265137,
0.9121478199958801, 0.9899479150772095, 0.9899479150772095,
0.9121478199958801, 0.7699999809265137, 0.5880830883979797,
0.39785218238830566, 0.23219992220401764, 0.11976908892393112,
0.07999999821186066)

Kaiser window function for 16 samples

Alpha = 1.0

(0.6473857760429382, 0.7741525173187256, 0.8301725387573242,
0.8797227740287781, 0.9217740297317505, 0.9554436802864075,
0.9800193309783936, 0.9949779510498047, 1.0, 0.9949779510498047,
0.9800193309783936, 0.9554436802864075, 0.9217740297317505,
0.8797227740287781, 0.8301725387573242, 0.7741525173187256)

Alpha = 2.5

(0.2832930386066437, 0.47887080907821655, 0.5863785147666931,
0.6930355429649353, 0.7924286127090454, 0.8780854940414429,
0.9441056251525879, 0.9857622385025024, 1.0, 0.9857622385025024,
0.9441056251525879, 0.8780854940414429, 0.7924286127090454,
0.6930355429649353, 0.5863785147666931, 0.47887080907821655)

Alpha = 8.0

(0.01416657492518425, 0.083808533847332, 0.16599954664707184,
0.29149267077445984, 0.45759791135787964, 0.6461663842201233,
0.8243628740310669, 0.9529938697814941, 1.0, 0.9529938697814941,
0.8243628740310669, 0.6461663842201233, 0.45759791135787964,
0.29149267077445984, 0.16599954664707184, 0.083808533847332)

… there you have it, folks. I’ve heard it said “it’s a jungle in
there”
(referring to the gnuradio codebase), to which I replied, “yeah and
there
are hidden stone walls, too”. I’m glad to have had the opportunity to
remove one of the stone walls.

[second attempt to post due to recent gnuradio.org domain issue]

This is a useful patch for anyone using the 3.6 code. Since we have
basically stopped updating 3.6, this may or may not ever make it back
in there to be fixed up from our side. There’s a chance we’ll need to
release 3.6.5.2 at some point, but we’d prefer not to unless it’s
something critical that can’t be fixed with a nice patch like this.

The bug that’s reported has been fixed in 3.7 and will be part of the
next release. It was actually discovered by our Coverity scan and
reported by Philip B… I had to make some fixes to the QA code
that I think are a lot more sanitary now than how we were doing some
things before (actually calculating results as opposed to storing a
set of results from previous runs).

The other change made is that the filter.hilbert_fc block now allows
us to set which window we want to use instead of forcing the default
to being a rectangular window. I wrote a blog post showing how the
windows affect the Hilbert transform:

Thanks again, Chris, for the detailed report and patch.

Tom