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

[jrose]

> On Jan. 13, 2012, 12:25 p.m., Matt Jordan wrote:
> > /branches/1.8/res/res_rtp_asterisk.c, line 2361
> > <https://reviewboard.asterisk.org/r/1663/diff/2/?file=23157#file23157line2361>
> >
> >     This isn't a terribly useful verbose message.  Since that's something users will see more often, it should be either something specific that is useful to them, or a debugging message (ideally it should include the sequence number that was set, as well as the RTP session pointer, which you referenced previously)

That's just a remnant of something.  Thanks for catching it.


> On Jan. 13, 2012, 12:25 p.m., Matt Jordan wrote:
> > /branches/1.8/res/res_rtp_asterisk.c, lines 543-545
> > <https://reviewboard.asterisk.org/r/1663/diff/2/?file=23157#file23157line543>
> >
> >     As you have it now, none of the udelta checking will ever get called.  If rtp->learning_probation is non-zero, we don't check any of it - and rtp->learning_probation should never be zero in this method as, as soon as it is 0, we set the learning mode to STRICT_RTP_CLOSED.  I'm not sure if this is what you expected or not, but it looks like the code that is executed when we receive a non-expected sequence number is not sufficient to account for all of the things that the pjmedia code was taking into account.

You are correct on the probation never being meant to be zero.  If anything in there is expecting a probation of 0, I'll go ahead and cull it since it was just a remnant from the pjmedia equivalent of this function.


> On Jan. 13, 2012, 12:25 p.m., Matt Jordan wrote:
> > /branches/1.8/res/res_rtp_asterisk.c, line 552
> > <https://reviewboard.asterisk.org/r/1663/diff/2/?file=23157#file23157line552>
> >
> >     Blob

Beginning to wish I'd taken the time to set up my vimrc on that machine a little better.


- jrose


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


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/6d77bf53/attachment.htm>