[Asterisk-code-review] res rtp asterisk: Add support for raising additional RTCP me... (asterisk[master])

Kevin Harwell asteriskteam at digium.com
Thu Mar 29 15:19:18 CDT 2018


Kevin Harwell has submitted this change and it was merged. ( https://gerrit.asterisk.org/8635 )

Change subject: res_rtp_asterisk: Add support for raising additional RTCP messages.
......................................................................

res_rtp_asterisk: Add support for raising additional RTCP messages.

This change extends the existing AST_FRAME_RTCP frame type to be
able to contain additional RTCP message types, such as feedback
messages. The payload type is contained in the subclass which allows
knowing what is in the frame itself.

The RTCP feedback message type is now handled and REMB[1] messages
are raised with their containing information.

This also fixes a bug where all feedback messages were triggering
video updates instead of just FIR and FUR.

Finally RTCP frames are now passed up through the Asterisk core to
what is handling the channel, mapped appropriately in the case of
bridging, and written to an outgoing stream. Since RTCP frames are
on a per-stream basis this is only done on multistream capable
channels.

[1] https://tools.ietf.org/html/draft-alvestrand-rmcat-remb-03

ASTERISK-27758
ASTERISK-26366

Change-Id: I680da0ad8d5059d5e9655d896fb9d92e9da8491e
---
M channels/chan_pjsip.c
M codecs/codec_speex.c
M include/asterisk/frame.h
M include/asterisk/rtp_engine.h
M main/bridge_channel.c
M main/channel.c
M res/res_rtp_asterisk.c
7 files changed, 104 insertions(+), 15 deletions(-)

Approvals:
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  Sean Bright: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved; Approved for Submit



diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 2c111fe..5cb52a5 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -965,6 +965,8 @@
 		break;
 	case AST_FRAME_CNG:
 		break;
+	case AST_FRAME_RTCP:
+		break;
 	default:
 		ast_log(LOG_WARNING, "Can't send %u type frames with PJSIP\n", frame->frametype);
 		break;
diff --git a/codecs/codec_speex.c b/codecs/codec_speex.c
index 0a93596..591fce9 100644
--- a/codecs/codec_speex.c
+++ b/codecs/codec_speex.c
@@ -372,6 +372,11 @@
 	if(!exp_rtcp_fb)
 		return;
 
+	/* We only accept feedback information in the form of SR and RR reports */
+	if (feedback->subclass.integer != AST_RTP_RTCP_SR && feedback->subclass.integer != AST_RTP_RTCP_RR) {
+		return;
+	}
+
 	rtcp_report = (struct ast_rtp_rtcp_report *)feedback->data.ptr;
 	if (rtcp_report->reception_report_count == 0)
 		return;
diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h
index eb6a647..c3c0f88 100644
--- a/include/asterisk/frame.h
+++ b/include/asterisk/frame.h
@@ -127,7 +127,7 @@
 	 * directly into bridges.
 	 */
 	AST_FRAME_BRIDGE_ACTION_SYNC,
-	/*! RTCP feedback */
+	/*! RTCP feedback (the subclass will contain the payload type) */
 	AST_FRAME_RTCP,
 };
 #define AST_FRAME_DTMF AST_FRAME_DTMF_END
diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h
index 4e32d6b..b552948 100644
--- a/include/asterisk/rtp_engine.h
+++ b/include/asterisk/rtp_engine.h
@@ -292,6 +292,14 @@
 #define AST_RTP_RTCP_SR 200
 /*! Receiver Report */
 #define AST_RTP_RTCP_RR 201
