[Home]

Summary:ASTERISK-02491: [patch] new jitter buffer
Reporter:Steve Davies . (stevedavies)Labels:
Date Opened:2004-09-28 09:43:51Date Closed:2008-01-15 15:27:59.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) jitterbsd.txt
( 1) jitterbuffer.txt
( 2) jitterbuf-jumbopatch.txt
( 3) jitterbuf-jumbopatch2.txt
( 4) plcoff.patch.txt
( 5) readme.new-jitterbuffer.txt
( 6) readme.new-jitterbuffer.txt
Description:Steve Kann and I have started discussing and documenting the design of a new jitter buffer.

I will start implementation as soon as there is reasonable agreement on the design.

For the moment, we are using the voip-info Wiki.  Please see http://www.voip-info.org/tiki-index.php?page=Asterisk+new+jitterbuffer

Please put your comments and contributions on that page, without removing existing text, please.

Regards,
Steve
Comments:By: twisted (twisted) 2004-10-27 15:59:47

Unfortunately, I'm going to close this since there is NOTHING going on here.  Maybe there is on the wiki, but this empty bug serves no purpose.

By: Clod Patry (junky) 2005-01-06 13:22:47.000-0600

stevekstevek wants to make a comment.

By: stevekstevek (stevekstevek) 2005-01-06 13:49:15.000-0600

OK, The jitterbuffer is now implemented, and working pretty well in iaxclient, and it is (past) time to start working on integrating this into asterisk.

The actual code for this can be found in iaxclient cvs:

cvs -d:pserver:anonymous@cvs.sourceforge.net:/cvsroot/iaxclient login
cvs -z3 -d:pserver:anonymous@cvs.sourceforge.net:/cvsroot/iaxclient co iaxclient
CVSweb: http://cvs.sourceforge.net/viewcvs.py/iaxclient/iaxclient/lib/

In iaxclient, the interesting parts that are applicable to asterisk are:

lib/jitterbuf.[ch]:  The actual jitterbuffer.  This code will be shared between iaxclient/libiax2 and asterisk, so we want to keep asterisk-isms out of this file, and keep it behind a strict API.

lib/libiax2/src/iax.c:  The integration of the jitterbuffer with libiax2.  (Eventually, I'll post these changes here, and probably move jitterbuf.[ch] into libiax2.

lib/codec_XXX.c: The "translators" for iaxclient:  These show the interpolation behavior.

Here's the basic parameters of what needs to be done:

1) Translators need to understand a way to know they need to interpolate.  In iaxclient, the way I've done this, is to send a voice frame with datalen==0.  This tells the translator to generate an interpolation frame, of normal size (i.e. for most codecs, 20ms, for iLBC, 30ms).

2) The translators need to actually implement interpolation :)  It's pretty straightforward for iLBC and speex to do this.  For GSM, uLaw, aLaw, slinear, we'll use a generic interpolation algorithm based on G711 appendix 1.  (Steve Underwood is looking at this).  Until Steve has something, we can do what I've done in iaxclient, which is to repeat the previous frame, with some attenuation after the first repeat.

3) We need to actually queue and dequeue frames into/through the jitterbuffer. This, of course, is the more difficult part to integrate.  First, the "sub-parameters" of how I think this ought to work:

a) The jitterbuffer facility should be available whenever asterisk is receiving any kind of VoIP stream.  This means IAX2, SIP, etc etc.

b) The jitterbuffer should be _enabled_ whenever asterisk is "rendering" the stream.  This would include talking to most applications, and/or bridging to a non-VoIP stream.

c) The jitterbuffer should be bypassed only when asterisk is bridging together two VoIP streams.

This may or may not want to be modified on a per-channel, or per-stream basis;  for each end of a conversation, you can assign two booleans, Have_jitter, and Want_jitter.  Have_jitter means that the input from this channel will have jitter (Basically, it's a VoIP stream), and Want_jitter will describe whether this channel or app can handle jitter itself.  So, you might have a matrix like this:

IAX2 and RTP (general case):  Have, Want
IAX2 and RTP (specific peers/users) (may override these)

Zap (general case):  !Have, !Want
Zap (specific peers/users) (may override these)

Applications: !Have, !Want
Specific apps (specific peers/users) (may override these)
Example:  app_echo: Want, (have, only if they get jitter in).
 - app_echo should probably mimic bridging to oneself, in that it should just reflect back the same timestamps it gets in



So, given that, there's two general approaches one can take:

I) Integrate directly into the channels that

By: stevekstevek (stevekstevek) 2005-01-06 14:16:10.000-0600

Crap! sorry about that, apparently, my browser moved focus..  (now, I dunno whether to just start again, or try to intersperse my comments..).

Anyway, to finish my above:

app_echo should probably just reflect back what it receives, timestamps and all.  So, on a VoIP channel, it should send back the same timestamps it receives, etc, and should _not_ perform local dejittering.

So, the two general approaches are:

a) Integrate this into chan_iax2.c, rtp.c, and any other VoIP channels, separately.  This could follow, more or less, the pattern I used in libiax2/iax.c

b) Integrate this into channel.c, and use some logic to enable/disable this on a per-channel basis.   I think it would basically be used whenever a readq is used, but I'm not sure..

===============================

For hermie's benefit (;)), here's the "feature" list/basic arch of the jitterbuffer:

First, the API:

/* new jitterbuf */
jitterbuf *             jb_new();

/* destroy jitterbuf */
void                    jb_destroy(jitterbuf *jb);

/* reset jitterbuf */
/* NOTE:  The jitterbuffer should be empty before you call this, otherwise
* you will leak queued frames, and some internal structures */
int                     jb_reset(jitterbuf *jb);

/* queue a frame data=frame data, timings (in ms): ms=length of frame (for voice), ts=ts (sender's time)
* now=now (in receiver's time)*/
int                     jb_put(jitterbuf *jb, void *data, int type, long ms, long ts, long now);

/* get a frame for time now (receiver's time)  return value is one of
* JB_OK:  You've got frame!
* JB_DROP: Here's an audio frame you should just drop.  Ask me again for this time..
* JB_NOFRAME: There's no frame scheduled for this time.
* JB_INTERP: Please interpolate an audio frame for this time (either we need to grow, or there was a lost frame
* JB_EMPTY: The jb is empty.
*/
int                     jb_get(jitterbuf *jb, jb_frame *frame, long now);

/* when is the next frame due out, in receiver's time (0=EMPTY)
* This value may change as frames are added (esp non-audio frames) */
long                    jb_next(jitterbuf *jb);

/* get jitterbuf info: only "statistics" may be valid */
int                     jb_getinfo(jitterbuf *jb, jb_info *stats);

/* set jitterbuf info: only "settings" may be honored */
int                     jb_setinfo(jitterbuf *jb, jb_info *settings);

***NOTE:  There's no settings to set at the moment, but I imagine that some things, like setting the lookback window for jitter history, drop pct, etc, would be set here.


So, as you receive frames, you put them into the jitterbuffer.  you call jb_get, and get frames from the jitterbuffer as needed.  You find out when that's needed, by asking jb_next().  jb_next() should be called whenever you put, or get, a frame from the jitterbuffer.

The jitterbuffer always gives you back all the frames it receives, although, if it's a voice frame, it may sometimes tell that you ought to drop the frame.  It also may have you call jb_get, and _not_ return you a frame, but instead tell you JB_INTERP, meaning, to interpolate a frame.

It does this whenever there's a missing voice frame, or if it needs to grow.

Unlike the present iax2 jitterbuffer, this jitterbuffer actually manages the queue:  This means that it makes adjustments as frames are rendered, as opposed to as they are received, which means, for example, that it can spread growth around smoothly, instead of making big jumps.

In using this jitterbuffer, and proper interpolation in the translators, asterisk gets:

a) Much better audio quality in any condition where there is a lot of jitter or loss.  You always get a smooth, continuous audio output stream.  For example, you can hardly notice 5% packet loss (with good interpolation), and you generally don't notice buffer growth and shrinking.

b) Instrumentation:  The jitterbuffer keeps track of the following:
       /* statistics */
       long frames_in;         /* number of frames input to the jitterbuffer.*/
       long frames_out;        /* number of frames output from the jitterbuffer.*/
       long frames_late;       /* number of frames which were too late, and dropped.*/
       long frames_lost;       /* number of missing frames.*/
       long frames_dropped;    /* number of frames dropped (shrinkage) */
       long frames_ooo;        /* number of frames received out-of-order */
       long frames_cur;        /* number of frames presently in jb, awaiting delivery.*/
       long jitter;            /* jitter measured within current history interval*/
       long min;               /* minimum lateness within current history interval */
       long current;           /* the present jitterbuffer adjustment */
       long target;            /* the target jitterbuffer adjustment */
       long last_voice_ts;     /* the last ts that was read from the jb - in receiver's time */
       long last_voice_ms;     /* the duration of the last voice frame */
       long silence;           /* we are presently playing out silence */
       long last_adjustment;   /* the time of the last adjustment */

