[asterisk-dev] [Code Review]: Add a configurable termination threshold to strictrtp's learning mode.

[jrose]

> On Jan. 13, 2012, 1:20 p.m., Mark Michelson wrote:
> > /branches/1.8/res/res_rtp_asterisk.c, line 524
> > <https://reviewboard.asterisk.org/r/1663/diff/2/?file=23157#file23157line524>
> >
> >     What is "st?"

Another remnant from when it was more similar to the one from pjmedia.  I'm getting rid of it.


> On Jan. 13, 2012, 1:20 p.m., Mark Michelson wrote:
> > /branches/1.8/res/res_rtp_asterisk.c, line 516
> > <https://reviewboard.asterisk.org/r/1663/diff/2/?file=23157#file23157line516>
> >
> >     You can get rid of these commented out stuffs.

Another thing I could have sworn I did... I'm beginning to think I must have gotten a little too crazy with UNDOs.


> On Jan. 13, 2012, 1:20 p.m., Mark Michelson wrote:
> > /branches/1.8/res/res_rtp_asterisk.c, line 531
> > <https://reviewboard.asterisk.org/r/1663/diff/2/?file=23157#file23157line531>
> >
> >     Here's where the use of uint16_t comes into play. If there is an RTP sequence number rollover during the probation period, then this will not properly detect that a transition from 65535 to 0 is just going up by 1.
> >     
> >     I think that the problem will correct itself eventually, but might as well make it easier.

I hadn't thought of using rollover to our advantage on this, but that does seem like a good answer.  I was noticing it was interpreting the sequence numbers as much less than they really were, but Asterisk was already using straight ints for seqno so I didn't think too much of it.  I'll fix all that.


- jrose


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1663/#review5168
-----------------------------------------------------------


On Jan. 13, 2012, 11:16 a.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1663/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2012, 11:16 a.m.)
> 
> 
> Review request for Asterisk Developers, Kevin Fleming, Matt Jordan, and jcolp.
> 
> 
> Summary
> -------
> 
> The purpose of this patch is to resolve some problems that can occur with peers using a mix of directmedia=yes and no occuring between multiple asterisk servers when strictrtp is enabled in rtp.conf.  When attempting to set the strictrtppeer during a reinvite, res_rtp_asterisk will ignore the requested sockaddr and instead set learning mode. Prior to this patch, learning mode would simply set the sockaddr owning the next related RTP packet received and then exit learning mode.  Now, learning mode will track how many times a particular socket address sent rtp packets without being interrupted by another socket address until it reaches the learning mode hit threshold value (set in rtp.conf) and then resume closed mode.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/res/res_rtp_asterisk.c 350549 
> 
> Diff: https://reviewboard.asterisk.org/r/1663/diff
> 
> 
> Testing
> -------
> 
> I tested this against two scenarios with the following configurations... always with strictrtp=yes
> 
> s1
> Phone 1 <-- DirectMedia=yes --> Asterisk 1 <-- DirectMedia=Yes --> Asterisk 2 <-- DirectMedia=No --> Phone 3
> 
> and
> 
> s2
> Phone 1 <-- DirectMedia=yes --> Asterisk 1 <-- DirectMedia=Yes --> Phone 2
> and then doing a blind transfer with:
> Phone 2 <-- DirectMedia=yes --> Asterisk 1 <-- DirectMedia=Yes --> Asterisk 2 <-- DirectMedia=No --> Phone 3
> 
> Prior to this test, in s1 there would be one way audio from phone 3 to phone 1 because RTP coming from phone 1 would be rejected by Asterisk 2.
> 
> In s2, audio would work normally from phone 1 to phone 2, but after the transfer the result would be the same with one way audio from phone 3 to phone 1 due to rejected
> packets from phone 1.
> 
> 
> After this patch is applied, both of these scenarios worked as expected as long as I used a threshold above 3.  Phones 1 and 3 were polycom SP430s while Phone 2 was a Grandstream GXP-2020.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120113/07b85409/attachment-0001.htm>