<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/1894/">https://reviewboard.asterisk.org/r/1894/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<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'm not seeing anything wrong with the code here. The only suggestions I have are style improvements, which you are free to ignore.</pre>
<br />
<div>
<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/1894/diff/1/?file=27630#file27630line529" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/channels/chan_local.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void check_bridge(struct local_pvt *p)</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void check_bridge(struct ast_channel *ast, struct local_pvt *p)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">522</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">ast_check_hangup</span><span class="p">(</span><span class="n">p</span><span class="o">-></span><span class="n">owner</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">526</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">ast_check_hangup</span><span class="p">(</span><span class="n">p</span><span class="o">-></span><span class="n">chan</span><span class="o">-></span><span class="n">_bridge</span><span class="p">)</span> <span class="o">&&</span> <span class="o">!</span><span class="n">ast_channel_trylock</span><span class="p">(</span><span class="n">p</span><span class="o">-></span><span class="n">owner</span><span class="p">))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You may as well invert this and exit early so you can reduce indentation a bit here.</pre>
</div>
<br />
<div>
<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/1894/diff/1/?file=27630#file27630line546" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/channels/chan_local.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void check_bridge(struct local_pvt *p)</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void check_bridge(struct ast_channel *ast, struct local_pvt *p)</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">543</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="o">!</span><span class="n">f</span> <span class="o">&&</span> <span class="o">!</span><span class="n">ast_check_hangup</span><span class="p">(</span><span class="n">p</span><span class="o">-></span><span class="n">owner</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You could also invert this if statement in order to decrease further indentation.</pre>
</div>
<br />
<p>- Mark</p>
<br />
<p>On May 2nd, 2012, 12:22 p.m., rmudgett 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 rmudgett.</div>
<p style="color: grey;"><i>Updated May 2, 2012, 12:22 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;">The masquerade torture test (ASTERISK-19807) is a long chain of local channels to be optimized out of a call when answered.
This patch fixes local channel so it can successfully optimize itself out when there are chains of channels.
The principle problem was check_bridge() failed to check the ast_channel_masquerade() return value to see if the masquerade was actually setup. In long chains of local channels, the masquerade occasionally fails to get setup because there is another masquerade already setup on an adjacent local channel in the chain.
* Checks the return value of ast_channel_masquerade().
* Made sure that the outgoing local channel (the ;2 channel) also does not have any frames in its queue before the masquerade.
* The outgoing channel will flush one voice or video frame per optimization attempt.
* Do the masquerade immediately to ensure that the outgoing channel queue does not get any new frames added and thus unconditionally flushed.
* Blocks indication -1 (Stop tones event) when the local channel is going to optimize itself out. When the call is answered, a chain of local channels pass down a -1 indication for each bridge. This blizzard of -1 events really slows down the optimization process.</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;">Chains of 100 local channels now are able to optimize themselves out.</pre>
</td>
</tr>
</table>
<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-16711">ASTERISK-16711</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/channels/chan_local.c <span style="color: grey">(364846)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1894/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>