Some of this information should be reported back to the sender, in receiver reports.  IAX2 Receiver reports are in bug 3236, and include the following:

+#define IAX_IE_RR_JITTER                       64              /* Received jitter (as in RFC1889) u32 */
+#define IAX_IE_RR_LOSS                         65              /* Received loss (high byte loss pct, low 24 bits loss count, as in rfc1889 */
+#define IAX_IE_RR_PKTS                         66              /* Received frames (total frames received) u32 */
+#define IAX_IE_RR_DELAY                                67              /* Max playout delay for received frames (in ms) u16 */
+#define IAX_IE_RR_DROPPED                      68              /* Dropped frames (presumably by jitterbuf) u32 */
+#define IAX_IE_RR_OOO                          69              /* Frames received Out of Order u32 */

Also, with this new buffer in place, we could also implement proper RTCP RR packets to send out via rtp.c.

Given two sides of a conversation that support this, you can, on either side, get the full picture of the quality of the network.  This is an example of a call I made to some #asterisk user, from iaxclient:

[Only local stats are here, because his * box doesn't send me RRs].

RTT     Ljit    Llos%   LlosC   Lpkts   Ldel    Ldrop   Looo
60      0       0       0       2       0       0       0
48      133     1       8       438     173     0       0
157     133     1       12      942     173     0       0
49      229     1       20      1430    297     0       0
204     229     1       28      1923    297     0       1
47      455     1       38      2415    517     0       1
49      455     1       43      2911    517     0       2
49      220     1       48      3404    400     3       2
155     229     1       56      3895    300     6       2
45      231     1       64      4390    302     6       2
61      255     1       73      4882    302     6       2
287     319     1       77      5380    362     6       2
342     317     1       86      5872    381     6       13
59      319     1       94      6365    383     6       14
55      321     1       103     6858    385     6       14
59      223     1       108     7351    286     8       14
51      114     1       110     7849    186     12      15
287     396     1       123     8337    446     12      15
49      398     1       132     8829    448     12      15
372     412     1       141     9323    468     12      17
45      412     1       146     9819    468     12      17
66      414     1       152     10314   470     12      18
49      162     1       156     10810   372     16      18
178     411     1       160     11308   452     19      19
128     411     1       167     11799   452     19      19

Note, that he has 1% packet loss, and RTT and jitter are changing a lot.  He sounded perfect to me, but he's been on #asterisk all morning saying that when he calls people, they tell him he sounds lousy..

So, hermie also wanted a patch for this, but that's going to take a bit of time, and I am off on vacation next week..  But, I _do_ really want feedback from people who are going to have to live with this..  I started looking at channel.c, and it's, err, has a lot of code paths that need to be maintained..

By: Kevin P. Fleming (kpfleming) 2005-01-06 19:10:45.000-0600

I think this is a great idea, but I'm concerned about how much effort it will take to integrate into Asterisk and whether the time that consumes will cause "drift" between CVS HEAD and the version you are integrating with... This will touch a _lot_ of parts of Asterisk and I'd hate to see them either have to be "on hold" while you do your work or to have to you spend 50% more time to track the quickly-moving CVS target.

With that said, Mark and others have been talking about adding some sort of "shim" layer that would allow modules to insert themselves into the media path of any channel; that might be the logical place for this to go. When a SIP call comes in, the RTP layer would "top" of the chain, but then it would feed the jitterbuffer, which would then feed a codec.

Actually, if I understand this correctly, the jitterbuffer would _only_ be used when the incoming media is being decoded into signed linear for later manipulation; if that's the case, can't the jitterbuffer just work at the signed linear level, after the packets have been decoded?

By: stevekstevek (stevekstevek) 2005-01-06 21:25:41.000-0600

Responding to KPFleming:

Re: Time it will take:  It really didn't take a lot of time to integrate this int libiax2/iaxclient, but then, iaxclient is less complicated than asterisk:  There's just one channel type, and all channels are either "bridged to nowhere" (i.e. on hold), or "bridged to the audio layer".   Assuming we just put this in chan_iax2, the integration is pretty similar;  We'd just need changes to chan_iax2 that resemble those in iax.c, and then relatively simple changes in the translators.

Take a look at libiax2/src/iax.c here: http://cvs.sourceforge.net/viewcvs.py/iaxclient/iaxclient/lib/
and the changes are pretty clearly marked with #ifdef NEWJB.

Also, the necessary changes can be done in stages:

1) Decide how we'll represent an "interpolation frame", and get the codecs to either (a) implement it, or (b) ignore it,

2) Get the jitterbuffer in, and make it configurable, but leave it off by default (and, perhaps, even leave it's enabling config item out of the sample configs).

3) Test, test, test!  Fix bugs, enhance, etc.

4) Expose, enable, rejoice!

5)

6) Profit!


Re "With that said, Mark and others have been talking about adding some sort of "shim" layer that would allow modules to insert themselves into the media path of any channel; that might be the logical place for this to go. When a SIP call comes in, the RTP layer would "top" of the chain, but then it would feed the jitterbuffer, which would then feed a codec." -- I think this is pretty much what the readq is doing now, in channel.c:  the protocol layer puts frames in the queue, and then they are read either by the bridge code, or an application, through a translator if the formats don't match.  So, this if we put it here, I think it has that effect.

Re: "Actually, if I understand this correctly, the jitterbuffer would _only_ be used when the incoming media is being decoded into signed linear for later manipulation; if that's the case, can't the jitterbuffer just work at the signed linear level, after the packets have been decoded?" -- the jitterbuffer itself works at the frame level:  It buffers frames, and provides them back to you in-order, with a minimum of lost frames, and in your own "time" perspective(*).  It also provides "hints" for a translator or renderer to use to "fill in the gaps" when one would expect an audio frame, but one isn't available, either because it is late, lost, or we need to increase the buffering delay.  

Regardless of what you're doing with the frames, though, as long as you're not bridging to another VoIP channel, I think you want the jitterbuffer in place, because at a minimum it will put frames in order for you.  The method I described earlier (i.e. making audio frames with zero length to indicate interpolation), _should_ (I think) allow applications to all handle them in a reasonable way even if that means ignoring them entirely.

The following gets a bit into details though, and this might be a diversion better left for later:  Even if you're not translating, it is still providing the frames to you in the right order, with a minimum of loss.    In some cases, you may (unavoidably) lose the "hints" that you're given about rendering.  How this is best handled is probably dependent on what you're doing, though.  

Example 1:  you have a GSM call coming in, and you're trying to write that file out as raw GSM-encoded audio.  (i.e. app_record, or app_voicemail) In this case:

a) You might _not_ want to drop audio frames marked for dropping (but only in the case where it's being dropped to shrink the buffer -- actually the jb doesn't tell you why it wants the frame dropped yet, but it could :)).
b) You can't properly handle an "INTERP" frame request -- i.e. to fill in time from a lost packet -- because the file format doesn't have a way to store this information.  So, you could either put nothing in for that frame request, or you could duplicate a previous frame.

If this same case wall all uLaw, though, you'd do PLC at the uLaw level, and put the interpolated frame into the saved ulaw file..  

Case 2: Channels of any type going into meetme, chan_oss, or bridged to zap, capi, etc..

In this case, you end up "rendering" the audio to slinear for meetme, and do all the interpolation, dropping, etc.

Case 3: Bridging between two VoIP technologies:

In this case, you'd bypass buffering altogether, although it might be useful to use the jitterbuffer just for statistics:  (maybe do this by putting/getting NULL frames the same way, but always forwarding them as soon as you get them?).  

You might, however, want to actually use the jitterbuffer in limited circumstances, in this case.  Say, for example, that you are in an office environment, and have a * box, on the same network as a bunch of cheap SIP phones.  These SIP phones all talk uLaw to asterisk, and that they also have lousy jitterbuffers/PLC themselves.  You get your trunks via VoIP, and there is sometimes reordering, loss, or jitter on the line.  In this case, you might _want_ asterisk to to jitterbuffering and PLC, etc, for what it gets from the IP trunks..  So, this ought to be configurable.

