<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/2603/">https://reviewboard.asterisk.org/r/2603/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 20th, 2013, 1:45 p.m. CDT, <b>opticron</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/2603/diff/1/?file=39555#file39555line1844" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/rtp_engine.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">1837</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="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">json_report_block</span><span class="p">)</span> <span class="p">{</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">1838</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="k">return</span><span class="p">;</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">1839</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="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;">This is unnecessary since ast_json_array_append is NULL-safe.</pre>
</blockquote>
<p>On June 21st, 2013, 7:16 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;">Yes, but I don't want to publish half a message. If you get a memory allocation error, publishing half a message means everyone who extracts the message will have to expect that the data may be missing.
If you can't build a message, you need to bail.</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;">Actually, the append will fail gracefully with -1 if provided a NULL value to append.</pre>
<br />
<p>- opticron</p>
<br />
<p>On June 11th, 2013, 9:57 p.m. CDT, Matt Jordan 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.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated June 11, 2013, 9:57 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-20754">ASTERISK-20754</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-21471">ASTERISK-21471</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 patch does the following:
* It merges Jaco Kroon's patch from ASTERISK-20754, which provides channel information in the RTCP events. Because Stasis provides a cache, Jaco's patch was modified to pass the channel uniqueid to the RTP layer as opposed to a pointer to the channel. This has the following benefits:
(1) It keeps the RTP engine 'clean' of references back to channels
(2) It prevents circular dependencies and other potential ref counting issues
* The RTP engine now allows any RTP implementation to raise RTCP messages. Potentially, other implementations (such as res_rtp_multicast) could also raise RTCP information. The engine provides structs to represent RTCP headers and RTCP SR/RR reports.
* Some general refactoring in res_rtp_asterisk to done to try and tame the RTCP code. It isn't perfect - that's *way* beyond the scope of this work - but it does feel marginally better.
* A few random bugs were fixed in the RTCP statistics. (Example: performing an assignment of a = a is probably not correct)
* We now raise RTCP events for each SR/RR sent/received. Previously we wouldn't raise an event when we sent a RR report.
Note that this work will be of use to others who want to monitor call quality or build modules that report call quality statistics. Since the events are now moving across the Stasis message bus, this is far easier to accomplish. It is also a first step (though by no means the last step) towards getting Olle's pinefrog work incorporated.</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;">Lots. A lot of wiresharking.
Additionally, testing was done to ensure that the AMI event payloads matched the RTCP packets sent between instances of Asterisk.</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/channels/chan_sip.c <span style="color: grey">(391524)</span></li>
<li>/trunk/channels/chan_skinny.c <span style="color: grey">(391524)</span></li>
<li>/trunk/channels/chan_unistim.c <span style="color: grey">(391524)</span></li>
<li>/trunk/include/asterisk/cdr.h <span style="color: grey">(391524)</span></li>
<li>/trunk/include/asterisk/channel.h <span style="color: grey">(391524)</span></li>
<li>/trunk/include/asterisk/json.h <span style="color: grey">(391524)</span></li>
<li>/trunk/include/asterisk/rtp_engine.h <span style="color: grey">(391524)</span></li>
<li>/trunk/main/asterisk.c <span style="color: grey">(391524)</span></li>
<li>/trunk/main/json.c <span style="color: grey">(391524)</span></li>
<li>/trunk/main/manager.c <span style="color: grey">(391524)</span></li>
<li>/trunk/main/rtp_engine.c <span style="color: grey">(391524)</span></li>
<li>/trunk/res/res_rtp_asterisk.c <span style="color: grey">(391524)</span></li>
<li>/trunk/channels/chan_multicast_rtp.c <span style="color: grey">(391524)</span></li>
<li>/trunk/channels/chan_motif.c <span style="color: grey">(391524)</span></li>
<li>/trunk/channels/chan_mgcp.c <span style="color: grey">(391524)</span></li>
<li>/trunk/channels/chan_jingle.c <span style="color: grey">(391524)</span></li>
<li>/trunk/channels/chan_h323.c <span style="color: grey">(391524)</span></li>
<li>/trunk/channels/chan_gulp.c <span style="color: grey">(391524)</span></li>
<li>/trunk/channels/chan_gtalk.c <span style="color: grey">(391524)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2603/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>