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