[Asterisk-code-review] media: Add experimental support for RTCP feedback. (asterisk[master])

George Joseph asteriskteam at digium.com
Fri Jan 27 07:04:53 CST 2017


George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/4614 )

Change subject: media: Add experimental support for RTCP feedback.
......................................................................


media: Add experimental support for RTCP feedback.

This change adds experimental support for providing RTCP
feedback information to codec modules so they can dynamically
change themselves based on conditions.

ASTERISK-26584

Change-Id: Ifd6aa77fb4a7ff546c6025900fc2baf332c31857
---
M codecs/codec_speex.c
M configs/samples/codecs.conf.sample
M funcs/func_frame_trace.c
M include/asterisk/frame.h
M include/asterisk/translate.h
M main/channel.c
M main/frame.c
M main/translate.c
M res/res_rtp_asterisk.c
9 files changed, 151 insertions(+), 2 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Matthew Fredrickson: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/codecs/codec_speex.c b/codecs/codec_speex.c
index 49990e9..72ac220 100644
--- a/codecs/codec_speex.c
+++ b/codecs/codec_speex.c
@@ -55,6 +55,9 @@
 #include "asterisk/frame.h"
 #include "asterisk/linkedlists.h"
 
+/* For struct ast_rtp_rtcp_report and struct ast_rtp_rtcp_report_block */
+#include "asterisk/rtp_engine.h"
+
 /* codec variables */
 static int quality = 3;
 static int complexity = 2;
@@ -64,6 +67,7 @@
 static float vbr_quality = 4;
 static int abr = 0;
 static int dtx = 0;	/* set to 1 to enable silence detection */
+static int exp_rtcp_fb = 0;	/* set to 1 to use experimental RTCP feedback for changing bitrate */
 
 static int preproc = 0;
 static int pp_vad = 0;
@@ -91,6 +95,11 @@
 	SpeexBits bits;
 	int framesize;
 	int silent_state;
+
+	int fraction_lost;
+	int quality;
+	int default_quality;
+
 #ifdef _SPEEX_TYPES_H
 	SpeexPreprocessState *pp;
 	spx_int16_t buf[BUFFER_SAMPLES];
@@ -136,6 +145,11 @@
 	if (dtx)
 		speex_encoder_ctl(tmp->speex, SPEEX_SET_DTX, &dtx); 
 	tmp->silent_state = 0;
+
+	tmp->fraction_lost = 0;
+	tmp->default_quality = vbr ? vbr_quality : quality;
+	tmp->quality = tmp->default_quality;
+	ast_debug(3, "Default quality (%s): %d\n", vbr ? "vbr" : "cbr", tmp->default_quality);
 
 	return 0;
 }
@@ -342,6 +356,69 @@
 	return result;
 }
 
