<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/1946/">https://reviewboard.asterisk.org/r/1946/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 29th, 2012, 10:04 a.m., <b>Mark Michelson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/1946/diff/2/?file=28284#file28284line30604" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *instance, struct ast_rtp_instance *vinstance, struct ast_rtp_instance *tinstance, const struct ast_format_cap *cap, int nat_active)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">30604</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">ast_test_flag</span><span class="p">(</span><span class="o">&</span><span class="n">p</span><span class="o">-></span><span class="n">flags</span><span class="p">[</span><span class="mi">2</span><span class="p">],</span> <span class="n">SIP_PAGE3_DIRECT_MEDIA_OUTGOING</span><span class="p">)</span> <span class="o">&&</span> <span class="o">!</span><span class="n">p</span><span class="o">-></span><span class="n">outgoing_call</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">30605</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_log</span><span class="p">(</span><span class="n">LOG_NOTICE</span><span class="p">,</span> <span class="s">"Can't send reinvite because it is an incoming call!</span><span class="se">\n</span><span class="s">"</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">30606</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">sip_pvt_unlock</span><span class="p">(</span><span class="n">p</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">30607</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_channel_unlock</span><span class="p">(</span><span class="n">chan</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I decided to do another round of testing before committing and noticed that despite having directmedia=outgoing set, I would see reinvite glares occasionally. I then noticed this block of code. I had meant to be returning here instead of continuing with an unlocked sip_pvt and channel. Adding the return results in one-way audio in my tests, so I need to figure out why that is happening and how to fix it.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Welp, it looks like this implementation of the patch is not going to work after all.
Consider a situation where phone A places a call through Asterisk 1, which then calls through Asterisk 2, which then calls phone B. Direct media is desired. With this patch, here is what occurs:
Upon call setup, Asterisk 1 sends a direct media reinvite to phone A saying to send media to Asterisk 2.
Since the call is outgoing, Asterisk 1 also sends a direct media reinvite to Asterisk 2 saying to send media to phone A.
Upon call setup, Asterisk 2 sends a direct media reinvite to phone B saying to send media to Asterisk 1.
Since the call is outgoing, Asterisk 2 does not send a direct media reinvite to Asterisk 1.
Upon receiving the direct media reinvite from Asterisk 1, Asterisk 2 then sends out a direct media reinvite to phone B to send media to phone A.
This is the final media setup of the call. Phone B properly sends media to Phone A, but Phone A is sending media to Asterisk 2.
I actually see Asterisk 1 send another reinvite each to phone A and to Asterisk 2, but the media addresses (c= lines in SDP) are the same as the previous reinvites (perhaps this is due to a connected line change?).
The issue here is that Asterisk 1 is never told that the remote media address on the outbound call leg has been updated to be phone B. Normally, Asterisk 1 would know of this change because of a reinvite being received on the outbound call leg indicating the address change. However, reinvites from Asterisk 2 to Asterisk 1 get blocked.
The only way I could get this setup to work properly was if I set nat=comedia so that for the brief period where Phone B sends media to Asterisk 1, Asterisk 1 detects a media address change and thus reinvites Phone A to send media to Phone B. I had strictrtp turned off, so this may not have worked properly with strictrtp enabled.
I'm at a bit of a loss for how to handle this properly. Obviously, Asterisk 2 has to be able to send a reinvite to Asterisk 1 so that Asterisk 1 can reinvite phone A properly. The obvious answer is to have a setting so that directmedia=outgoing means that the outgoing leg reinvites *first* rather than meaning that only the outgoing leg reinvites. The thing to work out for this is going to be figuring out how to trigger Asterisk 2 in the above scenario to send a reinvite to Asterisk 1. Is it enough to complete a single reinvite transaction between Asterisk 1 and 2 before Asterisk 2 is allowed to send a reinvite to Asterisk 1? I don't know for sure. I think that if there are more than 2 Asterisk servers in between the phones, then waiting for a single reinvite transaction might not be enough to effectively reduce the chances of glare.
Basically, the long and short of this is that the problem is not as easily solvable as what I have hear and will need a more complex solution. I am withdrawing this review until I come up with a better solution.</pre>
<br />
<p>- Mark</p>
<br />
<p>On May 25th, 2012, 2:12 p.m., Mark Michelson wrote:</p>
<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.</div>
<div>By Mark Michelson.</div>
<p style="color: grey;"><i>Updated May 25, 2012, 2:12 p.m.</i></p>
<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;">There are times where multiple Asterisk servers are peered together over SIP. In such situations, it is possible for both Asterisk servers to attempt to send direct media reinvites to each other simultaneously. This results in a glare situation in which each of the Asterisk servers sends a 491 to the other. After a waiting period, the reinvites are re-attempted. This waiting period can potentially be distracting since it can cause the media to take multiple seconds to finalize, especially if more than 2 Asterisk servers are involved.
This patch introduces a new SIP peer option called "directmedia_outgoing". If enabled, then when communicating with the peer, Asterisk will only attempt to send reinvites if the call direction is outgoing. The assumption is that the peer Asterisk server will also have this setting enabled. This way, when the two Asterisk servers communicate, they will never attempt to send direct media reinvites to each other. Instead, it will always be the peer that placed the call that will send the direct media reinvite.</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 have tested this by running two Asterisk servers and ensuring that the option was honored and that the media streams were still set up properly.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/channels/chan_sip.c <span style="color: grey">(367639)</span></li>
<li>/trunk/channels/sip/include/sip.h <span style="color: grey">(367639)</span></li>
<li>/trunk/configs/sip.conf.sample <span style="color: grey">(367639)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1946/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>