+/*! Payload Specific Feed Back (From RFC4585 also RFC5104) */
+#define AST_RTP_RTCP_PSFB    206
+
+/* Common RTCP feedback message types */
+/*! Full INTRA-frame Request (From RFC5104) */
+#define AST_RTP_RTCP_FMT_FIR	4
+/*! REMB Information (From draft-alvestrand-rmcat-remb-03) */
+#define AST_RTP_RTCP_FMT_REMB	15
 
 /*!
  * \since 12
@@ -327,6 +335,24 @@
 	struct ast_rtp_rtcp_report_block *report_block[0];
 };
 
+/*!
+ * \since 15.4.0
+ * \brief A REMB feedback message (see draft-alvestrand-rmcat-remb-03 for details) */
+struct ast_rtp_rtcp_feedback_remb {
+	unsigned int br_exp;		/*!< Exponential scaling of the mantissa for the maximum total media bit rate value */
+	unsigned int br_mantissa;	/*!< The mantissa of the maximum total media bit rate */
+};
+
+/*!
+ * \since 15.4.0
+ * \brief An object that represents data received in a feedback report */
+struct ast_rtp_rtcp_feedback {
+	unsigned int fmt; /*!< The feedback message type */
+	union {
+		struct ast_rtp_rtcp_feedback_remb remb; /*!< REMB feedback information */
+	};
+};
+
 /*! Structure that represents statistics from an RTP instance */
 struct ast_rtp_instance_stats {
 	/*! Number of packets transmitted */
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 89e5571..3aac5eb 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -653,7 +653,8 @@
 		case AST_FRAME_VIDEO:
 		case AST_FRAME_TEXT:
 		case AST_FRAME_IMAGE:
-			/* Media frames need to be mapped to an appropriate write stream */
+		case AST_FRAME_RTCP:
+			/* These frames need to be mapped to an appropriate write stream */
 			if (frame->stream_num < 0) {
 				/* Map to default stream */
 				frame->stream_num = -1;
diff --git a/main/channel.c b/main/channel.c
index 869b29f..815d5db 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -4123,8 +4123,7 @@
 			if (ast_channel_writetrans(chan)) {
 				ast_translate(ast_channel_writetrans(chan), f, 0);
 			}
-			ast_frfree(f);
-			f = &ast_null_frame;
+			break;
 		default:
 			/* Just pass it on! */
 			break;
@@ -5267,6 +5266,14 @@
 		/* Ignore these */
 		res = 0;
 		break;
+	case AST_FRAME_RTCP:
+		/* RTCP information is on a per-stream basis and only available on multistream capable channels */
+		if (ast_channel_tech(chan)->write_stream && stream) {
+			res = ast_channel_tech(chan)->write_stream(chan, ast_stream_get_position(stream), fr);
+		} else {
+			res = 0;
+		}
+		break;
 	default:
 		/* At this point, fr is the incoming frame and f is NULL.  Channels do
 		 * not expect to get NULL as a frame pointer and will segfault.  Hence,
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index d0e4824..b010f6c 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -106,7 +106,7 @@
 #define RTCP_PT_APP     204
 /* VP8: RTCP Feedback */
 /*! Payload Specific Feed Back (From RFC4585 also RFC5104) */
-#define RTCP_PT_PSFB    206
+#define RTCP_PT_PSFB    AST_RTP_RTCP_PSFB
 
 #define RTP_MTU		1200
 #define DTMF_SAMPLE_RATE_MS    8 /*!< DTMF samples per millisecond */
@@ -5185,6 +5185,7 @@
 #define RTCP_SR_BLOCK_WORD_LENGTH 5
 #define RTCP_RR_BLOCK_WORD_LENGTH 6
 #define RTCP_HEADER_SSRC_LENGTH   2
+#define RTCP_FB_REMB_BLOCK_WORD_LENGTH 5
 
 static struct ast_frame *ast_rtcp_interpret(struct ast_rtp_instance *instance, const unsigned char *rtcpdata, size_t size, struct ast_sockaddr *addr)
 {
@@ -5266,6 +5267,7 @@
 		RAII_VAR(struct ast_rtp_rtcp_report *, rtcp_report, NULL, ao2_cleanup);
 		struct ast_rtp_instance *child;
 		struct ast_rtp *rtp;
+		struct ast_rtp_rtcp_feedback *feedback;
 
 		i = position;
 		first_word = ntohl(rtcpheader[i]);
@@ -5284,7 +5286,15 @@
 			min_length += (rc * RTCP_RR_BLOCK_WORD_LENGTH);
 			break;
 		case RTCP_PT_FUR:
+			break;
 		case RTCP_PT_PSFB:
+			switch (rc) {
+			case AST_RTP_RTCP_FMT_REMB:
+				min_length += RTCP_FB_REMB_BLOCK_WORD_LENGTH;
+				break;
+			default:
+				break;
+			}
 			break;
 		case RTCP_PT_SDES:
 		case RTCP_PT_BYE:
@@ -5493,6 +5503,7 @@
 			/* Return an AST_FRAME_RTCP frame with the ast_rtp_rtcp_report
 			 * object as a its data */
 			transport_rtp->f.frametype = AST_FRAME_RTCP;
+			transport_rtp->f.subclass.integer = pt;
 			transport_rtp->f.data.ptr = rtp->rtcp->frame_buf + AST_FRIENDLY_OFFSET;
 			memcpy(transport_rtp->f.data.ptr, rtcp_report, sizeof(struct ast_rtp_rtcp_report));
 			transport_rtp->f.datalen = sizeof(struct ast_rtp_rtcp_report);
@@ -5514,18 +5525,55 @@
 			f = &transport_rtp->f;
 			break;
 		case RTCP_PT_FUR:
-		/* Handle RTCP FIR as FUR */
+		/* Handle RTCP FUR as FIR by setting the format to 4 */
+			rc = AST_RTP_RTCP_FMT_FIR;
 		case RTCP_PT_PSFB:
-			if (rtcp_debug_test_addr(addr)) {
-				ast_verbose("Received an RTCP Fast Update Request\n");
+			switch (rc) {
+			case AST_RTP_RTCP_FMT_FIR:
+				if (rtcp_debug_test_addr(addr)) {
+					ast_verbose("Received an RTCP Fast Update Request\n");
+				}
+				transport_rtp->f.frametype = AST_FRAME_CONTROL;
+				transport_rtp->f.subclass.integer = AST_CONTROL_VIDUPDATE;
+				transport_rtp->f.datalen = 0;
+				transport_rtp->f.samples = 0;
+				transport_rtp->f.mallocd = 0;
+				transport_rtp->f.src = "RTP";
+				f = &transport_rtp->f;
+				break;
+			case AST_RTP_RTCP_FMT_REMB:
+				/* If REMB support is not enabled ignore this message */
+				if (!ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_REMB)) {
+					break;
+				}
+
+				if (rtcp_debug_test_addr(addr)) {
+					ast_verbose("Received REMB report\n");
+				}
+				transport_rtp->f.frametype = AST_FRAME_RTCP;
+				transport_rtp->f.subclass.integer = pt;
+				transport_rtp->f.stream_num = rtp->stream_num;
+				transport_rtp->f.data.ptr = rtp->rtcp->frame_buf + AST_FRIENDLY_OFFSET;
+				feedback = transport_rtp->f.data.ptr;
+				feedback->fmt = rc;
+
+				/* We don't actually care about the SSRC information in the feedback message */
+				first_word = ntohl(rtcpheader[i + 2]);
+				feedback->remb.br_exp = (first_word >> 18) & ((1 << 6) - 1);
+				feedback->remb.br_mantissa = first_word & ((1 << 18) - 1);
+
+				transport_rtp->f.datalen = sizeof(struct ast_rtp_rtcp_feedback);
+				transport_rtp->f.offset = AST_FRIENDLY_OFFSET;
+				transport_rtp->f.samples = 0;
+				transport_rtp->f.mallocd = 0;
+				transport_rtp->f.delivery.tv_sec = 0;
+				transport_rtp->f.delivery.tv_usec = 0;
+				transport_rtp->f.src = "RTP";
+				f = &transport_rtp->f;
+				break;
+			default:
+				break;
 			}
-			transport_rtp->f.frametype = AST_FRAME_CONTROL;
-			transport_rtp->f.subclass.integer = AST_CONTROL_VIDUPDATE;
-			transport_rtp->f.datalen = 0;
-			transport_rtp->f.samples = 0;
-			transport_rtp->f.mallocd = 0;
-			transport_rtp->f.src = "RTP";
-			f = &transport_rtp->f;
 			break;
 		case RTCP_PT_SDES:
 			if (rtcp_debug_test_addr(addr)) {

-- 
To view, visit https://gerrit.asterisk.org/8635
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I680da0ad8d5059d5e9655d896fb9d92e9da8491e
Gerrit-Change-Number: 8635
Gerrit-PatchSet: 4
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180329/ed0f3578/attachment-0001.html>


More information about the asterisk-code-review mailing list