[Home]

Summary:ASTERISK-12135: Queue holdtime smoothing filter is not boxcar (FIR), but exponential average (IIR).
Reporter:David Woolley (davidw)Labels:
Date Opened:2008-06-03 13:01:08Date Closed:2008-06-17 10:51:22
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Applications/app_queue
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:The comment for recalc_holdtime says that it implements a recursive boxcar algorithm.  Even with the "recursive" qualifier, boxcar averaging refers to simple rolling average filters, which are finite impulse response ones.  The filter that recalc_holdtime actually implements is an exponential average, which is an infinite impulse response filter.

Incidentally, "recursive boxcar" has only three non-Asterisk Google hits, all of which are clearly finite impulse response.  "boxcar function" clearly refers to something with finite duration.

The recursive refers to an optimisation in which you add the next input value into a running total and then subtract it out n samples later, rather than summing the last n samples each time.  The boxcar, effectively, refers to the fact that all the historic samples that are actually used have the same weight.

The recursive boxcar formulation is an FIR formulation, but the resulting filter can be reduced to IIR form, which is not true of the filter used in Asterisk.
Comments:By: Mark Michelson (mmichelson) 2008-06-13 13:35:52

I understand that the comment and code do not match, but I guess there needs to be a distinction made regarding this report. Does the comment need to be changed to conform to what's in the code or does the code need to be changed to conform to what the comment says? Unfortunately, I don't really grasp the gain from changing the code to be a FIR function so my initial reaction upon seeing such a report is to change the comment. I didn't write the holdtime calculation function, but looking at it, I have to believe it was written the way it was to be lightweight.

By: David Woolley (davidw) 2008-06-17 07:48:51

I marked it as text, because I was suggesting the comment be changed.  Basically I was hoping it was a bit more sophisticated than it is, and didn't recognize the "boxcar" term,  but when I looked it up I found that it was being misused.

By: Mark Michelson (mmichelson) 2008-06-17 10:41:13

Okay, well that certainly makes things easy. :) I'll get the comment changed to be more accurate.

By: Digium Subversion (svnbot) 2008-06-17 10:49:57

Repository: asterisk
Revision: 123274

U   branches/1.4/apps/app_queue.c

------------------------------------------------------------------------
r123274 | mmichelson | 2008-06-17 10:49:55 -0500 (Tue, 17 Jun 2008) | 12 lines

davidw pointed out that the holdtime calculation used by
app_queue does not use "boxcar" filtering as the comments
say. The term "boxcar" means that the number of samples used
to calculate stays constant, with new samples replacing the
oldest ones. The queue holdtime calculation uses all holdtime
samples collected since the queue was loaded, so the comment
has been changed to be accurate.

(closes issue ASTERISK-12135)
Reported by: davidw

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=123274

By: Digium Subversion (svnbot) 2008-06-17 10:50:44

Repository: asterisk
Revision: 123275

_U  trunk/
U   trunk/apps/app_queue.c

------------------------------------------------------------------------
r123275 | mmichelson | 2008-06-17 10:50:43 -0500 (Tue, 17 Jun 2008) | 20 lines

Merged revisions 123274 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r123274 | mmichelson | 2008-06-17 10:56:55 -0500 (Tue, 17 Jun 2008) | 12 lines

davidw pointed out that the holdtime calculation used by
app_queue does not use "boxcar" filtering as the comments
say. The term "boxcar" means that the number of samples used
to calculate stays constant, with new samples replacing the
oldest ones. The queue holdtime calculation uses all holdtime
samples collected since the queue was loaded, so the comment
has been changed to be accurate.

(closes issue ASTERISK-12135)
Reported by: davidw

........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=123275

By: Digium Subversion (svnbot) 2008-06-17 10:51:22

Repository: asterisk
Revision: 123276

_U  branches/1.6.0/
U   branches/1.6.0/apps/app_queue.c

------------------------------------------------------------------------
r123276 | mmichelson | 2008-06-17 10:51:21 -0500 (Tue, 17 Jun 2008) | 28 lines

Merged revisions 123275 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r123275 | mmichelson | 2008-06-17 10:57:43 -0500 (Tue, 17 Jun 2008) | 20 lines

Merged revisions 123274 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r123274 | mmichelson | 2008-06-17 10:56:55 -0500 (Tue, 17 Jun 2008) | 12 lines

davidw pointed out that the holdtime calculation used by
app_queue does not use "boxcar" filtering as the comments
say. The term "boxcar" means that the number of samples used
to calculate stays constant, with new samples replacing the
oldest ones. The queue holdtime calculation uses all holdtime
samples collected since the queue was loaded, so the comment
has been changed to be accurate.

(closes issue ASTERISK-12135)
Reported by: davidw

........

................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=123276