<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/3338/">https://reviewboard.asterisk.org/r/3338/</a>
</td>
</tr>
</table>
<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/3338/diff/2-3/?file=55854#file55854line976" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/main/bridge_channel.c</a>
<span style="font-weight: normal;">
(Diff revisions 2 - 3)
</span>
</th>
</tr>
</thead>
<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">965</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="kt">unsigned</span> <span class="kt">int</span> <span class="n">id</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 added this field but it is not used anywhere.
</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/3338/diff/3/?file=55895#file55895line662" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/bridges/bridge_softmix.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 int softmix_bridge_write(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)</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">662</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">case</span> <span class="n">AST_FRAME_BRIDGE_ACTION_SYNC</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;">Doing this will require you to deal with the action running on all other channels in the bridge. You are currently setup to handle only one channel.</pre>
</div>
<br />
<p>- rmudgett</p>
<br />
<p>On March 13th, 2014, 2 p.m. CDT, Mark Michelson 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 and rmudgett.</div>
<div>By Mark Michelson.</div>
<p style="color: grey;"><i>Updated March 13, 2014, 2 p.m.</i></p>
<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;">ARI tests were occasionally showing a channel being stuck forever in Stasis. After looking at a live system where the problem was occurring, it became clear that the channel was stuck trying to wait for a playback to finish. However, it was also clear that the playback had been abandoned long ago.
When playing back a file to a channel that is in a bridge, the idea was to queue the playfile action onto the corresponding bridge channel, and then wait until the ARI custom playback function completed to signal that the playback had finished. There were several ways that this could fail:
* If the bridge channel were not found in the bridge, then we would never attempt to queue the playfile action, but we would still try to wait for it to finish happening.
* If queuing the playfile action did not succeed, then we would still attempt to wait for the action to occur.
* If the playback action was successfully queued, but the bridge channel were removed from the bridge before the playfile action could start, then the playback would never happen, and we'd wait forever.
The responsibility of knowing whether a playfile action has conpleted or ever will occur is the bridge's, since ARI can never fully know. With this in mind, I have created a new bridge channel API call to queue a playfile action synchronously. The way it is done, synchronous queuing operations could be created for other bridge actions quite easily. A new frametype, AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a synchronization ID and the actual payload of the bridge action that is to be queued. When the frame is queued, a synchronization structure is created and added to a linked list. The procedure then blocks until it is told that the frame has been disposed of.
When the frame gets freed (which will occur whether the playfile action succeeds or does not happen), the freer of the frame signals the waiting procedure that the playfile action has terminated, and control returns to whoever queued the playfile action in the first place.
>From ARI's perspective, this greatly simplifies its code. Most of the res_stasis_playback changes are code removal. The bridge_channel changes, though, are pretty large, which is why I've included Richard on this issue.</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;">Initially, I did some manual playbacks using Swagger-UI to a channel in a bridge to ensure that the specified file was actually being played as promised. I also queued up several files in quick succession to ensure that they played in series and not on top of each other.
In addition, I have created an automated test that is up for review at /r/3339</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/res/res_stasis_playback.c <span style="color: grey">(410467)</span></li>
<li>/branches/12/main/frame.c <span style="color: grey">(410467)</span></li>
<li>/branches/12/main/channel.c <span style="color: grey">(410467)</span></li>
<li>/branches/12/main/bridge_channel.c <span style="color: grey">(410467)</span></li>
<li>/branches/12/include/asterisk/frame.h <span style="color: grey">(410467)</span></li>
<li>/branches/12/include/asterisk/bridge_channel.h <span style="color: grey">(410467)</span></li>
<li>/branches/12/funcs/func_frame_trace.c <span style="color: grey">(410467)</span></li>
<li>/branches/12/bridges/bridge_softmix.c <span style="color: grey">(410467)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3338/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>