<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/3999/">https://reviewboard.asterisk.org/r/3999/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 18th, 2014, 8:12 a.m. CDT, <b>Joshua Colp</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/3999/diff/1/?file=67373#file67373line4126" style="color: black; font-weight: bold; text-decoration: underline;">/branches/13/channels/chan_iax2.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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 void __get_from_jb(const void *p)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4126</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span>ret = jb_get(pvt->jb, &frame, ms, ast_format_get_default_ms(voicefmt));</pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4126</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span>ret = jb_get(pvt->jb, &frame, ms, <span class="hl">voicefmt ? </span>ast_format_get_default_ms(voicefmt)<span class="hl"> : 0</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;">Despite it not changing behavior I'd still use 20 here to match 12.</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;">Alright, fixed that. The only difference in the code is that 20 is there instead of 0, so I think I'll hold off on actually posting another review for now.</pre>
<br />
<p>- Jonathan</p>
<br />
<p>On September 16th, 2014, 4:28 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, Matt Jordan and rmudgett.</div>
<div>By Jonathan Rose.</div>
<p style="color: grey;"><i>Updated Sept. 16, 2014, 4:28 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-24265">ASTERISK-24265</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;">This only occurs when the chan_iax jitterbuffer options are set and no when setting jitterbuffers via diaplan or anything like that.
The first time __get_from_jb is called, voiceformat has not been set on the IAX pvt. Trying to call ast_format_get_default_ms on a NULL pointer fails. This worked previously because Asterisk 12 and prior simply modified an ast_format on the stack, so when it used ast_codec_interp_len on that format pointer there was no possibility for it to be a NULL pointer... just one that doesn't have a real format associated with it.
One thing I might not be doing right here is that I'm using an interpolation value of 0 for a NULL format. Previously Asterisk would just check to see if the format was ILBC and if it was, return 30 and otherwise return 20... so it might be more appropriate to use 20 instead of 0. It doesn't appear to make a difference for the sake of behavior.</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;">Ran basic call from a PJSIP peer to an IAX peer with the following:
[general]
; The important parts
jitterbuffer=yes
forcejitterbuffer=yes
[deskbox]
type=friend
requirecalltoken = no
username = deskbox
secret = secret
host = dynamic
transfer = no
dtmfmode = auto
encryption = no
qualify = 300
context = default
disallow=all
allow=ulaw
allow=alaw
; Most of this is probably unnecessary for reproduction
Without the patch this would crash in 13 but work fine in 12.
With the patch, behavior strongly resembles 12 with the initial call into __get_from_jb attempting to jb_get and getting JB_OK back and then later, when the call was actually answered, the voice format would change and the function would call again with the proper format and the jitterbuffer would get started properly.</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/channels/chan_iax2.c <span style="color: grey">(423149)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3999/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>