So, to more concisely answer your question :), you probably only can or want to do PLC if you're rendering to slinear (or can afford to go through slinear, as in the last example), but you still want the jitterbuffer to keep things in-order even if you're not.

Also, to your question:  you don't want to do this all after decoding to slinear, because the codecs that natively support interpolation (speex, iLBC, G729 [although, I don't have that code]), will do a better job of it than the generic PLC code we'll have for codecs that don't support PLC (gsm, u/alaw, etc).  Also, by using the native interpolation facilities, the codecs may reproduce speech after the loss better, because they'll know that a packet was missing, and be able to adjust their internal state appropriately.

By: stevekstevek (stevekstevek) 2005-01-19 14:28:26.000-0600

Okay, the first patch, for discussion.

This patch is just PLC implementation for the codecs.

What it does, is for each translator that outputs signed linear, it adds the feature that if they translator receives a frame with datalen == 0, it will do PLC, and output a nominal-size frame's worth of PCM.

Notes:
1) Only ILBC and Speex use native PLC; the others use a generic PLC algorithm.
2) The Generic PLC algorithm in use isn't so great, unless you use define USE_SPANDSP_PLC, and include spandsp's PLC implementation.  It would be much better if we could include that with asterisk or something..
3) The Generic PLC algorithm has the following overhead:
  a) There's a small buffer
  b) When packets are received, the decoded PCM is memcpy'd to the buffer.
  c) Of course, there's a little more computation when interpolating, but, that's not overhead, and it's not much.
4) The SpanDSP interpolator has similar overhead, but a bunch more computation when interpolating, and for a frame or so afterward (I think).
5) Most importantly, this patch (shouldn't) have any affect until the jitterbuffer is in, and actually generates these zero-length voice frames..

Other minor changes in this patch:
1) ADPCM: I think that it could overflow the outbuf (tail) buffer, because it checked to see if the incoming frame would fit in the buffer, but didn't account for what the buffer may have already contained.
2) ADCPM: Fixed misleading comment.

By: stevekstevek (stevekstevek) 2005-01-19 17:28:45.000-0600

The patch I just uploaded, jitterbuf_iax2.patch, is a kinda hacky integration of the jitterbuffer and chan_iax2, just to get an idea of what it looks like.

When combined with codec_plc.patch, it yields an asterisk that:

a) Always has the new jitterbuffer enabled for iax2 channels.
b) supports PLC, and the new jitterbuffer's goodness.

There's still plenty to be done, assuming we continue on this path:
- Once the netstat stuff is decided upon (see bug 3236), this should be integrated with that.
- It might leak in some cases when the channel is destroyed; I do clean up in one case, but this needs to be audited.
- It's totally not configurable at the moment.
- It doesn't handle trunks, I don't think.
- When we go to it, we ought to remove the #ifdefs, and ditch the old jitterbuffer.
- Probably other things I'm forgetting :)

By: stevekstevek (stevekstevek) 2005-01-19 17:33:51.000-0600

Oops, forgot to say how you can test it:

If you checkout iaxclient from sourceforge CVS, in simpleclient/testcall there is an alternate "testcall" program called "testcall-jb".  It can simulate packet loss.  To simulate _outgoing_ packet loss, and make a call into asterisk, you can uncomment the line in jm_sendto which randomly drops packets, instead of sending them to asterisk.

Doing this, and making a call from iaxclient -> asterisk -> PSTN, where there is 15% packet loss between iaxclient and asterisk, with this patch, produces a call where you can hardly notice anything wrong.  As Opp s d to ha ing a call l ke th s.

By: stevekstevek (stevekstevek) 2005-01-19 19:06:49.000-0600

OK, just some notes in case anyone is adventurous enough to try to put this into sip or rtp.

I think, that the best way to do this might be to change all the RTP channels to use the callback mode of rtp.c (on only implement the jb for the callback mode), and then to put the jitterbuffer in rtp.c.

Then you basically put the jb_put stuff in rtpread, and jb_get can be scheduled like I did with iax2, and then call the callback into the channel driver when the packet is ready..

It might even be cleaner, since there's no old, crufty jitterbuffer stuff all over the place to dodge..

By: stevekstevek (stevekstevek) 2005-01-20 12:14:45.000-0600

New codec patch which includes Steve Underwood's Generic PLC algorithm.

Hermie should now be happy it ends with .txt :)

By: stevekstevek (stevekstevek) 2005-01-20 15:56:35.000-0600

I was taking a look at things to see how this will all work for trunked calls, and I see the following problem with trunked call timestamps:

trunked calls are handled in socket_read, when there's a meta trunk thinger, whose only valid value is META_TRUNK.

In this code, this happens:

We get the timestamp for the trunk frame:
  5628       ts = ntohl(mth->ts);

We unpack the sub-frames from the trunk frame:
  5643       while(res >= sizeof(struct ast_iax2_meta_trunk_entry)) {

[... start constructing an iax frame... ]

Setting the timestamp for the iax frame based on the timestamp of the trunk frame...
  5668          fr.ts = fix_peerts(&rxtrunktime, fr.callno, ts);

=======

The problem is, that if there's more than one frame for a call inside the trunk frame, then we set the timestamps on _all_ these frames to be the same..

Example:

Given a trunk "T", and calls a,b,c.  trunkfreq is 60, for example [which is a good choice, if you want to mix 20 and 30ms frames, I'd guess].  Given a,b use 20ms frames, and c uses some "free as in beer" codec with 30ms frames.

A well-formed trunk frame might come in at time t, which might have these subframes stuffed into it:

a1,b1,c1,a2,b2,a3,c2,b3  (i.e. 3 frames for the 20ms guys, 2 frames for the 30ms guys).

the timestamps that get put on these frames would _all_ be t.  When, a much better choice would be:

a1: t
a2: t+20
a3: t+40

b1: t
b2: t+20
b3: t+40

c1: t
c2: t+30

So, this could all be done in the code, by modifying socket_read to cound the number of frames it's seen for each call in the trunk, guessing the number of samples for audio frames in each one, and adding that to the timestamps of subsequent frames it receives for each call..

However, this all breaks down totally if there are missing frames for a call:  i.e. in the previous frame, say sub-frame a2 was lost;  There's be no way to represent that case given the current protocol..

By: stevekstevek (stevekstevek) 2005-01-24 14:27:40.000-0600

OK, I made a patch for trunk timestamps, in bug 3400.  That issue is orthogonal to the new jitterbuffer implementation:  The timestamps really are needed in many cases, including when any type of jitterbuffer is in use..  (old or new).

So, does anyone have any comments on this design and implementation?

By: rmaple (rmaple) 2005-01-25 17:11:39.000-0600

Just updated our server to CVS-HEAD-01/25/05-16:11:17 and, while I was at it, I merged this patch in to see if it helps (we get a LOT of jitter and sound quality suffers).

It sounds GREAT so far after my very limited testing, kudos to Steve & Co. for all the good work.

By: zoa (zoa) 2005-01-26 08:43:23.000-0600

i just tried all 4 patches, my servers seems to choke on it somehow.
(lotsa ast_channel_walk deadlock avoided errors._

Will try again

By: stevekstevek (stevekstevek) 2005-01-26 12:13:20.000-0600

Updated patches.

They need to be applied in this order:
codec_plc.patch3.txt (28,100 bytes) 01-26-05 12:10
jitterbuf_iax2.patch2.txt (28,628 bytes) 01-26-05 12:10

updated to current -head, and also the following changes:
1) Did away with jblock, just using the iaxsl[callno] lock instead;  The second lock was unnecessary, and caused deadlock..

2) Parameterized output functions.  iax2 debug also now turns on/off jitterbuf debugging  (this can be changed easily enough later).

3) fixed a C++/c99ism in a declaration.

By: stevekstevek (stevekstevek) 2005-01-26 13:47:01.000-0600

A little patch:
iax2_test_losspct.patch.txt (3,476 bytes) 01-26-05 13:38

This little patch adds a new CLI "iax2 test losspct <percent>", which simulates an adjustable randomly-distributed packet loss.

