[Home]

Summary:ASTERISK-04586: [patch] ast_translate doesn't advance outgoing timestamps during silent periods
Reporter:James Cowling (jcowling)Labels:
Date Opened:2005-07-14 03:29:34Date Closed:2008-01-15 15:42:31.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) translate_ts_fix_220705.patch
Description:If there are frames continually coming into the translator, but none coming out (such as when DTX is active), predicted outgoing timestamps will not be incremented.  When voice frames are again output from the translator, they are incorrectly given timestamps consecutive with those before the silence period, as if there was no time elapsed.

This can be seen as debugging output from chan_iax2 with Speex DTX enabled:

TS: Audio - 15110ms
TS: Audio - 15130ms
TS: Audio - 15170ms
<long silent period, prefaced by a CNG frame>
TS: Audio - 15190ms
TS: Audio - 15230ms
TS: Audio - 15250ms
TS: Audio - 15290ms

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

This is the cause of garbled audio I mistakenly blamed on the jitter buffer in 0004608 (sorry Steve!).  It is completely independent of jitter buffering and trunking, with the audible symptoms merely exacerbated by this combination.

The source of the problem is in ast_translate (translator.c).  In the code below path->nextout is incremented only on the basis of outgoing voice frames.  When a voice frame comes along after a period of silence, it is simply given the path->nextout timestamp, even if there is a sizeable delay between talk-spurts.

/* If we get nothing out, return NULL */
if (!out)
return NULL; /* this is where our silent frames get thrown */
/* If there is a next state, feed it in there.  If not,
  return this frame  */
if (p->next)
p->next->step->framein(p->next->state, out);
else {
if (delivery.tv_sec || delivery.tv_usec) {
/* Use next predicted outgoing timestamp */
out->delivery.tv_sec = path->nextout.tv_sec;
out->delivery.tv_usec = path->nextout.tv_usec;

/* Predict next outgoing timestamp from samples in this
  frame. */
path->nextout.tv_sec += (out->samples / 8000);
path->nextout.tv_usec += ((out->samples % 8000) * 125);
if (path->nextout.tv_usec >= 1000000) {
path->nextout.tv_sec++;
path->nextout.tv_usec -= 1000000;
}
} else {
out->delivery.tv_sec = 0;
out->delivery.tv_usec = 0;
}
return out;
}

The "The time has changed between what we expected and this most recent time on the new packet. Adjust our output time appropriately" block in ast_translate code only operates when there is a pause in *input*, not in output.
Comments:By: James Cowling (jcowling) 2005-07-14 03:35:18

I'm looking through it myself but hopefully someone can suggest a fix that isn't too much of a hack.

A possible alternative would be to return a CNG frame for every dtx frame (not just the start of silence), and then intercept it somewhere in channel.c to prevent it being sent over the wire.  I'm not too keen on this solution tho - it would make it very hard to distinguish between genuine comfort-noise frames (that should be transmitted) and pure DTX (that shouldn't).
By: James Cowling (jcowling) 2005-07-15 00:08:36

I should probably rephrase - the "timestamp" itself is not being set incorrectly by ast_translate, the "delivery" time is.  As calc_timestamp in chan_iax2 directly uses any specified delivery time to set the timestamp (regardless of silence status), this is essentially the same thing.

see calc_timestamp() in chan_iax2.c:

/* If we have a time that the frame arrived, always use it to make our timestamp */
if (delivery && (delivery->tv_sec || delivery->tv_usec)) {
ms = (delivery->tv_sec - p->offset.tv_sec) * 1000 +
(1000000 + delivery->tv_usec - p->offset.tv_usec) / 1000 - 1000;
if (option_debug > 2)
ast_log(LOG_DEBUG, "calc_timestamp: call %d/%d: Timestamp slaved to delivery time\n", p->callno, iaxs[p->callno]->peercallno);
} else {
By: James Cowling (jcowling) 2005-07-22 05:18:11

This little patch should sort things out - wasn't too much of a hack after all :)
By: Kevin P. Fleming (kpfleming) 2005-07-25 15:34:15

Committed to CVS HEAD, with formatting fixes (please review the coding guidelines carefully <G>). Thanks!
By: Digium Subversion (svnbot) 2008-01-15 15:42:31.000-0600

Repository: asterisk
Revision: 6203

U   trunk/translate.c

------------------------------------------------------------------------
r6203 | kpfleming | 2008-01-15 15:42:31 -0600 (Tue, 15 Jan 2008) | 2 lines

ensure translators don't generate old timestamps when silent periods end (bug ASTERISK-4586 with formatting fixes)

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

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