<p>George Joseph <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/12863">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Kevin Harwell: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved; Approved for Submit
</div><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, 98 insertions(+), 35 deletions(-)<br><br></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..adda039 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>@@ -4433,6 +4432,41 @@</span><br><span> return len;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* Lock instance before calling this if it isn't already</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * If successful, the overall packet length is returned</span><br><span style="color: hsl(120, 100%, 40%);">+ * If not, then 0 is returned</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+static int ast_rtcp_generate_compound_prefix(struct ast_rtp_instance *instance, unsigned char *rtcpheader,</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_rtp_rtcp_report *report, int *sr)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ int packet_len = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ int res;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Every RTCP packet needs to be sent out with a SR/RR and SDES prefixing it.</span><br><span style="color: hsl(120, 100%, 40%);">+ * At the end of this function, rtcpheader should contain both of those packets,</span><br><span style="color: hsl(120, 100%, 40%);">+ * and will return the length of the overall packet. This can be used to determine</span><br><span style="color: hsl(120, 100%, 40%);">+ * where further packets can be inserted in the compound packet.</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, 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%);">+ ast_debug(1, "Failed to generate %s report!\n", sr ? "SR" : "RR");</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</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, 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 generate SDES!\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</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%);">+ return packet_len + res;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static int ast_rtcp_generate_nack(struct ast_rtp_instance *instance, unsigned char *rtcpheader)</span><br><span> {</span><br><span> struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);</span><br><span>@@ -4523,19 +4557,9 @@</span><br><span> ao2_lock(instance);</span><br><span> rtcpheader = bdata;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- res = ast_rtcp_generate_report(instance, rtcpheader, rtcp_report, &sr);</span><br><span style="color: hsl(120, 100%, 40%);">+ res = ast_rtcp_generate_compound_prefix(instance, rtcpheader, rtcp_report, &sr);</span><br><span> </span><br><span> if (res == 0 || res == 1) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_debug(1, "Failed to add %s report to RTCP packet!\n", sr ? "SR" : "RR");</span><br><span style="color: hsl(0, 100%, 40%);">- goto cleanup;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- packet_len += res;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- res = ast_rtcp_generate_sdes(instance, rtcpheader + packet_len, rtcp_report);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (res == 0 || res == 1) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_debug(1, "Failed to add SDES to RTCP packet!\n");</span><br><span> goto cleanup;</span><br><span> }</span><br><span> </span><br><span>@@ -4842,11 +4866,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,26 +4899,46 @@</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%);">+ rtcpheader = bdata;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_lock(instance);</span><br><span style="color: hsl(120, 100%, 40%);">+ res = ast_rtcp_generate_compound_prefix(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%);">+ 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 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 style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_unlock(instance);</span><br><span> }</span><br><span> </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,17 +4964,32 @@</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%);">+ rtcpheader = bdata;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_lock(instance);</span><br><span style="color: hsl(120, 100%, 40%);">+ res = ast_rtcp_generate_compound_prefix(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%);">+ 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 style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_unlock(instance);</span><br><span> }</span><br><span> </span><br><span> /*! \pre instance is locked */</span><br><span>@@ -7634,10 +7698,9 @@</span><br><span> </span><br><span> memset(rtcpheader, 0, data_size);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- res = ast_rtcp_generate_report(instance, rtcpheader, rtcp_report, &sr);</span><br><span style="color: hsl(120, 100%, 40%);">+ res = ast_rtcp_generate_compound_prefix(instance, rtcpheader, rtcp_report, &sr);</span><br><span> </span><br><span> if (res == 0 || res == 1) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_debug(1, "Failed to add %s report to NACK, stopping here\n", sr ? "SR" : "RR");</span><br><span> return &ast_null_frame;</span><br><span> }</span><br><span> </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: 4 </div>
<div style="display:none"> Gerrit-Owner: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>