<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/3721/">https://reviewboard.asterisk.org/r/3721/</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 17th, 2014, 3:33 p.m. CDT, <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/3721/diff/3/?file=64757#file64757line95" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/funcs/func_audiohookinherit.c</a>
<span style="font-weight: normal;">
(Diff revision 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">93</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span>as a result of small set of similar operations for which inheriting all</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">94</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span>audiohooks from the<span class="tb"> </span>original channel is now safe, so in Asterisk 12.5+,</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;">... result of a small ...
Extra space between "the original"</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;">It's a trap! It's actually a tab and in my editor (4 spaces per tab), it just happens to have occupied just the right position for it to be visually identical to a space. :(</pre>
<br />
<p>- Jonathan</p>
<br />
<p>On July 17th, 2014, 3:10 p.m. CDT, Jonathan Rose 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, Joshua Colp, Matt Jordan, Mark Michelson, and rmudgett.</div>
<div>By Jonathan Rose.</div>
<p style="color: grey;"><i>Updated July 17, 2014, 3:10 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;">It involves masquerades, so you know it's perilous. It involves FAX, so you know it's maddening.
The idea is fairly fairly simple at its core. During a masquerade, audiohooks and framehooks will now be moved automatically (unless explicitly set not to via a value in the framehook interface). Framehooks are a little more complicated and some are simply not inheritable while others may require some kind of fixup (none currently use this. The framehooks in res_fax ended up getting a little too wierd to work well with the framehook fixup, so its fixup had to be handled as part of the datastore fixup function.)
It might be more appropriate to do this as an Asterisk 13 patch. I'm uncertain right now. There are some crashes resolved by this (including https://reviewboard.asterisk.org/r/3603/ which Corey Farrell is currently trying to solve for 11 and maybe for earlier versions as well), but it's a moderately big change and it likely has some potential to break things.</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;">Tested masquerades on channels containing every different type of framehook. Tested masquerades on channels containing every type of audiohook except for app_jack (extended support and I haven't every used it, so I would need to dig into its dependencies... plus none of the audiohooks so far have needed any special attention anyway and app_jack looks less likely too than some of the others).
For everything with obvious audio impact (volume gain, mixmonitors, pitch shift, chanspy etc), I also checked that audio kept going through as you would expect it to. I also tested multiple audiohooks and multiple framehooks for inheritance. Audiohooks were tested for multiple inheritance of the same type (previously unavailable with the AUDIOHOOK_INHERIT function)</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_pjsip_refer.c <span style="color: grey">(418716)</span></li>
<li>/branches/12/res/res_fax.c <span style="color: grey">(418716)</span></li>
<li>/branches/12/main/framehook.c <span style="color: grey">(418716)</span></li>
<li>/branches/12/main/channel.c <span style="color: grey">(418716)</span></li>
<li>/branches/12/main/bridge_basic.c <span style="color: grey">(418716)</span></li>
<li>/branches/12/main/audiohook.c <span style="color: grey">(418716)</span></li>
<li>/branches/12/include/asterisk/res_fax.h <span style="color: grey">(418716)</span></li>
<li>/branches/12/include/asterisk/framehook.h <span style="color: grey">(418716)</span></li>
<li>/branches/12/include/asterisk/audiohook.h <span style="color: grey">(418716)</span></li>
<li>/branches/12/funcs/func_audiohookinherit.c <span style="color: grey">(418716)</span></li>
<li>/branches/12/bridges/bridge_native_rtp.c <span style="color: grey">(418716)</span></li>
<li>/branches/12/CHANGES <span style="color: grey">(418716)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3721/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>