<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1663/">https://reviewboard.asterisk.org/r/1663/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Kevin Fleming, Matt Jordan, and jcolp.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated Jan. 13, 2012, 11:16 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">After discussing how pjmedia does this, we decided it would probably be best to mirror how they do it as much as possible. As such, the initial configurable concept behind this is gone for now, though it might be a good idea to bring it back in...
This patch brings functions over related to the concept of 'probation' which is basically the same idea as our learning mode from pjmedia. These are used to check sequence numbers on RTP to determine whether or not they should be considered as valid for accepting the new source when making transitions of RTP sources. The rtp structure also receives a few new values (some possibly redundant and/or unnecessary) used mostly to keep the manipulation performed by the new learning mode functions from spilling over into our activity outside of learning mode.
pjmedia uses a value of 2 for MIN_SEQUENTIAL, and when I attempted to use this value for our analog to that (LEARNING_MIN_SEQUENTIAL), I found that we still frequently received enough frames from the original Asterisk box to exit learning mode before hitting any phones from the frame. Changing this to 3 made this less frequent while changing it to 4 alleviated the problem in many repeated tests and I didn't receive any more failures. Unlike in the old learning mode, as long as we are in learning mode, any frames coming in will be dropped, so in the catastrophic case where no RTP source ever shuts up, what will happen instead of them just establishing the wrong source is frames will always be dropped, so it's somewhat safer. This appears to be how pjmedia handles its probation mode... which is probably why it uses so much grimmer of a name than we do.
Since this MIN_SEQUENTIAL value is requiring us to tweak it, I'm wondering if we should make this value configurable as the threshold value from before was. Likewise, it might be worthwhile to go ahead and add an elapsed time check just so that we can report a problem if we do remain in probation for an extended period of time, possibly coupled with some useful debugging information. If these aren't too concerning though, we could go ahead and throw this in as-is.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/res/res_rtp_asterisk.c <span style="color: grey">(350549)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1663/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>