[Home]

Summary:ASTERISK-03061: [patch] Fix for voice timestamp prediction in IAX2; prevent ts jumps.
Reporter:stevekstevek (stevekstevek)Labels:
Date Opened:2004-12-21 15:56:03.000-0600Date Closed:2008-01-15 15:19:45.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) iax2-ts.patch
Description:In chan_iax2.c, we do this, on "predicted" voice timestamps going out:
  330
  331 #define MAX_TIMESTAMP_SKEW      640
  332

2850                 if (voice) {
 2851                         /* On a voice frame, use predicted values if appropriate */
 2852                         if (abs(ms - p->nextpred) <= MAX_TIMESTAMP_SKEW) {
 2853                                 if (!p->nextpred) {
 2854                                         p->nextpred = ms; /*f->samples / 8;*/
 2855                                         if (p->nextpred <= p->lastsent)
 2856                                                 p->nextpred = p->lastsent + 3;
 2857                                 }
 2858                                 ms = p->nextpred;
 2859                         } else
 2860                                 p->nextpred = ms;
 2861                 } else {

I've actually changed this (well, the equivalent area) in libiax2, based on conversations from astricon, to do this:

  495         if(voice) {
  496 #ifdef USE_VOICE_TS_PREDICTION
  497                 /* If we haven't most recently sent silence, and we're
  498                  * close in time, use predicted time */
  499                 if(session->notsilenttx && abs(ms - session->nextpred) <= 240) {
  500                     /* Adjust our txcore, keeping voice and non-voice
  501                      * synchronized */
  502                     add_ms(&session->offset, (int)(ms - session->nextpred)/10);
  503
  504                     if(!session->nextpred)
  505                         session->nextpred = f->samples;
  506                     ms = session->nextpred;
  507                 } else {
  508                     /* in this case, just use the actual time, since
  509                      * we're either way off (shouldn't happen), or we're
  510                      * ending a silent period -- and seed the next predicted
  511                      * time.  Also, round ms to the next multiple of
  512                      * frame size (so our silent periods are multiples
  513                      * of frame size too) */
  514                     int diff = ms % (f->samples / 8);
  515                     if(diff)
  516                         ms += f->samples/8 - diff;
  517                     session->nextpred = ms;
  518                 }
  519 #else
  520                 if(ms <= session->lastsent)
  521                         ms = session->lastsent + 3;
  522 #endif
  523                 session->notsilenttx = 1;
  524         } else {


Basically, what this does, is the following:

1) it keeps track of silent periods, based on sending an AST_FRAME_CNG to indicate silence.  This is something we ought to do throughout asterisk, so we can do DTX properly everywhere.  Things still generally work without this optimization, though.

2) When we're not in a silent period, and we are "predicting" timestamps, we adjust our "txcore" (via a low-pass filter), so that we can continually send continuous, clean timestamps, even if the timebase of our audio, and gettimeofday don't match.

This patch implements this for IAX2 in non-trunked mode.  The same thing ought to be done for trunked mode, but I'm not familiar enough with the semantics of that [and I don't use it] to apply this there.

This patch is against CVS, but should probably (after some testing) make it's way to stable as well.




****** ADDITIONAL INFORMATION ******

An easy way to see how the existing method goes wrong is to use app_echo with a lossy line.  The same thing would happen with a (non-natively) bridged call when the source frames experience loss.

As soon as you lose 640ms worth of audio (32 20ms frames or 22 30ms frames), you trigger the "out of sync" code, and then your timestamps shoot forward 640ms, causing a 640ms gap of silence in your outgoing audio.

With this patch, we would instead just have a gradual timestamp skew, which is something that can be dealt with much more easily by jitterbuffers on the receiving end [present asterisk jitterbuffer would effectively use silence insertion, more advanced buffers will interpolate, etc].

Yes, disclaimer is on file.
Comments:By: stevekstevek (stevekstevek) 2004-12-21 16:20:25.000-0600

Also, this patch would have the effect, when you use zap hardware, of "clocking" outgoing IAX2 timestamps exactly based on the zap timer, instead of the local clock.
By: Mark Spencer (markster) 2004-12-21 18:09:21.000-0600

We don't have a way to send CNG yet in Asterisk...  Probably need to though!
By: stevekstevek (stevekstevek) 2004-12-21 18:32:55.000-0600

Heh.  You'll start getting it soon, though.  iaxclient already sends it (although, presently, it has no "data" associated with it).

It doesn't matter in relation to this patch, though. This patch still makes things better, although you can still have timestamp strangeness with short periods of no-voice transmission.  You can probably lower MAX_TIMESTAMP_SKEW a heck of a lot with this patch, though, and avoid that, because the SKEW should never get large with this patch.

It doesn't matter, specifically, because channels start out in "silent" mode (notsilenttx = 0), but as soon as they send one frame, they change to notsilent mode.

Sending CNG needs to (eventually) be done in many places in asterisk, though; I would imagine the most important would be from the dialplan, whenever you exit an application, as it doesn't send anything when you're sitting there waiting for digits, etc.
By: Mark Spencer (markster) 2004-12-21 19:23:58.000-0600

Added to CVS, I hope you know what you're doing! ;-)
By: Russell Bryant (russell) 2004-12-30 14:13:13.000-0600

fixed in 1.0
By: Digium Subversion (svnbot) 2008-01-15 15:17:45.000-0600

Repository: asterisk
Revision: 4523

U   trunk/channels/chan_iax2.c

------------------------------------------------------------------------
r4523 | markster | 2008-01-15 15:17:44 -0600 (Tue, 15 Jan 2008) | 2 lines

Merge Steve's timestamp patch (bug ASTERISK-3061)

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

http://svn.digium.com/view/asterisk?view=rev&revision=4523
By: Digium Subversion (svnbot) 2008-01-15 15:18:58.000-0600

Repository: asterisk
Revision: 4605

U   branches/v1-0/channels/chan_iax2.c

------------------------------------------------------------------------
r4605 | russell | 2008-01-15 15:18:58 -0600 (Tue, 15 Jan 2008) | 2 lines

prevent timestamp jumps (bug ASTERISK-3061)

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

http://svn.digium.com/view/asterisk?view=rev&revision=4605
By: Digium Subversion (svnbot) 2008-01-15 15:19:45.000-0600

Repository: asterisk
Revision: 4658

U   branches/v1-0/apps/app_voicemail.c
U   branches/v1-0/channels/chan_iax2.c

------------------------------------------------------------------------
r4658 | russell | 2008-01-15 15:19:44 -0600 (Tue, 15 Jan 2008) | 2 lines

I got a crash in iax that I think is related to this timestamp patch, so I am going to back it out until we get it figured out. (bug ASTERISK-3061)

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

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