[Asterisk-code-review] res_rtp_asterisk.c: Send RTCP as compound packets. (...asterisk[16])
Benjamin Keith Ford
asteriskteam at digium.com
Tue Sep 10 15:32:11 CDT 2019
Benjamin Keith Ford has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/12863
Change subject: res_rtp_asterisk.c: Send RTCP as compound packets.
......................................................................
res_rtp_asterisk.c: Send RTCP as compound packets.
According to RFC3550, ALL RTCP packets must be sent in a compond packet
of at least two individual packets, including SR/RR and SDES. REMB,
FIR, and NACK were not following this format, and as a result, would
fail the packet check in ast_rtcp_interpret. This was found from writing
unit tests for RTCP. The browser would accept the way we were
constructing these RTCP packets, but when sending directly from one
Asterisk instance to another, the above mentioned problem would occur.
Change-Id: Ieb140e9c22568a251a564cd953dd22cd33244605
---
M res/res_rtp_asterisk.c
1 file changed, 84 insertions(+), 22 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/63/12863/1
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 08138cb..2f20a83 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -215,7 +215,6 @@
static struct ast_ha *stun_blacklist = NULL;
static ast_rwlock_t stun_blacklist_lock = AST_RWLOCK_INIT_VALUE;
-
/*! \brief Pool factory used by pjlib to allocate memory. */
static pj_caching_pool cachingpool;
@@ -4842,11 +4841,16 @@
static void rtp_write_rtcp_fir(struct ast_rtp_instance *instance, struct ast_rtp *rtp, struct ast_sockaddr *remote_address)
{
- unsigned int *rtcpheader;
- char bdata[1024];
- int len = 20;
+ unsigned char *rtcpheader;
+ unsigned char bdata[1024];
+ int packet_len = 0;
+ int fir_len = 20;
int ice;
int res;
+ int sr;
+ RAII_VAR(struct ast_rtp_rtcp_report *, rtcp_report,
+ ast_rtp_rtcp_report_alloc(rtp->themssrc_valid ? 1 : 0),
+ ao2_cleanup);
if (!rtp || !rtp->rtcp) {
return;
@@ -4870,13 +4874,35 @@
rtp->rtcp->firseq = 0;
}
- rtcpheader = (unsigned int *)bdata;
- rtcpheader[0] = htonl((2 << 30) | (4 << 24) | (RTCP_PT_PSFB << 16) | ((len/4)-1));
- rtcpheader[1] = htonl(rtp->ssrc);
- rtcpheader[2] = htonl(rtp->themssrc);
- rtcpheader[3] = htonl(rtp->themssrc); /* FCI: SSRC */
- rtcpheader[4] = htonl(rtp->rtcp->firseq << 24); /* FCI: Sequence number */
- res = rtcp_sendto(instance, (unsigned int *)rtcpheader, len, 0, rtp->bundled ? remote_address : &rtp->rtcp->them, &ice);
+ ao2_lock(instance);
+ rtcpheader = bdata;
+
+ res = ast_rtcp_generate_report(instance, rtcpheader, rtcp_report, &sr);
+
+ if (res == 0 || res == 1) {
+ ao2_unlock(instance);
+ ast_debug(1, "Failed to add %s report to RTCP FIR packet!\n", sr ? "SR" : "RR");
+ return;
+ }
+
+ packet_len += res;
+
+ res = ast_rtcp_generate_sdes(instance, rtcpheader + packet_len, rtcp_report);
+
+ if (res == 0 || res == 1) {
+ ao2_unlock(instance);
+ ast_debug(1, "Failed to add SDES to RTCP FIR packet!\n");
+ return;
+ }
+
+ packet_len += res;
+
+ put_unaligned_uint32(rtcpheader + packet_len + 0, htonl((2 << 30) | (4 << 24) | (RTCP_PT_PSFB << 16) | ((fir_len/4)-1)));
+ put_unaligned_uint32(rtcpheader + packet_len + 4, htonl(rtp->ssrc));
+ put_unaligned_uint32(rtcpheader + packet_len + 8, htonl(rtp->themssrc));
+ put_unaligned_uint32(rtcpheader + packet_len + 12, htonl(rtp->themssrc)); /* FCI: SSRC */
+ put_unaligned_uint32(rtcpheader + packet_len + 16, htonl(rtp->rtcp->firseq << 24)); /* FCI: Sequence number */
+ res = rtcp_sendto(instance, (unsigned int *)rtcpheader, packet_len + fir_len, 0, rtp->bundled ? remote_address : &rtp->rtcp->them, &ice);
if (res < 0) {
ast_log(LOG_ERROR, "RTCP FIR transmission error: %s\n", strerror(errno));
}
@@ -4885,11 +4911,16 @@
static void rtp_write_rtcp_psfb(struct ast_rtp_instance *instance, struct ast_rtp *rtp, struct ast_frame *frame, struct ast_sockaddr *remote_address)
{
struct ast_rtp_rtcp_feedback *feedback = frame->data.ptr;
- unsigned int *rtcpheader;
- char bdata[1024];
- int len = 24;
+ unsigned char *rtcpheader;
+ unsigned char bdata[1024];
+ int remb_len = 24;
int ice;
int res;
+ int sr = 0;
+ int packet_len = 0;
+ RAII_VAR(struct ast_rtp_rtcp_report *, rtcp_report,
+ ast_rtp_rtcp_report_alloc(rtp->themssrc_valid ? 1 : 0),
+ ao2_cleanup);
if (feedback->fmt != AST_RTP_RTCP_FMT_REMB) {
ast_debug(1, "Provided an RTCP feedback frame of format %d to write on RTP instance '%p' but only REMB is supported\n",
@@ -4915,16 +4946,38 @@
return;
}
- rtcpheader = (unsigned int *)bdata;
- rtcpheader[0] = htonl((2 << 30) | (AST_RTP_RTCP_FMT_REMB << 24) | (RTCP_PT_PSFB << 16) | ((len/4)-1));
- rtcpheader[1] = htonl(rtp->ssrc);
- rtcpheader[2] = htonl(0); /* Per the draft this should always be 0 */
- rtcpheader[3] = htonl(('R' << 24) | ('E' << 16) | ('M' << 8) | ('B')); /* Unique identifier 'R' 'E' 'M' 'B' */
- rtcpheader[4] = htonl((1 << 24) | (feedback->remb.br_exp << 18) | (feedback->remb.br_mantissa)); /* Number of SSRCs / BR Exp / BR Mantissa */
- rtcpheader[5] = htonl(rtp->ssrc); /* The SSRC this feedback message applies to */
- res = rtcp_sendto(instance, (unsigned int *)rtcpheader, len, 0, rtp->bundled ? remote_address : &rtp->rtcp->them, &ice);
+ ao2_lock(instance);
+ rtcpheader = bdata;
+
+ res = ast_rtcp_generate_report(instance, rtcpheader, rtcp_report, &sr);
+ if (res == 0 || res == 1) {
+ ast_debug(1, "Failed to add %s report to REMB packet!\n", sr ? "SR" : "RR");
+ ao2_unlock(instance);
+ return;
+ }
+
+ packet_len += res;
+
+ res = ast_rtcp_generate_sdes(instance, rtcpheader + packet_len, rtcp_report);
+ if (res == 0 || res == 1) {
+ ast_debug(1, "Failed to add SDES to REMB packet!\n");
+ ao2_unlock(instance);
+ return;
+ }
+
+ packet_len += res;
+
+ put_unaligned_uint32(rtcpheader + packet_len + 0, htonl((2 << 30) | (AST_RTP_RTCP_FMT_REMB << 24) | (RTCP_PT_PSFB << 16) | ((remb_len/4)-1)));
+ put_unaligned_uint32(rtcpheader + packet_len + 4, htonl(rtp->ssrc));
+ put_unaligned_uint32(rtcpheader + packet_len + 8, htonl(0)); /* Per the draft, this should always be 0 */
+ put_unaligned_uint32(rtcpheader + packet_len + 12, htonl(('R' << 24) | ('E' << 16) | ('M' << 8) | ('B'))); /* Unique identifier 'R' 'E' 'M' 'B' */
+ put_unaligned_uint32(rtcpheader + packet_len + 16, htonl((1 << 24) | (feedback->remb.br_exp << 18) | (feedback->remb.br_mantissa))); /* Number of SSRCs / BR Exp / BR Mantissa */
+ put_unaligned_uint32(rtcpheader + packet_len + 20, htonl(rtp->ssrc)); /* The SSRC this feedback message applies to */
+ res = rtcp_sendto(instance, (unsigned int *)rtcpheader, packet_len + remb_len, 0, rtp->bundled ? remote_address : &rtp->rtcp->them, &ice);
if (res < 0) {
ast_log(LOG_ERROR, "RTCP PSFB transmission error: %s\n", strerror(errno));
+ } else {
+ ast_rtcp_calculate_sr_rr_statistics(instance, rtcp_report, rtp->bundled ? *remote_address : rtp->rtcp->them, ice, sr);
}
}
@@ -7643,6 +7696,15 @@
packet_len += res;
+ res = ast_rtcp_generate_sdes(instance, rtcpheader + packet_len, rtcp_report);
+
+ if (res == 0 || res == 1) {
+ ast_debug(1, "Failed to add SDES to NACK, stopping here\n");
+ return &ast_null_frame;
+ }
+
+ packet_len += res;
+
res = ast_rtcp_generate_nack(instance, rtcpheader + packet_len);
if (res == 0) {
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/12863
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ieb140e9c22568a251a564cd953dd22cd33244605
Gerrit-Change-Number: 12863
Gerrit-PatchSet: 1
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190910/0b8b153b/attachment-0001.html>
More information about the asterisk-code-review
mailing list