<p>Benjamin Keith Ford has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/8774">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_rtp_asterisk: Add support for receiving and handling NACK requests.<br><br>Adds the ability to receive and handle incoming NACK requests if<br>retransmissions are enabled. If retransmissions are enabled, a data<br>buffer is allocated that stores packets being sent. If a NACK request<br>is received, the packet requested for retransmission is sent if it is<br>still in the buffer. In the same request, if any of the following 16<br>packets are marked as not received, those will be sent as well if<br>available, as outlined in RFC4585.<br><br>Also changes RTCP RR and SR to use media source SSRC instead of packet<br>source SSRC when determining which instance to use for RTCP reports.<br><br>For more information, refer to the wiki page:<br>https://wiki.asterisk.org/wiki/display/AST/WebRTC+User+Experience+Improvements<br><br>Change-Id: I7f7f124af3b9d5d2fd9cffc6ba8cb48a6fff06ec<br>---<br>M include/asterisk/rtp_engine.h<br>M res/res_pjsip_sdp_rtp.c<br>M res/res_rtp_asterisk.c<br>3 files changed, 155 insertions(+), 10 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/74/8774/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h<br>index 8f044ce..3426b2a 100644<br>--- a/include/asterisk/rtp_engine.h<br>+++ b/include/asterisk/rtp_engine.h<br>@@ -292,10 +292,14 @@<br> #define AST_RTP_RTCP_SR 200<br> /*! Receiver Report */<br> #define AST_RTP_RTCP_RR 201<br>+/*! Transport Layer Feed Back (From RFC4585 also RFC5104) */<br>+#define AST_RTP_RTCP_RTPFB 205<br> /*! Payload Specific Feed Back (From RFC4585 also RFC5104) */<br>-#define AST_RTP_RTCP_PSFB 206<br>+#define AST_RTP_RTCP_PSFB 206<br> <br> /* Common RTCP feedback message types */<br>+/*! Generic NACK (From RFC4585 also RFC5104) */<br>+#define AST_RTP_RTCP_FMT_NACK 1<br> /*! Picture loss indication (From RFC4585) */<br> #define AST_RTP_RTCP_FMT_PLI 1<br> /*! Full INTRA-frame Request (From RFC5104) */<br>diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c<br>index 03d3765..14ed3b1 100644<br>--- a/res/res_pjsip_sdp_rtp.c<br>+++ b/res/res_pjsip_sdp_rtp.c<br>@@ -1096,6 +1096,9 @@<br> <br> attr = pjmedia_sdp_attr_create(pool, "rtcp-fb", pj_cstr(&stmp, "* goog-remb"));<br> pjmedia_sdp_attr_add(&media->attr_count, media->attr, attr);<br>+<br>+ attr = pjmedia_sdp_attr_create(pool, "rtcp-fb", pj_cstr(&stmp, "* nack"));<br>+ pjmedia_sdp_attr_add(&media->attr_count, media->attr, attr);<br> }<br> <br> /*! \brief Function which negotiates an incoming media stream */<br>diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c<br>index c889037..61cd883 100644<br>--- a/res/res_rtp_asterisk.c<br>+++ b/res/res_rtp_asterisk.c<br>@@ -71,6 +71,7 @@<br> #include "asterisk/smoother.h"<br> #include "asterisk/uuid.h"<br> #include "asterisk/test.h"<br>+#include "asterisk/data_buffer.h"<br> #ifdef HAVE_PJPROJECT<br> #include "asterisk/res_pjproject.h"<br> #endif<br>@@ -92,6 +93,8 @@<br> <br> #define TURN_STATE_WAIT_TIME 2000<br> <br>+#define DEFAULT_RTP_BUFFER_SIZE 100<br>+<br> /*! Full INTRA-frame Request / Fast Update Request (From RFC2032) */<br> #define RTCP_PT_FUR 192<br> /*! Sender Report (From RFC3550) */<br>@@ -105,6 +108,8 @@<br> /*! Application defined (From RFC3550) */<br> #define RTCP_PT_APP 204<br> /* VP8: RTCP Feedback */<br>+/*! Transport Layer Feed Back (From RFC4585 also RFC5104) */<br>+#define RTCP_PT_RTPFB AST_RTP_RTCP_RTPFB<br> /*! Payload Specific Feed Back (From RFC4585 also RFC5104) */<br> #define RTCP_PT_PSFB AST_RTP_RTCP_PSFB<br> <br>@@ -373,6 +378,8 @@<br> <br> struct rtp_red *red;<br> <br>+ struct ast_data_buffer *send_buffer; /*!< Buffer for storing sent packets for retransmission */<br>+<br> #ifdef HAVE_PJPROJECT<br> ast_cond_t cond; /*!< ICE/TURN condition for signaling */<br> <br>@@ -507,6 +514,11 @@<br> unsigned char buf_data[64000]; /*!< buffered primary data */<br> int hdrlen;<br> long int prev_ts;<br>+};<br>+<br>+struct ast_rtp_rtcp_nack_payload {<br>+ unsigned char buf[0];<br>+ size_t size;<br> };<br> <br> AST_LIST_HEAD_NOLOCK(frame_list, ast_frame);<br>@@ -3675,6 +3687,11 @@<br> rtp->red = NULL;<br> }<br> <br>+ /* Destroy the send buffer if it was being used */<br>+ if (rtp->send_buffer) {<br>+ ast_data_buffer_free(rtp->send_buffer);<br>+ }<br>+<br> ao2_cleanup(rtp->lasttxformat);<br> ao2_cleanup(rtp->lastrxformat);<br> ao2_cleanup(rtp->f.subclass.format);<br>@@ -4369,7 +4386,7 @@<br> } else {<br> /* This is the first frame with sequence number we've seen, so start keeping track */<br> rtp->expectedseqno = frame->seqno + 1;<br>- }<br>+ }<br> } else {<br> rtp->expectedseqno = -1;<br> }<br>@@ -4383,13 +4400,25 @@<br> /* If we know the remote address construct a packet and send it out */<br> if (!ast_sockaddr_isnull(&remote_address)) {<br> int hdrlen = 12, res, ice;<br>+ int packet_len = frame->datalen + hdrlen;<br> unsigned char *rtpheader = (unsigned char *)(frame->data.ptr - hdrlen);<br> <br> put_unaligned_uint32(rtpheader, htonl((2 << 30) | (codec << 16) | (seqno) | (mark << 23)));<br> put_unaligned_uint32(rtpheader + 4, htonl(rtp->lastts));<br> put_unaligned_uint32(rtpheader + 8, htonl(rtp->ssrc));<br> <br>- if ((res = rtp_sendto(instance, (void *)rtpheader, frame->datalen + hdrlen, 0, &remote_address, &ice)) < 0) {<br>+ /* If retransmissions are enabled, we need to store this packet for future use */<br>+ if (ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_RETRANS_SEND) && rtp->send_buffer) {<br>+ struct ast_rtp_rtcp_nack_payload *payload;<br>+<br>+ if ((payload = ast_calloc(1, sizeof(*payload) + packet_len))) {<br>+ payload->size = packet_len;<br>+ memcpy(payload->buf, rtpheader, packet_len);<br>+ ast_data_buffer_put(rtp->send_buffer, rtp->seqno, payload);<br>+ }<br>+ }<br>+<br>+ if ((res = rtp_sendto(instance, (void *)rtpheader, packet_len, 0, &remote_address, &ice)) < 0) {<br> if (!ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_NAT) || (ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_NAT) && (ast_test_flag(rtp, FLAG_NAT_ACTIVE) == FLAG_NAT_ACTIVE))) {<br> ast_debug(1, "RTP Transmission error of packet %d to %s: %s\n",<br> rtp->seqno,<br>@@ -5145,8 +5174,8 @@<br> }<br> <br> /*! \pre instance is locked */<br>-static struct ast_rtp_instance *rtp_find_instance_by_ssrc(struct ast_rtp_instance *instance,<br>- struct ast_rtp *rtp, unsigned int ssrc)<br>+static struct ast_rtp_instance *__rtp_find_instance_by_ssrc(struct ast_rtp_instance *instance,<br>+ struct ast_rtp *rtp, unsigned int ssrc, int source)<br> {<br> int index;<br> <br>@@ -5158,8 +5187,9 @@<br> /* Find the bundled child instance */<br> for (index = 0; index < AST_VECTOR_SIZE(&rtp->ssrc_mapping); ++index) {<br> struct rtp_ssrc_mapping *mapping = AST_VECTOR_GET_ADDR(&rtp->ssrc_mapping, index);<br>+ unsigned int mapping_ssrc = source ? ast_rtp_get_ssrc(mapping->instance) : mapping->ssrc;<br> <br>- if (mapping->ssrc_valid && mapping->ssrc == ssrc) {<br>+ if (mapping->ssrc_valid && mapping_ssrc == ssrc) {<br> return mapping->instance;<br> }<br> }<br>@@ -5169,6 +5199,20 @@<br> return instance;<br> }<br> return NULL;<br>+}<br>+<br>+/*! \pre instance is locked */<br>+static struct ast_rtp_instance *rtp_find_instance_by_packet_source_ssrc(struct ast_rtp_instance *instance,<br>+ struct ast_rtp *rtp, unsigned int ssrc)<br>+{<br>+ return __rtp_find_instance_by_ssrc(instance, rtp, ssrc, 0);<br>+}<br>+<br>+/*! \pre instance is locked */<br>+static struct ast_rtp_instance *rtp_find_instance_by_media_source_ssrc(struct ast_rtp_instance *instance,<br>+ struct ast_rtp *rtp, unsigned int ssrc)<br>+{<br>+ return __rtp_find_instance_by_ssrc(instance, rtp, ssrc, 1);<br> }<br> <br> static const char *rtcp_payload_type2str(unsigned int pt)<br>@@ -5201,6 +5245,55 @@<br> break;<br> }<br> return str;<br>+}<br>+<br>+/*! \pre instance is locked */<br>+static int ast_rtp_rtcp_handle_nack(struct ast_rtp_instance *instance, unsigned int *nackdata, unsigned int length)<br>+{<br>+ struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);<br>+ int res = 0, j, i, ice;<br>+ struct ast_rtp_rtcp_nack_payload *payload;<br>+ unsigned int current_word;<br>+ unsigned int pid; /* Packet ID which refers to seqno of lost packet */<br>+ unsigned int blp; /* Bitmask of following lost packets */<br>+ struct ast_sockaddr remote_address = { {0,} };<br>+<br>+ if (!rtp->send_buffer) {<br>+ return res;<br>+ }<br>+<br>+ ast_rtp_instance_get_remote_address(instance, &remote_address);<br>+<br>+ for (j = 3; j < length; j++) {<br>+ current_word = ntohl(nackdata[j]);<br>+ pid = current_word >> 16;<br>+ /* We know the remote end is missing this packet. Go ahead and send it if we still have it. */<br>+ payload = (struct ast_rtp_rtcp_nack_payload *)ast_data_buffer_get(rtp->send_buffer, pid);<br>+ if (payload) {<br>+ res += rtp_sendto(instance, payload->buf, payload->size, 0, &remote_address, &ice);<br>+ }<br>+ /*<br>+ * The bitmask. Denoting the least significant bit as 1 and its most significant bit<br>+ * as 16, then bit i of the bitmask is set to 1 if the receiver has not received RTP<br>+ * packet (pid+i)(modulo 2^16). Otherwise, it is set to 0. We cannot assume bits set<br>+ * to 0 after a bit set to 1 have actually been received.<br>+ */<br>+ blp = current_word & 0xFF;<br>+ i = 1;<br>+ while (blp) {<br>+ if (blp & 1) {<br>+ /* Packet (pid + i)(modulo 2^16) is missing too. */<br>+ payload = (struct ast_rtp_rtcp_nack_payload *)ast_data_buffer_get(rtp->send_buffer, (pid + i) % 65536);<br>+ if (payload) {<br>+ res += rtp_sendto(instance, payload->buf, payload->size, 0, &remote_address, &ice);<br>+ }<br>+ }<br>+ blp >>= 1;<br>+ i++;<br>+ }<br>+ }<br>+<br>+ return res;<br> }<br> <br> /*<br>@@ -5243,6 +5336,7 @@<br> #define RTCP_RR_BLOCK_WORD_LENGTH 6<br> #define RTCP_HEADER_SSRC_LENGTH 2<br> #define RTCP_FB_REMB_BLOCK_WORD_LENGTH 4<br>+#define RTCP_FB_NACK_BLOCK_WORD_LENGTH 2<br> <br> static struct ast_frame *ast_rtcp_interpret(struct ast_rtp_instance *instance, const unsigned char *rtcpdata, size_t size, struct ast_sockaddr *addr)<br> {<br>@@ -5319,6 +5413,8 @@<br> unsigned int ssrc_valid;<br> unsigned int length;<br> unsigned int min_length;<br>+ /*! Always use packet source SSRC to find the rtp instance unless explicitly told not to. */<br>+ unsigned int use_packet_source = 1;<br> <br> struct ast_json *message_blob;<br> RAII_VAR(struct ast_rtp_rtcp_report *, rtcp_report, NULL, ao2_cleanup);<br>@@ -5341,8 +5437,19 @@<br> /* fall through */<br> case RTCP_PT_RR:<br> min_length += (rc * RTCP_RR_BLOCK_WORD_LENGTH);<br>+ use_packet_source = 0;<br> break;<br> case RTCP_PT_FUR:<br>+ break;<br>+ case RTCP_PT_RTPFB:<br>+ switch (rc) {<br>+ case AST_RTP_RTCP_FMT_NACK:<br>+ min_length += RTCP_FB_NACK_BLOCK_WORD_LENGTH;<br>+ break;<br>+ default:<br>+ break;<br>+ }<br>+ use_packet_source = 0;<br> break;<br> case RTCP_PT_PSFB:<br> switch (rc) {<br>@@ -5392,12 +5499,15 @@<br> }<br> rtcp_report->reception_report_count = rc;<br> <br>- ssrc = ntohl(rtcpheader[i + 1]);<br>+ ssrc = ntohl(rtcpheader[i + 2]);<br> rtcp_report->ssrc = ssrc;<br> break;<br> case RTCP_PT_FUR:<br> case RTCP_PT_PSFB:<br> ssrc = ntohl(rtcpheader[i + 1]);<br>+ break;<br>+ case RTCP_PT_RTPFB:<br>+ ssrc = ntohl(rtcpheader[i + 2]);<br> break;<br> case RTCP_PT_SDES:<br> case RTCP_PT_BYE:<br>@@ -5417,7 +5527,15 @@<br> <br> /* Determine the appropriate instance for this */<br> if (ssrc_valid) {<br>- child = rtp_find_instance_by_ssrc(transport, transport_rtp, ssrc);<br>+ /*<br>+ * Depending on the payload type, either the packet source or media source<br>+ * SSRC is used.<br>+ */<br>+ if (use_packet_source) {<br>+ child = rtp_find_instance_by_packet_source_ssrc(transport, transport_rtp, ssrc);<br>+ } else {<br>+ child = rtp_find_instance_by_media_source_ssrc(transport, transport_rtp, ssrc);<br>+ }<br> if (child && child != transport) {<br> /*<br> * It is safe to hold the child lock while holding the parent lock.<br>@@ -5438,7 +5556,7 @@<br> }<br> <br> if (ssrc_valid && rtp->themssrc_valid) {<br>- if (ssrc != rtp->themssrc) {<br>+ if (ssrc != rtp->themssrc && use_packet_source) {<br> /*<br> * Skip over this RTCP record as it does not contain the<br> * correct SSRC. We should not act upon RTCP records<br>@@ -5584,6 +5702,24 @@<br> case RTCP_PT_FUR:<br> /* Handle RTCP FUR as FIR by setting the format to 4 */<br> rc = AST_RTP_RTCP_FMT_FIR;<br>+ case RTCP_PT_RTPFB:<br>+ switch (rc) {<br>+ case AST_RTP_RTCP_FMT_NACK:<br>+ /* If retransmissions are not enabled ignore this message */<br>+ if (!ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_RETRANS_SEND)) {<br>+ break;<br>+ }<br>+<br>+ if (rtcp_debug_test_addr(addr)) {<br>+ ast_verbose("Received generic RTCP NACK message\n");<br>+ }<br>+<br>+ ast_rtp_rtcp_handle_nack(instance, rtcpheader, length);<br>+ break;<br>+ default:<br>+ break;<br>+ }<br>+ break;<br> case RTCP_PT_PSFB:<br> switch (rc) {<br> case AST_RTP_RTCP_FMT_PLI:<br>@@ -5981,7 +6117,7 @@<br> ssrc = ntohl(rtpheader[2]);<br> <br> /* Determine the appropriate instance for this */<br>- child = rtp_find_instance_by_ssrc(instance, rtp, ssrc);<br>+ child = rtp_find_instance_by_packet_source_ssrc(instance, rtp, ssrc);<br> if (!child) {<br> /* Neither the bundled parent nor any child has this SSRC */<br> return &ast_null_frame;<br>@@ -6614,6 +6750,8 @@<br> }<br> } else if (property == AST_RTP_PROPERTY_ASYMMETRIC_CODEC) {<br> rtp->asymmetric_codec = value;<br>+ } else if (property == AST_RTP_PROPERTY_RETRANS_SEND) {<br>+ rtp->send_buffer = ast_data_buffer_alloc(ast_free, DEFAULT_RTP_BUFFER_SIZE);<br> }<br> }<br> <br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/8774">change 8774</a>. To unsubscribe, 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/8774"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I7f7f124af3b9d5d2fd9cffc6ba8cb48a6fff06ec </div>
<div style="display:none"> Gerrit-Change-Number: 8774 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Benjamin Keith Ford <bford@digium.com> </div>