<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/3997/">https://reviewboard.asterisk.org/r/3997/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 13th, 2014, 2:04 p.m. CDT, <b>rmudgett</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/3997/diff/3/?file=68004#file68004line143" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/bridges/bridge_native_rtp.c</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 enum ast_rtp_glue_result native_rtp_bridge_get(struct ast_channel *c0, struct ast_channel *c1, struct ast_rtp_glue **glue0,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">143</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="n">c0</span> <span class="o"><span class="hl">==</span></span><span class="hl"> </span><span class="n">c1</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">143</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o"><span class="hl">!</span></span><span class="n"><span class="hl">b</span>c0</span> <span class="o"><span class="hl">||</span></span><span class="hl"> </span><span class="o"><span class="hl">!</span></span><span class="n"><span class="hl">b</span>c1</span><span class="p">)</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;">Needs to be
if (bc0 == bc1)
to handle the first channel joining the bridge tech case.
</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;">If that's the case, then wouldn't bc1 be NULL?</pre>
<br />
<p>- Matt</p>
<br />
<p>On October 13th, 2014, 12:32 p.m. CDT, Matt Jordan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated Oct. 13, 2014, 12:32 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-24327">ASTERISK-24327</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<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;">When a native RTP bridge that is remotely bridging its participants switches to a softmix bridge, it may not properly re-INVITE the media for one or both participants back to Asterisk. This is due to two factors:
(1) The current bridge_native_rtp code only re-INVITEs if it believes the channel will survive the bridge operation. Currently, that code is failing, as it expects the channels to have a soft hangup flag set on it indicating that a redirect has occurred or that the channel is going to leave the bridge. (The code did not take into account a smart bridge operation).
(2) When the bridge layer performs a smart bridge, it passes a dummy bridge down into the old mixing technology when it is stopped. That breaks the native RTP bridge, as it looks to bridge->channels to know which channels to re-INVITE back. That list has no entries, as the dummy bridge does not populate that value.
This patch modifies bridge_native_rtp such that it keeps track of the channels itself. Given how tricky this code is - both smart bridging and native RTP bridging - this keeps the mixing technology insulated from changes in the core, which is probably a good thing.</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;">The tests that extercised this code the most are the PJSIP blind transfer tests, as they change the bridge mixing technology from native_rtp to simple and back in various tests. Shocking the callee_with_hold/caller_with_hold tests worked right off the bat. The direct media tests still fail, but this is not surprising as the messages from Asterisk arrive interleaved, which is not something SIPp handles well (at all).
</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>/branches/12/bridges/bridge_native_rtp.c <span style="color: grey">(425404)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3997/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>