<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/3982/">https://reviewboard.asterisk.org/r/3982/</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 8th, 2014, 1:35 p.m. CDT, <b>Matt Jordan</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/3982/diff/1/?file=67291#file67291line850" style="color: black; font-weight: bold; text-decoration: underline;">/branches/13/res/res_rtp_asterisk.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">846</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="p">(</span><span class="kt">int</span><span class="p">)</span> <span class="n">status</span><span class="p">,</span> <span class="n">buf</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;">Nitpick: no space between (int) and status.</pre>
</blockquote>
<p>On September 8th, 2014, 2:26 p.m. CDT, <b>rmudgett</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 prefer
(int) status,
over
(int)status,</pre>
</blockquote>
<p>On September 8th, 2014, 3: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;">Generally, the coding guidelines would suggest that you not do that however:
{quote}
-nprs: no space after parentheses
{quote}
I'll grant nothing calls that out specifically with casting, and the convention in the code is all over the map on this one. I'd guess based on some quick grepping that we tend to use (int)foo more than (int) foo, but that's just a quick gut check.</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;">That option refers to spaces between parentheses "if ( x )" vs "if (x)" and "func( x )" vs "func(x)".
{quote}
-nprs, --no-space-after-parentheses
Do not put a space after every ’(’ and before every ’)’.
See STATEMENTS.
{quote}
This is the one referring to casts and it says to put one there.
{quote}
-cs, --space-after-cast
Put a space after a cast operator.
See STATEMENTS.
{quote}
But this is getting into fine hair splitting. :)</pre>
<br />
<p>- rmudgett</p>
<br />
<p>On September 7th, 2014, 10:07 a.m. CDT, Joshua Colp 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 Joshua Colp.</div>
<p style="color: grey;"><i>Updated Sept. 7, 2014, 10:07 a.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-23577">ASTERISK-23577</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-23634">ASTERISK-23634</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;">The TURN support in res_rtp_asterisk has been exercised by only a few, which has uncovered a slew of issues.
1. The number of file descriptors that an ioqueue instance in pjlib can support is a fixed number. Exceeding this causes an assertion. The code will now dynamically create/terminate threads as needed to ensure that this limit is not exceeded on ioqueue instances.
2. The API did not allow users of the TURN code to explicitly request a TURN session with details. This has been added.
3. The ICE code has a fixed size array of 4 for transports. As our transport identifiers started at 1 we were exceeding this causing an assertion. Our identifiers now start at 0.
4. The TURN client did not set up client binding causing needless bandwidth usage. Upon ICE completion if TURN is used channel binding is now established.
5. The code will no longer update address information on every sent packet. The remote address is now updated only once upon ICE completion to the target address.
6. When relaying was in use STUN traffic was getting looped back to res_rtp_asterisk due to it being handled on the normal socket. It is now handled in the TURN session callback instead.
7. Logging when a TURN relay is in use now uses the IP address that the TURN relay is sending/receiving to/from on our behalf.
8. Synchronization between the TURN session and the RTP instance now ensures that the session has been fully created or fully destroyed before continuing.
9. Some cleanup has occurred when tearing down the pj environment.</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;">Sabotaged the code so the only candidates I sent were of the relay type. Confirmed bidirectional media flow using the TURN relay.</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/res/res_rtp_asterisk.c <span style="color: grey">(422835)</span></li>
<li>/branches/13/include/asterisk/rtp_engine.h <span style="color: grey">(422835)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3982/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>