This patch should more-or-less apply to either a stock chan_iax2, or to one with the jitterbuffer (actually, I realize now there's a hunk that will fail against stock, my mistake).

If the percentage is non-zero, we just short-cut out of socket_read <percent> percent of the time.  

If you have the new JB and PLC in place, you shouldn't notice this too much, for low percentages (i.e. up to 10% or so).  If you don't, it will make your calls sound like crap pretty quickly.

A good test of this is to apply this patch to a box (A) which has either chan_zap or chan_oss channels.  Make a call from some other IAX endpoint (B) to this box, bridged to the zap/oss channel.

So, in the direction this matters, you have:

(some channel type on box B) <IAX2> [loss happens here, effectively] <Box A, with JB and PLC> [OSS or Zap Channel].

Speak into the channel on box B, and listen through A.  try different loss percentages.

Presently, if, on box A, you do this, and bridge to another VoIP channel, things might get a bit messed up;

By: stevekstevek (stevekstevek) 2005-01-26 17:59:36.000-0600

jitterbuf_iax2.patch3.txt (29,691 bytes) 01-26-05 17:56

This patch works a bit better when calls are bridged, or in the (weird) case of using app_echo..

Interp frames get the correct #samples, and they don't ever get sent from chan_iax2.  (this might need to be adjusted on other channel types too).  

Sending them is kinda useless, and it crashes libiax2-based clients, at least, because they allocate new "events" on the heap, and only set datalen if it's != 0, so they ended up reading random crap from memory and trying to decode it.. sometimes..  So that was a fun adventure..

By: capouch (capouch) 2005-02-08 21:56:00.000-0600

I just patched against CVS-HEAD.

The PLC stuff causes Asterisk to crash when I try to use the iLBC codec.

I'm suspecting it has to do with the 30ms sample size used by that codec?

It's stopping right after these lines in chan_iax2.c:
case JB_INTERP:
    // if(next + 20 != jb_next(pvt->jb)) fprintf(stderr, "NEXT %ld is
not %ld+20!\n", jb_next(pvt->jb), next);

I "uncommented" (although I don't think C++ comments are legal) and it printed:

NEXT 177 is not 147 + 20

Then it dies.
I'm betting the different sample sizes aren't properly being accounted for.

By: Andrew Kohlsmith (akohlsmith) 2005-02-09 02:29:11.000-0600

I've attached a .tar.gz with five packet dumps showing something odd.

newjitterbuffer-no3400patch-peeris2532+3400-callcutsoff-and-outstanding-pkts-climbsfast.dump
newjitterbuffer-no3400patch-peerisjustHEAD-callcutsoff-and-outstanding-pkts-climbsfast.dump
newjitterbuffer-no3400patch-peerisjustHEAD.dump
oldjitterbuffer-no3400patch-peeris2532+3400.dump
oldjitterbuffer-no3400patch-peerisjustHEAD.dump

Pretty self explanatory as to which dump is what.  With the new jitter buffer I get steady call quality worsening and iax2 show stats shows that there are more and more packets outstanding.  With the 'callcutsoff' dumps I actually lose received audio and the outstanding packets climbs rapidly, whereas the ones that don't drop off have the outstanding counter climb by about 1 or 2 every few seconds.  

What is interesting is that it definitely seems to be on my end -- peerisjustHEAD is my * box (wu-ast) which ends up native bridging to switch-1.nufone.net for the call cutting off, and the calls that don't cut off with the new jitter buffer are terminated by the same wu-ast box via TE405P to Bell Canada PRI.

All dumps are with the following iax.conf entries:

jitterbuffer=yes
dropcount=2
maxjitterbuffer=500
maxexcessbuffer=100
minexcessbuffer=50
jittershrinkrate=1

trunkfreq=20
trunktimestamps=no

Let me know what else I can do to help.  As it sits now it's broken and, if I include the bug 3400 patch, breaks all my Zap audio.

By: Andrew Kohlsmith (akohlsmith) 2005-02-09 02:41:27.000-0600

Just another note -- the one system I seem to be having so much trouble with this and the 3400 patches has BOTH a T100P+TDM430P in it.  Not sure if that's perhaps useful information or not...

By: Andrew Kohlsmith (akohlsmith) 2005-02-09 03:06:31.000-0600

Got back home here and this+3400's patches don't seem to be causing any problems whatsoever (TDM420 only) -- Are there any specific tests you'd like me to run to see why the one box behaves so unbelievably badly and this one (and presumably everyone else) doesn't?

they're similar boxes:
P3/700, 256M, 2.4.26, T100P+TDM430P, no IRQs shared, does nothing but run *
T100P is pri_net (dms100 switchtype) to Norstar MICS
it *does* have two NICs... one for LAN and one dedicated VOIP link but again... no shared IRQs.

P3/733, 512M, 2.6.10, TDM420P, IRQ shared with ethernet card (!!), box has 8 IDE drives in it and is an NFS/samba/tftp server so theoretically it should be causing insane problems especially with eth0+TDM420 sharing an IRQ but it's rock-solid reliable.

Problem box runs zttest and gets 100.00% across the board for as long as I'd care to run it.  The box that runs fine gets 99.987793% across the board.

By: Andrew Kohlsmith (akohlsmith) 2005-02-09 22:36:37.000-0600

Just some feedback.

As mentioned in 3400 I have to have trunk=no throughout my iax.conf or I get one-way audio to my PRIs.  I don't know why.

One thing I am noticing is that "iax2 show stats" shows a slow but continually growing number of outstanding frames (almost always ingress).  I don't know if this is some kind of frame leak or what...  I'm sitting with 0 calls in progress and iax2 show stats shows 56 frames (55 ingress, 1 outgress)...  Which seems REALLY odd to me.  The outgress [sic -- it's egress, dammit!] will eventually go away but the ingress never seems to go down to zero unless I restart.

As the call progresses it'll climb a couple, drop a couple, climb a bit, climb a bit higher, drop a little...  but never go back to zero.

Enhanced call quality is certainly noticeable with this patch.  As mentioned by someone else, I can sustain 10% packet loss without significant decay in audio quality, while the person I was IAX'd to (whose box did not have this code) could not make out my voice.

One thing I have noticed is that received voice sounds "pixelly" or "digitized" from time to time -- I don't know if this is merely because the PLC is kicking in and interpolating for me (gsm codec) or if it's kicking in for no good reason -- I certainly didn't expect any kind of packet loss or high jitter during this time of day.

I am going to let every LD call go through switch-3.nufone.net tomorrow (jerjer has graciously set up a box just for this test) to give the code a good run.  Hopefully this frame leak won't kill me.  :-)

By: Mark Spencer (markster) 2005-02-10 01:31:42.000-0600

Is the concept to make the jitter buffer part of the Asterisk core so that it could be used for other purposes?  Who wrote the jitter buffer code and do they have a disclaimer on file, etc?

By: stevekstevek (stevekstevek) 2005-02-10 08:27:51.000-0600

Finally kram arrives!

> Is the concept to make the jitter buffer part of the Asterisk core so that it
> could be used for other purposes?

Yes, and the same jb code is presently used in iaxclient.  I talked in this bug about how it could be applied to rtp input channels as well, and I think zoa has someone experimenting with this.

> Who wrote the jitter buffer code and do they have a disclaimer on file, etc?

I have written everything in the attached patches so far.

By: stevekstevek (stevekstevek) 2005-02-10 17:48:06.000-0600

OK, Sorry for taking a couple of days to respond to people's comments here:

capouch:  It _should_ work fine with iLBC;  Did you also apply the codec_plc patch?  otherwise, you'll probably pass a meaningless pointer to codec_ilbc's decode, and it will get quite confused.

The commented line there is really sanity checking during development (and, most gcc's will honor the c++ comment style; it's actually part of C99, I think too).   Unfortunately, the sanity checking doesn't cover every case; in the case of iLBC for example.

But the jb itself does take the frame sizes into account, by keeping track of the /last/ frame that it saw, and assuming that an INTERP frame will cover up for the same amount.  It would break only in the case where the frame sizes are variable during a call, which isn't a situation that currently exists, AFAIK.

akohlsmith:  

Re: "iax2 show stats"

iax2 show stats prints out counters which are incremented when someone calls iax_frame_new, and decremented when you call iax2_frame_free (which calls iax_frame_free).  So, there's basically two reasons why the numbers would climb:

a) There are frames that _are_ presently in the jitterbuffer.  Normally, each call will have about (jitter/framesize)+2 frames or so in the buffer at any period of time..  So, having 55 outstanding frames isn't really a lot.

b) There are frames that are leaking:  This could happen, of course, either if they leak during normal operation, or when channels are destroyed, but I don't see it, unless there's another channel for a call to become invalid other than calling iax2_destroy where pvt->owner == NULL, which is where I clear out the jb for a dead channel.

I tried looking for a leak which would lead to the situation you describe, but I didn't see it.  However, I _was_ able to reproduce a similar behavior in iaxclient during today's conference call, where I got calculate jitter levels which were 65536-jitter or something, where something screwed up unwrapping timestamps... This could lead to the jitter buffer growing a lot for unexplained reasons, and, of course, lots of outstanding frames..


to debug this best, at some point, it would be good to patch the calculated stats from this JB into chan_iax2..  But that relies on the stuff in /yet another/ bug, bug 3236...

Then, you could get up-to-date stats from the CLI about what the jitterbuffer is seeing..

But, you can turn on debug stuff in the jb itself, by doing iax2 debug and iax2 no debug..  The patch already hooks that into the jb debug stuff, which should give you a bunch of output that would help..



Other Specific points:
The config entries you mentioned:

jitterbuffer=yes
dropcount=2
maxjitterbuffer=500
maxexcessbuffer=100
minexcessbuffer=50
jittershrinkrate=1

Are presently _NOT_ doing anything with this jitterbuffer implementation:  I'm not sure what configuration will end up being necessary for this, so I didn't implement (or honor) any of those.  I'd really like to see this get to the point where it can do the "right thing" automatically.

The ZAP hardware you have, I can't see how it makes a difference..


"One thing I have noticed is that received voice sounds "pixelly" or "digitized" from time to time -- I don't know if this is merely because the PLC is kicking in and interpolating for me (gsm codec) or if it's kicking in for no good reason -- I certainly didn't expect any kind of packet loss or high jitter during this time of day."

It could be.  The debug output can tell you more; It will print a single character every time it dequeues a frame;  The meaning of these characters are:

o: A non-voice frame is coming through
v: A normal voice frame
V: The first voice frame after a silent period.

L: A voice frame was lost, interpolate for it (PLC)
l: A voice frame was too late, and should be discarded.

G: We're interpolating a frame because we want to grow

s: We want to shrink, so discard this frame
S: We want to shrink, _and_ we lost a frame, (calls jb_get again)


So, normally you get "vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvovvvvvvvvvvvvvv" or so.  If you have loss, you'll get some "L"'s in there, etc.  It's a handy visualization tool..

By: stevekstevek (stevekstevek) 2005-02-10 17:55:42.000-0600

Forgot to describe the latest patch:

jitterbuf_iax2.patch4.txt replaces patch3, and has just a couple of small changes:   A bit of a change in the way history and jitter is calculated, which solved a problem seen in iaxclient, and an optimization when doing interpolation (saves malloc/free).

By: capouch (capouch) 2005-02-10 18:05:13.000-0600

Ob iLBC:

I did in fact apply the codec_plc patch, and saw the PLC code in codec_ilbc.c.  I tried the code on two separate platforms, and both behaved identically (I uncommented all your debug messages so I would know where it was failing).

It reliably quit on both platforms at the same place, with the same error message, and the difference between what it was expecting and what it had in each case was the difference between 20 and 30.

I'll repatch everything with the latest stuff you put up here, against fresh CVS, and let you know what happens.

By: capouch (capouch) 2005-02-10 19:15:22.000-0600

Just patched up against the latest CVS.

You may need to check your patch, unless I'm doing something idiotic (totally possible) I couldn't get it to build (even using *previous* patch v3) without moving iax2_test_losspct in chan_iax2.c.

Same problem with iLBC: instantaneous death when I try to use it.  If I remove the PLC code from codec_ilbc.c, things work just fine.

By: Andrew Kohlsmith (akohlsmith) 2005-02-10 20:01:56.000-0600

I can confirm that it does *not* work with ILBC.  I get an immedate segfault.  If I was smart I'd have the corefile around for you, and when I get some time I should generate another.

One other thing I've noticed from time to time were the odd

NEXT 3130 is not 3115+20!

type messages.  I am using gsm and ulaw codecs.

now compiling patch4...  which throws up an error on a freshly-co'd asterisk with codec_plc.patch3, jitterbuf_iax2.patch4 and iax2_test_losspct:

chan_iax2.c:4206: error: `iax2_test_losspct' undeclared here (not in a function)
chan_iax2.c:4206: error: initializer element is not constant
chan_iax2.c:4206: error: (near initialization for `cli_test_losspct.handler')

(which is odd, because it compiled just fine with yesterday's CVS HEAD and jitterbuf_iax2.patch3)

-A.

By: capouch (capouch) 2005-02-10 20:07:15.000-0600

Look at my note right above.

I had to cut and paste that function, which I think got put in the middle of another one by some sort of patch weirdness.

I had a copy of an older CVS tree laying around; I looked at where the function was in that copy, moved the function in the fresh copy to the same place, and it builds just fine.

But it still dies when trying to use iLBC.

By: Andrew Kohlsmith (akohlsmith) 2005-02-10 21:57:26.000-0600

looking at the debug output (more tomorrow for sure)

home* --- main* --- switch-3

all three running this patchset.  main* will bridge the call between home* and switch-3 since home* can't get to switch-3 directly.

home* shows lots of vvvvvvv with the ocassional G at the very start (makes sense)....  main* however shows nothing but LLLLLLLLLLLLLLLLLLL even though the audio's just fine -- I imagine this is just a bug in the debugging code.  :-)

I'm going to hack the patch a little such that the regular iax2 debug isn't shown and I only see the single char stuff.  Very handy.

By: Andrew Kohlsmith (akohlsmith) 2005-02-10 22:37:01.000-0600

Hmm I can't add the file I want so it's at http://www.mixdown.ca/~andrew/dump/newjitter4-pktloss-oddness.tar.gz.  I get application error 401:

APPLICATION ERROR ASTERISK-398
 Database query failed. Error received from database was ASTERISK-1147: Got a packet bigger than 'max_allowed_packet' for the query: INSERT INTO mantis_bug_file_table
(id, bug_id, title, description, diskfile, filename, folder, filesize, file_type, date_added, content)
VALUES

Anyway -- it's 3 packet captures along with the jitter buffer debug output.  Especially with the first 2, whenever I hit DTMF my jitter buffer goes nuts for the next while.  I don't *think* I'm actually losing packets, especially since it there aren't any (many) in the first run through the IVR I call.

The last packet capture and log is similar, except the time that the jitter buffer goes nuts is on the order of a few short seconds instead of tens of seconds.

By: Andrew Kohlsmith (akohlsmith) 2005-02-10 22:47:19.000-0600

oh just a datapoint -- after those tests my box's output from iax2 show stats was 81 outstanding frames (81 ingress, 0 outgress) -- When I was testing with our office * box (TE405P only one span turned up, swapped the T100P out for it to try an eliminate variables) I had the outstanding frames up to 3300 or so (all incoming) and eventually the * console locked up solid.  I was in a screen session so I could open another session and kill it off, but the screen session wouldn't let go, I had to kill bash off manually...  odd shit but once restarted it was great again and the outstanding frames climbed steadily (I think it was up around 1200 or so when I left, but I restarted it for this new patch so I can't tell now).

My home box has a single TDM420P in it and is a P3/700 and is, interestingly enough, sharing an IRQ between the TDM420P and my ethernet card...  and that card is busy, as the same box is my home media and NFS server for mythtv...

By: Andrew Kohlsmith (akohlsmith) 2005-02-10 22:47:20.000-0600

oh just a datapoint -- after those tests my box's output from iax2 show stats was 81 outstanding frames (81 ingress, 0 outgress) -- When I was testing with our office * box (TE405P only one span turned up, swapped the T100P out for it to try an eliminate variables) I had the outstanding frames up to 3300 or so (all incoming) and eventually the * console locked up solid.  I was in a screen session so I could open another session and kill it off, but the screen session wouldn't let go, I had to kill bash off manually...  odd shit but once restarted it was great again and the outstanding frames climbed steadily (I think it was up around 1200 or so when I left, but I restarted it for this new patch so I can't tell now).

My home box has a single TDM420P in it and is a P3/700 and is, interestingly enough, sharing an IRQ between the TDM420P and my ethernet card...  and that card is busy, as the same box is my home media and NFS server for mythtv...

By: stevekstevek (stevekstevek) 2005-02-11 18:56:58.000-0600

Cut 4 of the codec_plc stuff..

This adds a new config parameter to codecs.conf, which can turn off PLC for the people who want to save a few cycles..

It only affects the "generic" PLC algorithm, since there's really no overhead for the "native" PLC in speex, iLBC, etc..

By: Andrew Kohlsmith (akohlsmith) 2005-02-11 21:48:45.000-0600

Well I think stevek and I figured something out today.

This jitter buffer *hates* IAX2 Bridge Optimization.  *usually* when sending DTMF but sometimes even on its own (just voice frames) the iax2 bridge optimization code will screw with the voice frame timestamps as follows:

Real timestamps:
0 20 40 60 80 100 120 140 160 180 200 220 240 260 280...

timestamps after a an iax2 bridging host:
0 20 40 60 80 81 82 83 84 180 200 220 221 260 280...

I have many, many packet captures showing this behaviour and can even half-assed compensate for it with the following patch:

(near the end of schedule_delivery())
if(fr->af.frametype == AST_FRAME_VOICE) {
 type = JB_TYPE_VOICE;
 len = get_samples(&fr->af)/8;

 if(fr->ts < iaxs[fr->callno]->last_real_ts + ((len / 2)+1)) {
   iaxs[fr->callno]->num_fake_ts++;
   fr->ts = iaxs[fr->callno]->last_real_ts + (iaxs[fr->callno]->num_fake_ts * len) - 1;
 } else {
   iaxs[fr->callno]->last_real_ts = fr->ts;
   iaxs[fr->callno]->num_fake_ts = 0;
 }

What it does is rewrite the frame's timestamp if the frame comes in and it has a timestamp that is closer to the last timestamp than it is to what the next timestamp should be.  Obviously this is a band-aid solution.

The problem is in calc_fakestamp which is used with the bridge optimization in chan_iax2.c.  It's even listed as a FIXME? (who's SLD?):

/* FIXME? SLD would rather remove this and leave it to the end system to deal with */
if (fakets <= p2->lastsent)
 fakets = p2->lastsent + 1;
p2->lastsent = fakets;

I'm going to post something similar to this bugnote in -dev to see whether a new bug should be opened for this or not.  I'm hoping mark will have some time to comment, as it is not intuitive (at least to me) what calc_fakestamp is trying to do.

By: zoa (zoa) 2005-02-12 09:52:52.000-0600

I can confirm we are trying to expand this to RTP based channels.

By: stevekstevek (stevekstevek) 2005-02-14 14:04:20.000-0600

Updated Patches:

I re-organized these a bit.  You still need 3 patches to get the whole package:

* codec_plc.patch4.txt (33,980 bytes) 02-11-05 18:55
  Same as before: with configurable generic PLC

* jitterbuf.patch.txt (21,104 bytes) 02-14-05 13:49
  Just adds the jitterbuffer to asterisk core
  (adds jitterbuffer.h and jitterbuffer.c, and adds to Makefile).

  - Changes:  losspct is now a 1:500 decaying average of loss


*  jitterbuf-iax2.patch.txt (19,045 bytes) 02-14-05 13:49
   Applies jitterbuffer to chan_iax2.

Changes to chan_iax2 integration:
- You can choose old/new jitterbuffer implementations at compile time, even with patch applied.

- New jitterbuffer now honors "usejitterbuf" flag

- New jitterbuffer integrats with iax2 show netstats, for local and remote network status monitoring:

[this doesn't really fit here, but this is what you get -- imagine the channel ID being the first column]:

*CLI> iax2 show netstats
    -------- LOCAL ---------------------  -------- REMOTE --------------------
RTT  Jit  Del  Lost   %  Drop  OOO  Kpkts  Jit  Del  Lost   %  Drop  OOO  Kpkts
16   99  179    49   0     7    0      1   148  225    28   0     1    0      0

===========================

Reorganizing the patches this way should make it possible for someone working in chan_sip and rtp.c [or other VoIP technologies] to make a jitterbuf-rtp or jitterbuffer-sip patch, which would not conflict with these..

===========================
TODO:
- Make an "auto" option for the jitterbuffer, where it is normally on, but off, when the channel is bridged to another VoIP technology.
- Make other configurable options for JB behavior?
- Lots of testing and debugging.

TODO orthogonally to JB:
- Find/fix more issues where chan_iax2 generated incorrect (especially discontinuous) timestamps.  This should probably be a different bug.  The already identified places where this can happen are:
-- in calc_fakestamp, the "FIXME" should be fixed as SLD indicated.
-- Whenever we change from chan_iax2-generated timestamps, to timestamps generated from another source (or back), we need to ensure there is no timestamp discontinuity -- right now, there may be a huge discontinuity.

By: stevekstevek (stevekstevek) 2005-02-21 16:28:40.000-0600

Here's some fresh patches.  Aside from updating to today's CVS, changes include:

codec_plc:  Fix a bug in adpcm, and a bug in the case of plc being disabled (we'd add garbage to your audio, instead of silence; who knows what's better!).

jitterbuffer proper:  Update the history mechanism a bit; it uses just one history buffer, presently 500 frames long, but it's pretty smart about (a) finding minima and maxima quickly, and (b) avoiding doing so when they haven't changed.   This should make it possible to ignore spikes much better.

Also, slow the growth speed a bit..

jitterbuffer/iax2 integration: demonstration code to disable jitterbuffer when bridged to a VoIP channel.  It's really ugly right now, though; it does strcmp on channel->type.  You can disable this part by changing the handy #if 1 to #if 0, here:

#if 1   /* this is way expensive, with strcmp and all, but it's here to describe what could be done */

By: stevekstevek (stevekstevek) 2005-02-21 17:58:58.000-0600

Oops.  While that ugly code stops sending frames into the jb when you're bridged to a VoIP channel, it doesn't stop trying to get them out, and then, it thinks they're all lost.  This kinda messes up the statistics (we think we're losing 100% of the packets)..

By: mjr (mjr) 2005-02-21 22:05:10.000-0600

I tried this out today on a fresh CVS and the latest version of the patches.

Roughly 25% of the time, things worked very badly.  The other 75%, they were great.  I'm using g.729.

When things work well, it looks like this:

                               -------- LOCAL ---------------------  -------- REMOTE --------------------
Channel                    RTT  Jit  Del  Lost   %  Drop  OOO  Kpkts  Jit  Del  Lost   %  Drop  OOO  Kpkts
IAX2/chicago-oakland-3    1000    0    0     0   0     0    0      0    0    0     0   0     0    0      0

Which is strange because my actual RTT is way less than that.


When it doesn't work well, nearly no audible voice is heard.  The audio is bears some resemblance to what it should be, but is most just garbled noise.  The netstats look like this:

*CLI> iax2 show netstats
                               -------- LOCAL ---------------------  -------- REMOTE --------------------
Channel                    RTT  Jit  Del  Lost   %  Drop  OOO  Kpkts  Jit  Del  Lost   %  Drop  OOO  Kpkts
IAX2/chicago-oakland-1    1000    0   40   272  41     0    0      0    0    0     0   0     0    0      0
1 active IAX channel(s)

and Lost keeps going up...

*CLI> iax2 show netstats
                               -------- LOCAL ---------------------  -------- REMOTE --------------------
Channel                    RTT  Jit  Del  Lost   %  Drop  OOO  Kpkts  Jit  Del  Lost   %  Drop  OOO  Kpkts
IAX2/chicago-oakland-1    1000    0   40   693  74     0    0      0    0    0     0   0     0    0      0
1 active IAX channel(s)
*CLI> iax2 show netstats
                               -------- LOCAL ---------------------  -------- REMOTE --------------------
Channel                    RTT  Jit  Del  Lost   %  Drop  OOO  Kpkts  Jit  Del  Lost   %  Drop  OOO  Kpkts
IAX2/chicago-oakland-1    1000    0   40   738  76     0    0      0    0    0     0   0     0    0      0
1 active IAX channel(s)
*CLI> iax2 show netstats
                               -------- LOCAL ---------------------  -------- REMOTE --------------------
Channel                    RTT  Jit  Del  Lost   %  Drop  OOO  Kpkts  Jit  Del  Lost   %  Drop  OOO  Kpkts
IAX2/chicago-oakland-1    1000    0   40   870  82     0    0      0    0    0     0   0     0    0      0


When things are in "bad mode", hanging up and calling right back will usually fix it.

edited on: 02-21-05 22:06

By: zoa (zoa) 2005-02-22 03:05:53.000-0600

Please not that no changes were made to g729 so far.

g729 is not a good codec to evaluate these patches with (all others are).

and yes, we are trying to get kram to make some changes to g729 :)

By: stevekstevek (stevekstevek) 2005-02-22 08:49:26.000-0600

mjr:

As zoa said, the G.729 codec that digium sent out probably does not yet support PLC, so you won't see one of the main advantages of the new jitterbufer when using it.

As to your posted netstats:  They basically show that no packets are being received locally, and the remote side doesn't report anything (this could be because it doesn't support netstats, added recently to head, or something else).  

Really, it just seems like there might be a networking issue preventing the call from being established.

I don't think the jb should cause this, but I can't be sure it's not some strange bug.  To diagnose it, you'd want to run ethereal and see what's actually being sent back and forth.  Also, if we are to diagnose it, you'll need to describe the environment the call is being made in (i.e. describe all the servers/devices involved with the call, and how the call is made), along with a trace..

By: stevekstevek (stevekstevek) 2005-02-23 18:24:41.000-0600

Here's something that still needs to be worked out.  It doesn't cause a regression of any kind, but it does cause PLC to not get applied.

Presently, this patch only does PLC at a translator.  There's a bunch of situations where a call might come in via VoIP, and end up somewhere not VoIP, where without a translator.  In these cases, PLC won't be applied (you'll get, more or less, the present behavior, with either get silence substitution [SS] or time-compression [TC]):

Going to Zap [SS]
Going to app_record, voicemail, or anything using ast_writestream, when the channel's format == the file's format [i.e. recording GSM from a GSM channel, etc].

Maybe other applications I haven't thought of..

I'm not sure the best way to handle this;  I guess the following are things we know the same "useplc" flag that the translator uses should turn this on and off.

We could put code into zap to handle this easy enough, since the writing is all in zt_write (and/or zt_mywrite).  But that doesn't handle other channel types where the same issue may occur (other TDM cards? -- it will happen to all the "local" channels if the source format is slinear, but that's a pretty silly case).  We could put this into ast_write(), which would do the generic PLC stuff if there's no translator already in use..


It doesn't look that hard to put it into file.c/ast_writestream either, but that also doesn't fix all applications;  should we just put it in ast_read() for whenever a readtrans isn't being used?




If you have a call coming in via VoIP, and going to Zap (or any channel that doesn't need a translator), then the present PLC model gets bypassed.

Cases where this happens:

a)

By: stevekstevek (stevekstevek) 2005-03-04 13:48:22.000-0600

Alright, here's the present "up-to-date" patches;  god, I wish we used bugzilla so I could update the attachments on my own, etc..

Add PLC to codecs [unchanged]:
codec_plc.patch5.txt (34,088 bytes) 02-21-05 16:21

The Jitterbuffer
jitterbuf.patch3.txt (24,308 bytes) 03-04-05 13:39
- minor changes since last time to support a configurable hard-clamp.

The jitterbuffer's basic chan_iax2 implementation:
jitterbuf-iax2.patch3.txt (19,181 bytes) 03-04-05 13:39
- removed the hacky don't JB when bridged thing.
- Added support for maxjitterbuffer configuration.


Add a "properties" bitfield to channel_tech structure, and set a channel property called WANTSJITTER on channels which can accept jittered input from a pridged peer channel. [presently, that's IAX2, SIP, H323, MGCP, SKINNY
channel-properties.patch1.txt (2,188 bytes) 03-04-05 13:40

Using the properties bitfield above, suspend, and bypass the jb when bridged to a channel sporting the WANTSJITTER tech property flag.
jitterbuf-iax2-notalways.patch1.txt (11,727 bytes) 03-04-05 13:40

By: stevekstevek (stevekstevek) 2005-03-04 16:02:57.000-0600

crap: channel-properties.patch1.txt  missed setting .properties = AST_CHAN_TP_WANTSJITTER on chan_sip.  This is fixed with channel-properties.patch2.txt.


BugMarshalls:  If you'd like to clean up, you can remove all attachments except for the following:

codec_plc.patch5.txt (34,088 bytes) 02-21-05 16:21
jitterbuf.patch3.txt (24,308 bytes) 03-04-05 13:39
jitterbuf-iax2.patch3.txt (19,181 bytes) 03-04-05 13:39
jitterbuf-iax2-notalways.patch1.txt (11,727 bytes) 03-04-05 13:40
channel-properties.patch2.txt (2,546 bytes) 03-04-05 16:00

By: stevekstevek (stevekstevek) 2005-03-08 16:13:56.000-0600

I'm not even going to post this as a patch, but here's a quick hack that makes PLC work properly for chan_zap, when the incoming format is companded ulaw/alaw.  

Basically, we just force chan_zap to only advertise itself as supporting slinear mode.  Then, the already implemented mechanism of doing PLC during xlaw->slinear will work, and then zap will probably convert it back to xlaw in the kernel, or on the chip, if it supports that..

I'm not sure how people would want to implement this, though.  You could theoretically avoid the ulaw->slinear->ulaw conversions, except for the frame before a loss, if you wanted to..

Also, the first chunk of this patch didn't seem to do what I wanted it to..

Anyway, it's ugly, but with it, I can make an IAX2 call to some remote box playing MOH, and do "iax2 test losspct 10", and not notice it.  Without this, of course, it sounds pretty crappy.


diff -u -w -r1.414 chan_zap.c
--- channels/chan_zap.c 5 Mar 2005 02:08:37 -0000       1.414
+++ channels/chan_zap.c 8 Mar 2005 22:08:24 -0000
@@ -630,7 +630,8 @@
static const struct ast_channel_tech zap_tech = {
       .type = type,
       .description = tdesc,
-       .capabilities = AST_FORMAT_SLINEAR | AST_FORMAT_ULAW,
+//     .capabilities = AST_FORMAT_SLINEAR | AST_FORMAT_ULAW,
+       .capabilities = AST_FORMAT_SLINEAR,
       .requester = zt_request,
       .send_digit = zt_digit,
       .send_text = zt_sendtext,
@@ -4681,6 +4682,16 @@
               tmp->rawwriteformat = deflaw;
               tmp->writeformat = deflaw;
               i->subs[index].linear = 0;
+
+/* HACK */
+               tmp->nativeformats =
+               tmp->rawreadformat =
+               tmp->readformat =
+               tmp->rawwriteformat =
+               tmp->writeformat = AST_FORMAT_SLINEAR;
+               i->subs[index].linear = 1;
+/* /HACK */
+
               zt_setlinear(i->subs[index].zfd, i->subs[index].linear);
               features = 0;
               if (i->busydetect && CANBUSYDETECT(i)) {

By: stevekstevek (stevekstevek) 2005-03-10 13:24:42.000-0600

ack.  That hack doesn't work so well..  It works fine if the call is originated from zap, but not if it terminates to zap.  I'll have to see if there's a better hack later..

By: Mark Spencer (markster) 2005-03-11 02:40:14.000-0600

Why not use the existing channel flags, why add a new field?  Also, in CVS head I've added some new targets:

make patchlist
make PATCH=<foo> apply
make update
make PATCH=<foo> unapply

When you're ready, i can add this to patches/jitter_buf and then people can do "make PATCH=jitter_buf apply" to use it.

By: stevekstevek (stevekstevek) 2005-03-11 14:01:07.000-0600

Small updates here only:

1) Part of one patch had leaked into another, so the old set didn't apply correctly unless you applied the whole set.  You can now apply/test these incrementally.

2) There was an error in jb_getall, where it never actually returned frames from the jitterbuffer (ye olde 0 when we meant LONG_MAX or something).   This led to a memory/frame leak at the end of calls, or whenever resetting the jitterbuffer.

So, The active patches now are (in order):
codec_plc.patch5.txt (34,088 bytes) 02-21-05 16:21
jitterbuf.patch4.txt (24,509 bytes) 03-11-05 13:46
jitterbuf-iax2.patch4.txt (19,186 bytes) 03-11-05 13:47
channel-properties.patch3.txt (2,546 bytes) 03-11-05 13:47
jitterbuf-iax2-notalways.patch2.txt (11,541 bytes) 03-11-05 13:47


To Mark's question: I didn't want to use a channel flag, because the information I was trying to flag (/* Channels have this property if they can accept input with jitter; i.e. most VoIP channels */  AST_CHAN_TP_WANTSJITTER (1 << 0)) seems like a property of the channel technology, not something assigned to each individual call on the channel.   Figured I'd save 1 bit per call. ya know :)


Also, although I understand what your patch system does technically, I'm not sure how dropping this stuff into that is really going to help anything.  It's not going to make maintaining all of this any easier, because changes to the places where this patch goes will still keep happening.  I don't think that maintaining a patch in CVS is going to be any easier than just maintaining them here..

What we do need, is some more people actively testing this stuff and reporting issues..   I am now using it in production (just for the past 2 days or so, though), on a * box that is used for relatively low-volume IAX termination/origination (maybe a hundred calls/day) over a transatlantic link (to chan_zap).  I know tzanger has had it in production for a couple of weeks so far.

In general, the main known issues with this is that in some circumstances, it can be more sensitive to garbage timestamps that can be generated by chan_iax2.  The main identified time this happens is when you switch back and forth from locally generated (implicit, or predicted) timestamps, to explicit timestamps.  At those transition-points, chan_iax2 often makes a big jump, which the jitterbuffer can only see as jitter, and this adds some delay to the call, or can lose some audio, until things adjust again.   If tzanger doesn't actually write a patch using an algorithm I explained to him earlier sometime soon, I guess I'll do that.

By: stevekstevek (stevekstevek) 2005-03-14 11:12:44.000-0600

This new codec_plc patch ( codec_plc.patch6.txt (34,170 bytes) 03-14-05 11:11) has a one-character fix to the previous patch, which caused iLBC to crash when PLC was activated.

By: Brian West (bkw918) 2005-03-14 14:31:12.000-0600

Steve find me on IRC so we can clean this bug up and unify the patch into one thing.  PLEASE!!!!

/b

By: zoa (zoa) 2005-03-14 14:58:19.000-0600

Stevek, have a look at http://astertest.com/downloads/sip%20jitter%20buffer/jitterbuf.patch3-memleak-patch1.txt

It should fix a memleak. (i dont post it here as the .diff is not conform with the  patch guidelines.)

By: stevekstevek (stevekstevek) 2005-03-15 18:03:21.000-0600

OK, It's been explained to me that the separate patches are too complicated for people, expecially those whose browsers suck.

So, here's an updated patch which contains updates of the previous [5] patches here, _plus_ as a bonus, the patch from bug 3400.  It's created by catting things together, and vi will be very happy to take them apart again as well.

So, in the unlikely event that someone would actually want to review the code, things are still broken up into a logical progression.

We'll call this the "jumbopatch".  

Also: Zoa:  The leak has been addressed since 3/11/05 here, although I didn't use slav's method to do it.  Thanks for sending it to me.

BKW: I already helped your lazy ass out on IRC, and you have this code already.

Anyone:  I'm looking for feedback on how they might want to handle IAX2(ulaw) -> zap(ulaw) and PLC, because presently you'll still get silence substitution there, and my hack didn't seem to work too well.  A patch implementing that would be very cool.

By: stevekstevek (stevekstevek) 2005-03-15 18:58:24.000-0600

Fixed a patch-creation issue where jitterbuf.o wasn't included in Makefile.

By: stevekstevek (stevekstevek) 2005-03-17 14:59:39.000-0600

New jumbopatch.  Only change is moving opening braces on function definitions to the beginning of the next line in jitterbuf.c

As requested by mark.

By: stevekstevek (stevekstevek) 2005-03-17 15:26:52.000-0600

OK, plcoff.patch.txt makes generic PLC _off_ by default.  

It must now be turned on by setting genericplc => true in codecs.conf

Also removed unused "int res" local variable.

By: Mark Spencer (markster) 2005-03-17 15:35:09.000-0600

Added to CVS, thanks to everyone!

By: Olle Johansson (oej) 2005-03-17 15:51:25.000-0600

Can't find plc.h in cvs?

By: Olle Johansson (oej) 2005-03-17 16:01:18.000-0600

Patch jitterbuffer.txt
* removes unused variables in plc.c
* Adds FreeBSD compatibility to plc.c (told Coppice earlier)
* PLC.h needs to include inttypes.h on FreeBSD (missing file in CVS)

Disclaimer on file

By: Olle Johansson (oej) 2005-03-17 16:01:48.000-0600

Also removes duplicate function in chan_iax2.c

By: Brian West (bkw918) 2005-03-17 16:03:54.000-0600

Fixed

By: Olle Johansson (oej) 2005-03-17 16:26:06.000-0600

The FreeBSD compatibility part still needs to be fixed.

By: Olle Johansson (oej) 2005-03-17 16:32:59.000-0600

The FreeBSD patch is not committed.

By: Mark Spencer (markster) 2005-03-17 16:43:02.000-0600

Fixed for BSD (hopefully properly)

By: Olle Johansson (oej) 2005-03-18 01:29:14.000-0600

Mark: stdint.h does not include the definitions of INT16 MAX/MIN on my FreeBSD 4.10. Also, as noted earlier, we need to include inttypes.h in plc.h on FreeBSD.
(See earlier patch/bug note)

By: Olle Johansson (oej) 2005-03-18 01:38:28.000-0600

New patch file uploaded
* Fixes FreeBSD 4.10 compatibility (again) in plc.h
* Fixes FreeBSD 4.10 compatibility in plc.c
* Removes include of stdint.h in plc.c (it's done correctly already in plc.h)

Disclaimer on file.

By: Olle Johansson (oej) 2005-03-18 01:54:34.000-0600

Added README based on Steve K's mail to the -dev mailinglist. Good information.

By: Olle Johansson (oej) 2005-03-18 02:17:21.000-0600

BTW - I can't disclaim the README - Stevekstevek maybe wants to do that?

By: Kevin P. Fleming (kpfleming) 2005-03-18 07:59:51.000-0600

No disclaimer is needed for the README; first of all, it's text and doesn't fall under the GPL, and second it was previously posted to a public mailing list without any license or duplication restrictions applied.

By: Olle Johansson (oej) 2005-03-18 08:18:39.000-0600

I know, but out of courtesy I just wanted to get Steve's comments on reusing this text with my small changes...

/O ;-)

By: stevekstevek (stevekstevek) 2005-03-18 10:58:16.000-0600

I hereby disclaim the e-mail that OEJ used to write this readme.  In it's entirety, except for this header:

User-Agent: Debian Thunderbird 1.0 (X11/20050116)

By: Olle Johansson (oej) 2005-03-18 12:14:05.000-0600

Updated the readme with information about recent fix in Asterisk stable v1.0.7. Stevek: Please check my text about this in 3)  - I hope I understand this fully.

/O

By: stevekstevek (stevekstevek) 2005-03-18 13:13:26.000-0600

"Asterisk Stable v1.0.7 and later versions will be able to handle this without errors.
Trunking IAX2 calls from Asterisk to versions of Asterisk prior to v1.0.7,
you need to disable the jitterbuffer in the configuration file in order to avoid
strange problems with the trunk."

This is not correct.  If you send trunktimestamps to asterisk 1.0.6 or earlier, it will misinterpret the trunk frames, and bad things[tm] will happen.  If you send trunktimestamps to 1.0.7 or later, it will print warning messages, and discard these frames.  The calls still won't work.  [My earlier methods of implementing this would have still worked in this case, but ignored the trunktimestamps, for all previous versions, but they were much more complicated].

So, the rule still applies that if you send the trunktimestamps to earlier versions of asterisk, it will never work, but with 1.0.7+, the consequences are less severe, and you'll get some warning about the error.

By: Olle Johansson (oej) 2005-03-18 15:22:15.000-0600

A bit updated README after discussion with Stevek, but not necessarily done and approved...

By: Mark Spencer (markster) 2005-03-19 20:58:57.000-0600

Added to CVS, thanks!

By: Digium Subversion (svnbot) 2008-01-15 15:27:41.000-0600

Repository: asterisk
Revision: 5192

U   trunk/Makefile
U   trunk/channels/chan_h323.c
U   trunk/channels/chan_iax2.c
U   trunk/channels/chan_mgcp.c
U   trunk/channels/chan_sip.c
U   trunk/channels/chan_skinny.c
U   trunk/channels/iax2.h
U   trunk/codecs/codec_adpcm.c
U   trunk/codecs/codec_alaw.c
U   trunk/codecs/codec_g726.c
U   trunk/codecs/codec_gsm.c
U   trunk/codecs/codec_ilbc.c
U   trunk/codecs/codec_lpc10.c
U   trunk/codecs/codec_speex.c
U   trunk/codecs/codec_ulaw.c
U   trunk/configs/codecs.conf.sample
U   trunk/configs/iax.conf.sample
U   trunk/include/asterisk/channel.h
U   trunk/include/asterisk/translate.h
A   trunk/jitterbuf.c
A   trunk/jitterbuf.h
A   trunk/plc.c

------------------------------------------------------------------------
r5192 | markster | 2008-01-15 15:27:41 -0600 (Tue, 15 Jan 2008) | 2 lines

Add PLC and jitter buffer and iax2 meta trunk with timestamps (bug ASTERISK-2491, ASTERISK-3327)

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

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

By: Digium Subversion (svnbot) 2008-01-15 15:27:59.000-0600

Repository: asterisk
Revision: 5211

U   trunk/include/asterisk/plc.h
U   trunk/plc.c

------------------------------------------------------------------------
r5211 | markster | 2008-01-15 15:27:59 -0600 (Tue, 15 Jan 2008) | 2 lines

Fix PLC for BSD (bug ASTERISK-2491)

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

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