<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/2670/">https://reviewboard.asterisk.org/r/2670/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 18th, 2013, 7:43 p.m. UTC, <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/2670/diff/2-3/?file=41376#file41376line691" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/core_unreal.c</a>
<span style="font-weight: normal;">
(Diff revisions 2 - 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; ">void ast_unreal_call_setup(struct ast_channel *semi1, struct ast_channel *semi2)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">684</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">chan</span> <span class="o">=</span> <span class="n">p</span><span class="o">-></span><span class="n">chan</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">691</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">chan</span> <span class="o">=</span> <span class="n">p</span><span class="o">-></span><span class="n">chan</span><span class="p">;</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">685</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">chan</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">692</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">chan</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">686</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="k">return</span> <span class="o">-</span><span class="mi">1</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">693</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="k">return</span> <span class="o">-</span><span class="mi">1</span><span class="p">;</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">687</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="p">}</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">694</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="p">}</span></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">695</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">owner</span> <span class="o">=</span> <span class="n">p</span><span class="o">-></span><span class="n">owner</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">696</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">owner</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">697</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="k">return</span> <span class="o">-</span><span class="mi">1</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">698</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="p">}</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">688</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">ast_channel_ref</span><span class="p">(</span><span class="n">chan</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">699</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">ast_channel_ref</span><span class="p">(</span><span class="n">chan</span><span class="p">);</span></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">700</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">ast_channel_ref</span><span class="p">(</span><span class="n">owner</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;">Move this code out of the block. It does not need to be done while ast is locked. It should be done while p is locked.</pre>
</blockquote>
<p>On July 18th, 2013, 8:44 p.m. UTC, <b>jrose</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So if I have this right... I should have the following:
ast is locked. The pvt is obtained and referenced. ast is unlocked.
With the pvt referenced, I lock it and grab both owner and chan out of the pvt and then unlock the pvt.
Then if I have the bridge call ID, I lock one channel, check for a call ID, if it doesn't have one I set it to the bridge call ID, then unlock that channel and do the same with the other channel.</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;">Correct.</pre>
<br />
<p>- rmudgett</p>
<br />
<p>On July 18th, 2013, 8:45 p.m. UTC, jrose 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.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, David Lee, kmoore, Matt Jordan, and rmudgett.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated July 18, 2013, 8:45 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-21592">ASTERISK-21592</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-21593">ASTERISK-21593</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;">The diff has become somewhat wide reaching, but this review primarily takes care of the following:
1. Provides a new channel driver for creating unreal channels with specific roles already applied to them and a mechanism for shoving the ;2 end of that channel into a bridge
2. Provides Playback and Record on Bridge ARI functions which make use of that channel
In the process of working on this a few bugs got in the way and this review takes care of those as well.
1. Confbridge announcer channels have a reference leak and don't get destroyed on leaving the bridge
2. Numerous ARI function documentation items were flawed and others didn't match the intended implementation and had to be tweaked.</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;">All ARI playback and recording functions on channels were tested to make sure they pass validation after the changes I've made
All ARI playback and recording functions on bridges were tested for functionality in softmix bridges
I also did some rudimentary tests to see that memory wasn't leaking in the files I touched under these normal conditions. It's hardly exhaustive, but it's a start.</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_bridge_media.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/include/asterisk/channel.h <span style="color: grey">(394203)</span></li>
<li>/trunk/include/asterisk/core_unreal.h <span style="color: grey">(394203)</span></li>
<li>/trunk/include/asterisk/logger.h <span style="color: grey">(394203)</span></li>
<li>/trunk/include/asterisk/stasis_app.h <span style="color: grey">(394203)</span></li>
<li>/trunk/include/asterisk/stasis_app_playback.h <span style="color: grey">(394203)</span></li>
<li>/trunk/main/channel.c <span style="color: grey">(394203)</span></li>
<li>/trunk/main/core_unreal.c <span style="color: grey">(394203)</span></li>
<li>/trunk/res/res_stasis.c <span style="color: grey">(394203)</span></li>
<li>/trunk/res/res_stasis_http_bridges.c <span style="color: grey">(394203)</span></li>
<li>/trunk/res/res_stasis_http_channels.c <span style="color: grey">(394203)</span></li>
<li>/trunk/res/res_stasis_http_playback.c <span style="color: grey">(394203)</span></li>
<li>/trunk/res/res_stasis_playback.c <span style="color: grey">(394203)</span></li>
<li>/trunk/res/stasis/control.c <span style="color: grey">(394203)</span></li>
<li>/trunk/res/stasis_http/ari_model_validators.h <span style="color: grey">(394203)</span></li>
<li>/trunk/res/stasis_http/ari_model_validators.c <span style="color: grey">(394203)</span></li>
<li>/trunk/res/stasis_http/resource_bridges.h <span style="color: grey">(394203)</span></li>
<li>/trunk/res/stasis_http/resource_bridges.c <span style="color: grey">(394203)</span></li>
<li>/trunk/res/stasis_http/resource_channels.c <span style="color: grey">(394203)</span></li>
<li>/trunk/rest-api/api-docs/bridges.json <span style="color: grey">(394203)</span></li>
<li>/trunk/rest-api/api-docs/channels.json <span style="color: grey">(394203)</span></li>
<li>/trunk/rest-api/api-docs/playback.json <span style="color: grey">(394203)</span></li>
<li>/trunk/rest-api/api-docs/recordings.json <span style="color: grey">(394203)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2670/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>