+/*! \brief handle incoming RTCP feedback and possibly edit encoder settings */
+static void lintospeex_feedback(struct ast_trans_pvt *pvt, struct ast_frame *feedback)
+{
+	struct speex_coder_pvt *tmp = pvt->pvt;
+
+	struct ast_rtp_rtcp_report *rtcp_report;
+	struct ast_rtp_rtcp_report_block *report_block;
+
+	int fraction_lost;
+	int percent;
+	int bitrate;
+	int q;
+
+	if(!exp_rtcp_fb)
+		return;
+
+	rtcp_report = (struct ast_rtp_rtcp_report *)feedback->data.ptr;
+	if (rtcp_report->reception_report_count == 0)
+		return;
+	report_block = rtcp_report->report_block[0];
+	fraction_lost = report_block->lost_count.fraction;
+	if (fraction_lost == tmp->fraction_lost)
+		return;
+	/* Per RFC3550, fraction lost is defined to be the number of packets lost
+	 * divided by the number of packets expected. Since it's a 8-bit value,
+	 * and we want a percentage value, we multiply by 100 and divide by 256. */
+	percent = (fraction_lost*100)/256;
+	bitrate = 0;
+	q = -1;
+	ast_debug(3, "Fraction lost changed: %d --> %d percent loss\n", fraction_lost, percent);
+	/* Handle change */
+	speex_encoder_ctl(tmp->speex, SPEEX_GET_BITRATE, &bitrate);
+	ast_debug(3, "Current bitrate: %d\n", bitrate);
+	ast_debug(3, "Current quality: %d/%d\n", tmp->quality, tmp->default_quality);
+	/* FIXME BADLY Very ugly example of how this could be handled: probably sucks */
+	if (percent < 10) {
+		/* Not that bad, default quality is fine */
+		q = tmp->default_quality;
+	} else if (percent < 20) {
+		/* Quite bad, let's go down a bit */
+		q = tmp->default_quality-1;
+	} else if (percent < 30) {
+		/* Very bad, let's go down even more */
+		q = tmp->default_quality-2;
+	} else {
+		/* Really bad, use the lowest quality possible */
+		q = 0;
+	}
+	if (q < 0)
+		q = 0;
+	if (q != tmp->quality) {
+		ast_debug(3, "  -- Setting to %d\n", q);
+		if (vbr) {
+			float vbr_q = q;
+			speex_encoder_ctl(tmp->speex, SPEEX_SET_VBR_QUALITY, &vbr_q);
+		} else {
+			speex_encoder_ctl(tmp->speex, SPEEX_SET_QUALITY, &q);
+		}
+		tmp->quality = q;
+	}
+	tmp->fraction_lost = fraction_lost;
+}
+
 static void speextolin_destroy(struct ast_trans_pvt *arg)
 {
 	struct speex_coder_pvt *pvt = arg->pvt;
@@ -400,6 +477,7 @@
 	.newpvt = lintospeex_new,
 	.framein = lintospeex_framein,
 	.frameout = lintospeex_frameout,
+	.feedback = lintospeex_feedback,
 	.destroy = lintospeex_destroy,
 	.sample = slin8_sample,
 	.desc_size = sizeof(struct speex_coder_pvt),
@@ -446,6 +524,7 @@
 	.newpvt = lin16tospeexwb_new,
 	.framein = lintospeex_framein,
 	.frameout = lintospeex_frameout,
+	.feedback = lintospeex_feedback,
 	.destroy = lintospeex_destroy,
 	.sample = slin16_sample,
 	.desc_size = sizeof(struct speex_coder_pvt),
@@ -491,6 +570,7 @@
 	.newpvt = lin32tospeexuwb_new,
 	.framein = lintospeex_framein,
 	.frameout = lintospeex_frameout,
+	.feedback = lintospeex_feedback,
 	.destroy = lintospeex_destroy,
 	.desc_size = sizeof(struct speex_coder_pvt),
 	.buffer_samples = BUFFER_SAMPLES,
@@ -586,6 +666,9 @@
 				pp_dereverb_level = res_f;
 			} else
 				ast_log(LOG_ERROR,"Error! Preprocessor Dereverb Level must be >= 0\n");
+		} else if (!strcasecmp(var->name, "experimental_rtcp_feedback")) {
+			exp_rtcp_fb = ast_true(var->value) ? 1 : 0;
+			ast_verb(3, "CODEC SPEEX: Experimental RTCP Feedback. [%s]\n",exp_rtcp_fb ? "on" : "off");
 		}
 	}
 	ast_config_destroy(cfg);
diff --git a/configs/samples/codecs.conf.sample b/configs/samples/codecs.conf.sample
index 63d0352..e40aa35 100644
--- a/configs/samples/codecs.conf.sample
+++ b/configs/samples/codecs.conf.sample
@@ -57,6 +57,9 @@
 pp_dereverb_decay => 0.4
 pp_dereverb_level => 0.3
 
+; experimental bitrate changes depending on RTCP feedback [true / false]
+experimental_rtcp_feedback => false
+
 
 [plc]
 ; for all codecs which do not support native PLC
