freeswitch/spandsp

super_tone detector not working after commit d0dfd573

Closed this issue · 2 comments

I've found that after commit d0dfd57 tone detector stopped working for me.
I've pinpointed the issue to be with the following piece of code in super_tone_rx.c/super_tone_chunk():

            for (j = 2;  j < s->desc->monitored_frequencies;  j++)
            {
                if (res[j] >= res[k1])
                {
                    k2 = k1;
                    k1 = j;
                }
                else if (res[j] >= res[k2])
                {
                    k1 =     // <----this line
                    k2 = j;
                }
                /*endif*/
            }}

If remove the problem line, the issue goes away. As per my understanding, k1 should point to a tone with more energy, k2 to the one with less energy. So the line, in question, is logically incorrect.

Also there's uncertanity about DETECTION_THRESHOLD declared in super_tone_rx.c for float point:

// note the extra SUPER_TONE_BINS multiplier
#define DETECTION_THRESHOLD         2104205.6f      /* -42dBm0 [((SUPER_TONE_BINS*SUPER_TONE_BINS*32768.0/1.4142)*10^((-42 - DBM0_MAX_SINE_POWER)/20.0))^2] */

while ademco_contactid.c declares it as:

#define DETECTION_THRESHOLD         49728296.6f     /* -42dBm0 [((GOERTZEL_SAMPLES_PER_BLOCK*32768.0/1.4142)*10^((-42 - DBM0_MAX_SINE_POWER)/20.0))^2] */

For me, the formula from ademco_contactid.c works fine for tone detection, while one from super_tone_rx.c is not working.
@coppice-git , what's the right formula?

That k1 = line is a bug, now fixed. Several modules in spandsp use Goertzel filters. They all use common routines to implement the filters, but used different code to process the results. I've changed things to increase the consistency, and make the code more clearly show the way things like detection thresholds work. I might make more changes, so a total energy calculation and a Goertzel filter have a similarly scaled output. That only adds one multiply per filter block. 25 years ago when that code started I was trying to squeeze out every single operation I could. :) Going for maximum clarity might be better now.

Fixed in 5394b2c