<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/3424/">https://reviewboard.asterisk.org/r/3424/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 10th, 2014, 4:21 p.m. EDT, <b>Corey Farrell</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/3424/diff/2/?file=57155#file57155line1070" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/apps/app_mixmonitor.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 mixmonitor_exec(struct ast_channel *chan, const char *data)</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">1070</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="tb"> </span><span class="n">snprintf</span><span class="p">(</span><span class="n">func</span><span class="p">,</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">func</span><span class="p">),</span> <span class="s">"PERIODIC_HOOK(%s,%s,%u)"</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;">Mark's suggestion to use ast_custom_function_find made me think about module references. We don't keep a reference to func_periodic_hook for the channels that have hooks. I missed this in the original review, I guess I expected the channel's datastore and/or the audiohook to reference the module, but they don't appear to. Not sure if you want to add the fix to this review or if we need to do it separate.</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;">Valid point, though I think it's a logically separate change from this, so I'll do it separately.</pre>
<br />
<p>- Russell</p>
<br />
<p>On April 8th, 2014, 7:49 p.m. EDT, Russell Bryant 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 Russell Bryant.</div>
<p style="color: grey;"><i>Updated April 8, 2014, 7:49 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;">Add an option to enable a periodic beep to be played into a call if it
is being recorded. If enabled, it uses the PERIODIC_HOOK() function
internally to play the 'beep' prompt into the call at a specified
interval.
</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;">exten => 103,1,Answer()
same => n,MixMonitor(test.gsm,B(5))
same => n,MusicOnHold()
exten => 104,1,Answer()
same => n,MixMonitor(test.gsm,B)
same => n,MusicOnHold()
exten => 105,1,Answer()
same => n,MixMonitor(test.gsm,B(3))
same => n,StartMusicOnHold()
same => n,Wait(15)
same => n,StopMusicOnHold()
same => n,StopMixMonitor()
same => n,Wait(5)
same => n,Hangup()
</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/main/app.c <span style="color: grey">(412023)</span></li>
<li>/trunk/include/asterisk/app.h <span style="color: grey">(412023)</span></li>
<li>/trunk/funcs/func_periodic_hook.c <span style="color: grey">(412023)</span></li>
<li>/trunk/apps/app_mixmonitor.c <span style="color: grey">(412023)</span></li>
<li>/trunk/CHANGES <span style="color: grey">(412023)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3424/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>