diff --git a/funcs/func_frame_trace.c b/funcs/func_frame_trace.c
index 08c4261..8a0b3dd 100644
--- a/funcs/func_frame_trace.c
+++ b/funcs/func_frame_trace.c
@@ -370,6 +370,9 @@
 		}
 		ast_verbose("Bytes: %d\n", frame->datalen);
 		break;
+	case AST_FRAME_RTCP:
+		ast_verbose("FrameType: RTCP\n");
+		break;
 	case AST_FRAME_NULL:
 		ast_verbose("FrameType: NULL\n");
 		break;
diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h
index 20f40f8..45bc8fc 100644
--- a/include/asterisk/frame.h
+++ b/include/asterisk/frame.h
@@ -127,6 +127,8 @@
 	 * directly into bridges.
 	 */
 	AST_FRAME_BRIDGE_ACTION_SYNC,
+	/*! RTCP feedback */
+	AST_FRAME_RTCP,
 };
 #define AST_FRAME_DTMF AST_FRAME_DTMF_END
 
diff --git a/include/asterisk/translate.h b/include/asterisk/translate.h
index 8188eb8..f0fa839 100644
--- a/include/asterisk/translate.h
+++ b/include/asterisk/translate.h
@@ -121,7 +121,7 @@
  *
  * As a minimum, a translator should supply name, srcfmt and dstfmt,
  * the required buf_size (in bytes) and buffer_samples (in samples),
- * and a few callbacks (framein, frameout, sample).
+ * and a few callbacks (framein, frameout, feedback, sample).
  * The outbuf is automatically prepended by AST_FRIENDLY_OFFSET
  * spare bytes so generic routines can place data in there.
  *
@@ -158,6 +158,10 @@
 	struct ast_frame * (*frameout)(struct ast_trans_pvt *pvt);
 	                                       /*!< Output frame callback. Generate a frame 
 	                                        *   with outbuf content. */
+
+	void (*feedback)(struct ast_trans_pvt *pvt, struct ast_frame *feedback);
+	                                       /*!< Feedback frame callback. Handle
+	                                        *   input frame. */
 
 	void (*destroy)(struct ast_trans_pvt *pvt);
 	                                       /*!< cleanup private data, if needed 
@@ -316,7 +320,9 @@
 /*!
  * \brief translates one or more frames
  * Apply an input frame into the translator and receive zero or one output frames.  Consume
- * determines whether the original frame should be freed
+ * determines whether the original frame should be freed.  In case the frame type is
+ * AST_FRAME_RTCP, the frame is not translated but passed to the translator codecs
+ * via the feedback callback, and a pointer to ast_null_frame is returned after that.
  * \param path tr translator structure to use for translation
  * \param f frame to translate
  * \param consume Whether or not to free the original frame
diff --git a/main/channel.c b/main/channel.c
index 00cfa31..68c45a2 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1531,6 +1531,7 @@
 	case AST_FRAME_IAX:
 	case AST_FRAME_CNG:
 	case AST_FRAME_MODEM:
+	case AST_FRAME_RTCP:
 		return 0;
 	}
 	return 0;
@@ -2866,6 +2867,7 @@
 				case AST_FRAME_IMAGE:
 				case AST_FRAME_HTML:
 				case AST_FRAME_MODEM:
+				case AST_FRAME_RTCP:
 					done = 1;
 					break;
 				case AST_FRAME_CONTROL:
@@ -4348,6 +4350,14 @@
 			 */
 			ast_read_generator_actions(chan, f);
 			break;
+		case AST_FRAME_RTCP:
+			/* Incoming RTCP feedback needs to get to the translator for
+			 * outgoing media, which means we treat it as an ast_write */
+			if (ast_channel_writetrans(chan)) {
+				ast_translate(ast_channel_writetrans(chan), f, 0);
+			}
+			ast_frfree(f);
+			f = &ast_null_frame;
 		default:
 			/* Just pass it on! */
 			break;
