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