[Asterisk-code-review] Experimental patch to add RTCP feedback to codec modules (asterisk[14])

Lorenzo Miniero asteriskteam at digium.com
Tue Nov 29 09:58:27 CST 2016


Lorenzo Miniero has uploaded a new change for review. ( https://gerrit.asterisk.org/4520 )

Change subject: Experimental patch to add RTCP feedback to codec modules
......................................................................

Experimental patch to add RTCP feedback to codec modules

Change-Id: Ie74d254c904faed9173e4379ecce4267bcdb3099

Fixed ISO C90 warnings

Change-Id: I4e7e9ee4e4d156dc32732695e9c45a5d4ce6a0bb
---
M codecs/codec_speex.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
7 files changed, 122 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/20/4520/1

diff --git a/codecs/codec_speex.c b/codecs/codec_speex.c
index ca48eae..979b5ff 100644
--- a/codecs/codec_speex.c
+++ b/codecs/codec_speex.c
@@ -57,6 +57,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;
@@ -93,6 +96,10 @@
 	SpeexBits bits;
 	int framesize;
 	int silent_state;
+
+	int fraction_lost;
+	int quality, default_quality;
+
 #ifdef _SPEEX_TYPES_H
 	SpeexPreprocessState *pp;
 	spx_int16_t buf[BUFFER_SAMPLES];
@@ -138,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_verb(3, "Default quality (%s): %d\n", vbr ? "vbr" : "cbr", tmp->default_quality);
 
 	return 0;
 }
@@ -344,6 +356,55 @@
 	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 *)feedback->data.ptr;
+	if(rtcp_report->reception_report_count > 0) {
+		struct ast_rtp_rtcp_report_block *report_block = rtcp_report->report_block[0];
+		int fraction_lost = report_block->lost_count.fraction;
+		if(fraction_lost != tmp->fraction_lost) {
+			int percent = (fraction_lost*100)/256;
+			int bitrate = 0;
+			int q = -1;
+			ast_verb(3, "Fraction lost changed: %d --> %d percent loss\n", fraction_lost, percent);
+			/* Handle change */
+			speex_encoder_ctl(tmp->speex, SPEEX_GET_BITRATE, &bitrate);
+			ast_verb(3, "Current bitrate: %d\n", bitrate);
+			ast_verb(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_verb(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;
@@ -402,6 +463,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),
@@ -448,6 +510,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),
@@ -493,6 +556,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,
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 b8cd219..18e34ea 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.
  *
@@ -159,6 +159,10 @@
 	                                       /*!< 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 
 	                                        *   (often unnecessary). */
diff --git a/main/channel.c b/main/channel.c
index b4d451a..9283e95 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1534,6 +1534,7 @@
 	case AST_FRAME_IAX:
 	case AST_FRAME_CNG:
 	case AST_FRAME_MODEM:
+	case AST_FRAME_RTCP:
 		return 0;
 	}
 	return 0;
@@ -2869,6 +2870,7 @@
 				case AST_FRAME_IMAGE:
 				case AST_FRAME_HTML:
 				case AST_FRAME_MODEM:
+				case AST_FRAME_RTCP:
 					done = 1;
 					break;
 				case AST_FRAME_CONTROL:
@@ -4351,6 +4353,13 @@
 			 */
 			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 92b92b6..834d81f 100644
--- a/main/frame.c
+++ b/main/frame.c
@@ -535,6 +535,8 @@
 			break;
 		}
 		break;
+	case AST_FRAME_RTCP:
+		ast_copy_string(subclass, "RTCP", slen);
 	default:
 		ast_copy_string(subclass, "Unknown Subclass", slen);
 		break;
@@ -586,6 +588,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;
@@ -623,6 +628,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 43e6e29..ae8c0ab 100644
--- a/main/translate.c
+++ b/main/translate.c
@@ -532,6 +532,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 588247b..f639fa6 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -4321,6 +4321,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[0], sizeof(struct ast_rtp_rtcp_report_block));
+				rtcp_report2 = (struct ast_rtp_rtcp_report *)rtp->f.data.ptr;
+				rtcp_report2->report_block[0] = 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/4520
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4e7e9ee4e4d156dc32732695e9c45a5d4ce6a0bb
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Lorenzo Miniero <lminiero at gmail.com>



More information about the asterisk-code-review mailing list