diff --git a/main/frame.c b/main/frame.c
index 0175c72..71feacb 100644
--- a/main/frame.c
+++ b/main/frame.c
@@ -533,6 +533,8 @@
 			break;
 		}
 		break;
+	case AST_FRAME_RTCP:
+		ast_copy_string(subclass, "RTCP", slen);
 	default:
 		ast_copy_string(subclass, "Unknown Subclass", slen);
 		break;
@@ -584,6 +586,9 @@
 	case AST_FRAME_VIDEO:
 		ast_copy_string(ftype, "Video", len);
 		break;
+	case AST_FRAME_RTCP:
+		ast_copy_string(ftype, "RTCP", len);
+		break;
 	default:
 		snprintf(ftype, len, "Unknown Frametype '%u'", frame_type);
 		break;
@@ -621,6 +626,9 @@
 	if (f->frametype == AST_FRAME_VIDEO) {
 		return;
 	}
+	if (f->frametype == AST_FRAME_RTCP) {
+		return;
+	}
 
 	ast_frame_type2str(f->frametype, ftype, sizeof(ftype));
 	ast_frame_subclass2str(f, subclass, sizeof(subclass), moreinfo, sizeof(moreinfo));
diff --git a/main/translate.c b/main/translate.c
index fa606e7..168a72a 100644
--- a/main/translate.c
+++ b/main/translate.c
@@ -530,6 +530,17 @@
 	long len;
 	int seqno;
 
+	if (f->frametype == AST_FRAME_RTCP) {
+		/* Just pass the feedback to the right callback, if it exists.
+		 * This "translation" does nothing so return a null frame. */
+		struct ast_trans_pvt *tp;
+		for (tp = p; tp; tp = tp->next) {
+			if (tp->t->feedback)
+				tp->t->feedback(tp, f);
+		}
+		return &ast_null_frame;
+	}
+
 	has_timing_info = ast_test_flag(f, AST_FRFLAG_HAS_TIMING_INFO);
 	ts = f->ts;
 	len = f->len;
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 58c217e..91d09b9 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -4320,6 +4320,29 @@
 					rtcp_report,
 					message_blob);
 			ast_json_unref(message_blob);
+
+			/* Return an AST_FRAME_RTCP frame with the ast_rtp_rtcp_report
+			 * object as a its data */
+			rtp->f.frametype = AST_FRAME_RTCP;
+			rtp->f.data.ptr = rtp->rawdata + AST_FRIENDLY_OFFSET;
+			memcpy(rtp->f.data.ptr, rtcp_report, sizeof(struct ast_rtp_rtcp_report));
+			rtp->f.datalen = sizeof(struct ast_rtp_rtcp_report);
+			if (rc > 0) {
+				/* There's always a single report block stored, here */
+				struct ast_rtp_rtcp_report *rtcp_report2;
+				report_block = rtp->f.data.ptr + rtp->f.datalen + sizeof(struct ast_rtp_rtcp_report_block *);
+				memcpy(report_block, rtcp_report->report_block[report_counter-1], sizeof(struct ast_rtp_rtcp_report_block));
+				rtcp_report2 = (struct ast_rtp_rtcp_report *)rtp->f.data.ptr;
+				rtcp_report2->report_block[report_counter-1] = report_block;
+				rtp->f.datalen += sizeof(struct ast_rtp_rtcp_report_block);
+			}
+			rtp->f.offset = AST_FRIENDLY_OFFSET;
+			rtp->f.samples = 0;
+			rtp->f.mallocd = 0;
+			rtp->f.delivery.tv_sec = 0;
+			rtp->f.delivery.tv_usec = 0;
+			rtp->f.src = "RTP";
+			f = &rtp->f;
 			break;
 		case RTCP_PT_FUR:
 		/* Handle RTCP FIR as FUR */

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ifd6aa77fb4a7ff546c6025900fc2baf332c31857
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Lorenzo Miniero <lminiero at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Lorenzo Miniero <lminiero at gmail.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>



More information about the asterisk-code-review mailing list