Summary:ASTERISK-05216: [patch] Choppy dial tone on Linksys routers / FPU usage in indications.c
Reporter:inducer (inducer)Labels:
Date Opened:2005-10-02 23:23:19Date Closed:2008-01-15 15:51:17.000-0600
Versions:Frequency of
Environment:Attachments:( 0) asterisk-1.0.7-no-fpu-indications.patch
( 1) asterisk-cvs-no-fpu-indications.patch
Description:Here's a patch that switches tone generation in indications.c over from using
actual floating point code to using a pregenerated sine table. This patch
allows my poor FPU-less Linksys router to successfully play a dialtone with
almost zero CPU usage.

(If you care about the details: Generating the 8000-entry sine table at
startup takes a bit more than a second on that machine, which is a MIPS using
an FPU emulator. Since each sample out of playtones_generator() takes two
sine evaluations and we're sampling at 8000Hz, the Linksys is too slow by
about a factor of two.)

This might even save some cycles on bigger setups. The patch applies cleanly
and works on 1.0.x with x>=7, on 1.2.x it needs to be told where asterisk.h
is, but otherwise seems to work fine.
Comments:By: Russell Bryant (russell) 2005-10-04 15:27:56

Since this isn't necessarily a bug, it won't be included in the 1.0 tree.  Please update the patch to be against CVS HEAD.

Something to consider is that Asterisk uses a define called LOW_MEMORY.  Some people might not want the extra 16k used by this.  However, most people probably already build Asterisk with LOW_MEMORY defined for a Linksys, or other embedded platform, but would obviously benefit from this.  Maybe it could be another define that is on by default.  I don't know ... I just wanted to throw it out there for discussion.

By: Kevin P. Fleming (kpfleming) 2005-10-04 18:45:42

It's a tough call whether LOW_MEMORY should be used for this or not; I think I'd rather see an option in indications.conf to control whether the pre-generation is done or not.

By: Russell Bryant (russell) 2005-10-04 18:55:30

Good call.  What do you think about the default value could be based on whether LOW_MEMORY is defined or not?

By: Kevin P. Fleming (kpfleming) 2005-10-04 19:01:33

Yeah, the default could be 'on' when LOW_MEMORY is defined, but I'd rather not... it would be hard to describe in the sample config file :-)

By: inducer (inducer) 2005-10-04 19:10:36

Updated patch for CVS.

By: Kevin P. Fleming (kpfleming) 2005-10-04 19:15:13

We will still need a disclaimer from you before we can effectively review and merge your work :-)

By: inducer (inducer) 2005-10-04 19:16:00

The way I see it, this only becomes a problem when there's plenty of CPU/FPU horsepower, but really *no* memory. (Remember, this is 16k per instance--not per connection, per thread, or anything else.) Is there anybody out there whose memory is that tight?

As you noted, machines with LOW_MEMORY defined are typically the ones most likely to want the pregeneration, thus using more memory. Hmm.

By: Russell Bryant (russell) 2005-10-04 21:48:44

kpfleming:  I was thinking more along the lines of having this default to 'off' if LOW_MEMORY is defined, but whatever.  :)

Every time the discussion of LOW_MEMORY comes up, someone always says, "but it's only XXX bytes!".  I know, I know, ... but a lot of these optimizations add up to be significant, and that is the whole point of having it.

I just had another thought ... what if the sine table was pre-generated and we just throw it into a header file instead of having it generated at run time?

By: Mark Spencer (markster) 2005-10-13 16:40:28

Have you considered using the method that zaptel uses for calculating the coeffients and operating?

By: Mark Spencer (markster) 2005-10-15 23:12:43

Fixed in CVS head using the zaptel.c method

By: Digium Subversion (svnbot) 2008-01-15 15:51:17.000-0600

Repository: asterisk
Revision: 6796

U   trunk/indications.c

r6796 | markster | 2008-01-15 15:51:16 -0600 (Tue, 15 Jan 2008) | 2 lines

Make indications work more efficiently and without heavy floating point (bug ASTERISK-5216)