<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/4549/">https://reviewboard.asterisk.org/r/4549/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 31st, 2015, 12:23 p.m. CDT, <b>Mark Michelson</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;">One thing to take into consideration here is that there are some places within Asterisk where we will send an AST_CONTROL_UNHOLD frame on a channel, even though it may not currently be on hold. This means you may send some unhold ARI events that don't match up with a previous hold event. This is probably worth documenting somewhere so that ARI application writers know what they might have to deal with.</pre>
</blockquote>
<p>On March 31st, 2015, 2:21 p.m. CDT, <b>Matt Jordan</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;">In what circumstances do we do that?</pre>
</blockquote>
<p>On March 31st, 2015, 2:26 p.m. CDT, <b>Mark Michelson</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;">I was thinking of transfer code in particular. The transfer code does not know whether the channel being transferred is on hold or not, but it will send an unhold frame anyway.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hm. That might actually be a slight issue in the transfer code. The channel structure has a property on it that is ostensibly supposed to track if a channel is on hold or not, ast_channel_hold_state. This maps to the hold_state property on the channel:
int hold_state; /*!< Current Hold/Unhold state */
We should be able to look at that to see if the channel is currently on hold before queuing up an unhold frame.
bridge_channel does do this when a channel leaves the bridge:
/* Complete any active hold before exiting the bridge. */
if (ast_channel_hold_state(bridge_channel->chan) == AST_CONTROL_HOLD) {
ast_debug(1, "Channel %s simulating UNHOLD for bridge end.\n",
ast_channel_name(bridge_channel->chan));
ast_indicate(bridge_channel->chan, AST_CONTROL_UNHOLD);
}
While there are places where the bridging framework sends an unhold to the channel, they appear to all be matched with a hold frame. Which particular place are you thinking of?</pre>
<br />
<p>- Matt</p>
<br />
<p>On March 27th, 2015, 10:19 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 and Joshua Colp.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated March 27, 2015, 10:19 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-24922">ASTERISK-24922</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;">For some applications - such as SLA - a phone pressing hold should not behave in the fashion that the Asterisk core would like it to. Instead, the hold action has some application specific behaviour associated with it - such as disconnecting the channel that initiated the hold; only playing MoH to channels in the bridge if the channels are of a particular type, etc.
One way of accomplishing this is to use a framehook to intercept the hold/unhold frames, raise an event, and eat the frame. Tasty. The patch attached to this issue accomplished that as a new dialplan function, HOLD_INTERCEPT.
In addition:
* ARI now queues hold/unhold frames instead of indicating frames directly. This allows for the Stasis hold/unhold messages to be raised.
* Some general cleanup of raising hold/unhold Stasis messages was done, including removing some RAII_VAR usage.</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;">See Gerrit reviews:
https://gerrit.asterisk.org/#/c/16
https://gerrit.asterisk.org/#/c/17</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/13/rest-api/api-docs/events.json <span style="color: grey">(433677)</span></li>
<li>/branches/13/res/stasis/control.c <span style="color: grey">(433677)</span></li>
<li>/branches/13/res/stasis/app.c <span style="color: grey">(433677)</span></li>
<li>/branches/13/res/ari/ari_model_validators.c <span style="color: grey">(433677)</span></li>
<li>/branches/13/res/ari/ari_model_validators.h <span style="color: grey">(433677)</span></li>
<li>/branches/13/main/stasis_channels.c <span style="color: grey">(433677)</span></li>
<li>/branches/13/main/manager_channels.c <span style="color: grey">(433677)</span></li>
<li>/branches/13/main/channel.c <span style="color: grey">(433677)</span></li>
<li>/branches/13/main/bridge_channel.c <span style="color: grey">(433677)</span></li>
<li>/branches/13/funcs/func_holdintercept.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/CHANGES <span style="color: grey">(433677)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4549/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>