[prev in list] [next in list] [prev in thread] [next in thread]
List: asterisk-dev
Subject: Re: [asterisk-dev] [Code Review] 3810: res_hep_rtcp: Add module that sends RTCP information to a Hom
From: "Matt Jordan" <reviewboard () asterisk ! org>
Date: 2014-07-31 11:58:06
Message-ID: 20140731115806.27497.17156 () sonic ! digium ! api
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3810/
-----------------------------------------------------------
(Updated July 31, 2014, 6:58 a.m.)
Status
------
This change has been marked as submitted.
Review request for Asterisk Developers.
Repository: Asterisk
Description
-------
This patch adds a new module to Asterisk, res_hep_rtcp. The module subscribes to \
Stasis and receives RTCP information back from the message bus, which it encodes into \
HEPv3 packets and sends to the res_hep module for transmission.
Using this, someone with a Homer server can get live call quality monitoring for all \
channels in their Asterisk 12+ systems.
NOTE:
There were a few bugs uncovered by the tests written for the Asterisk Test Suite. As \
it turned out, these bugs were actually all in modules other than res_hep_rtcp, but \
I've included them with this diff as they are relatively small. 1) chan_pjsip failed \
to set its channel unique ids on its RTP instance on outbound calls. It now does this \
in the appropriate location, in the serialized call callback. 2) The rtp_engine was \
overflowing some values when packed into JSON. Specifically, some longs and unsigned \
ints can't be be packed into integer values, for obvious reasons. Since libjansson \
only supports integers, floats, strings, booleans, and objects, we print these values \
into strings. 3) res_rtp_asterisk had a few problems:
(a) it would emit a source IP address of 0.0.0.0 if bound to that IP address. We \
now use ast_find_ourip to get a better IP address, and properly marshal the result \
into an ast_strdupa'd string. (b) Reports can be generated with no report bodies. In \
particular, this occurs when a sender is transmitting information to a receiver (who \
will send no RTP back to the sender). As such, the sender has no report body for what \
it received. We now properly handle this case, and the sender will emit SR reports \
with no body. Likewise, if we receive an RTCP packet with no report body, we will \
still generate the appropriate events.
Diffs
-----
/branches/12/res/res_rtp_asterisk.c 419680
/branches/12/res/res_hep_rtcp.c PRE-CREATION
/branches/12/main/rtp_engine.c 419680
/branches/12/channels/chan_pjsip.c 419680
/branches/12/CHANGES 419680
Diff: https://reviewboard.asterisk.org/r/3810/diff/
Testing
-------
Some manual testing has be done, and automated tests have been written that exercise \
two scenarios:
* One where both sides transmit RTP information to each other (rtcp-sender)
* One where one side transmits RTP information, and the other is a passive receiver \
(rtcp-receiver)
See https://reviewboard.asterisk.org/r/3863
As a side note, Alexander actually demo'd this at Kamailio World - you can see it on \
the 'dangerous demos' here - http://www.youtube.com/watch?v=ykBdOTCCSHs
Thanks,
Matt Jordan
[Attachment #5 (text/html)]
<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/3810/">https://reviewboard.asterisk.org/r/3810/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#e0e0e0" width="100%" cellpadding="8" style="border: 1px gray \
solid;"> <tr>
<td>
<h1 style="margin-right: 0.2em; padding: 0; font-size: 10pt;">This change has been \
marked as submitted.</h1> </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.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated July 31, 2014, 6:58 a.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;">This patch adds a new module to Asterisk, res_hep_rtcp. The module \
subscribes to Stasis and receives RTCP information back from the message bus, which \
it encodes into HEPv3 packets and sends to the res_hep module for transmission.
Using this, someone with a Homer server can get live call quality monitoring for all \
channels in their Asterisk 12+ systems.
NOTE:
There were a few bugs uncovered by the tests written for the Asterisk Test Suite. As \
it turned out, these bugs were actually all in modules other than res_hep_rtcp, but \
I've included them with this diff as they are relatively small. 1) chan_pjsip \
failed to set its channel unique ids on its RTP instance on outbound calls. It now \
does this in the appropriate location, in the serialized call callback. 2) The \
rtp_engine was overflowing some values when packed into JSON. Specifically, some \
longs and unsigned ints can't be be packed into integer values, for obvious \
reasons. Since libjansson only supports integers, floats, strings, booleans, and \
objects, we print these values into strings. 3) res_rtp_asterisk had a few problems:
(a) it would emit a source IP address of 0.0.0.0 if bound to that IP address. We \
now use ast_find_ourip to get a better IP address, and properly marshal the result \
into an ast_strdupa'd string. (b) Reports can be generated with no report \
bodies. In particular, this occurs when a sender is transmitting information to a \
receiver (who will send no RTP back to the sender). As such, the sender has no report \
body for what it received. We now properly handle this case, and the sender will emit \
SR reports with no body. Likewise, if we receive an RTCP packet with no report body, \
we will still generate the appropriate events.</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;">Some manual testing has be done, and automated tests have been written \
that exercise two scenarios:
* One where both sides transmit RTP information to each other (rtcp-sender)
* One where one side transmits RTP information, and the other is a passive receiver \
(rtcp-receiver)
See https://reviewboard.asterisk.org/r/3863
As a side note, Alexander actually demo'd this at Kamailio World - you can see it \
on the 'dangerous demos' here - http://www.youtube.com/watch?v=ykBdOTCCSHs
</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_rtp_asterisk.c <span style="color: \
grey">(419680)</span></li>
<li>/branches/12/res/res_hep_rtcp.c <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>/branches/12/main/rtp_engine.c <span style="color: grey">(419680)</span></li>
<li>/branches/12/channels/chan_pjsip.c <span style="color: \
grey">(419680)</span></li>
<li>/branches/12/CHANGES <span style="color: grey">(419680)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3810/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic