<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/3747/">https://reviewboard.asterisk.org/r/3747/</a>
</td>
</tr>
</table>
<br />
<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, Corey Farrell, Joshua Colp, Matt Jordan, and rmudgett.</div>
<div>By Jonathan Rose.</div>
<p style="color: grey;"><i>Updated July 11, 2014, 3:59 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</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;">Culling for preference order not in bitfield. Move framing along with order in codec_pref_remove.</pre>
</td>
</tr>
</table>
<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-23958">ASTERISK-23958</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;">Hmmm, where to begin...
The media formats branch currently has a lot of channels in a less than usable state. This patch brings chan_iax2 back to being usable, but there are still problems remaining (right now I'm working on codec negotiation).
First, chan_iax2 wouldn't build because ast_codec_pref_convert wasn't defined. After fixing that function to get it to work again, I realized that codecs weren't being set for IAX peers/users because the iax2_parse_allow_disallow function wasn't actually saving the formats between passes, just the format preferences. Also the preferences would get out of order because every time it entered a new allow/disallow line, the preferences would be converted into an ast_format_cap without order information preserved and then re-added. The ast_codec_pref_append function wasn't working how I expected it to and would sometimes double up the same pref or delete in a semi-random way. Looking into that the ast_codec_remove function wasn't deleting existing codecs in a way that could be considered proper. That function got mostly rewritten and I think it's a good bit simpler now.
Some functions were abusing formats pointers declared on the stack and treating them as if they were pointing at alloc'd format structs. I ended up rewriting the ast_codec_pref_string function as well since it was doing some of that format abuse.</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;">Configured two IAX peers
[lappy]
type=friend
requirecalltoken = no
username = lappy
secret = secret
host = dynamic
dtmfmode = auto
transfer = no
encryption = no
qualify = 300
disallow = all
allow=ulaw
allow=gsm
allow=alaw
context = default
[deskbox]
type=friend
requirecalltoken = no
username = deskbox
secret = secret
host = dynamic
transfer = no
dtmfmode = auto
encryption = no
qualify = 300
disallow = all
allow=ulaw
allow=alaw
allow=gsm
context = default
Made sure they were configured in a way that the prefs order values were the same as in current builds of Asterisk 12. Speaking of which, I'm not sure why the arrays were changed from integers to uint64_t... that indicates to me that the idea might have been to store the bitflag representations of the formats in these structs, but little was changed to account for that difference, so I'm not really sure. As a result I had to make a few functions for going back and forth between the format indices that chan_iax2 was using in the order values and the format bitfield representations that can be used to add to ast_format_cap structs.
Made sure the output for ast_codec_pref_string would have the same order as allow/disallow options specified. Made sure that if the buffer for ast_codec_pref_string was less than what was needed to write the full string that it would be terminated early in a predictable way.
Made sure the ast_codec_pref_convert function wrote the same strings to buf when right=1 as they did in Asterisk 12. Still haven't verified how it works in right=0 mode.
Made sure that I could make a call from one IAX softphone on my laptop to an IAX softphone on my desktop. Received audio going both ways. Codec preferences didn't have an effect unfortunately while they did in twelve. Instead, all audio chosen was GSM, presumably because it has the lowest format value. I'm still looking into that.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/team/group/media_formats-reviewed-trunk/main/format_compatibility.c <span style="color: grey">(418208)</span></li>
<li>/team/group/media_formats-reviewed-trunk/include/asterisk/format_compatibility.h <span style="color: grey">(418208)</span></li>
<li>/team/group/media_formats-reviewed-trunk/channels/iax2/parser.c <span style="color: grey">(418208)</span></li>
<li>/team/group/media_formats-reviewed-trunk/channels/chan_iax2.c <span style="color: grey">(418208)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3747/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>