[Asterisk-code-review] bridge softmix: Fix some REMB bugs. (asterisk[15.4])

[George Joseph]

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

Change subject: bridge_softmix: Fix some REMB bugs.
......................................................................

bridge_softmix: Fix some REMB bugs.

This change fixes a bug where a REMB collector may be
freed twice, and also tweaks REMB combining such that if
there is no bitrate from anyone (or there are no sources)
we report 0 instead of using an old bitrate.

ASTERISK-27804

Change-Id: Ia9dc9c150043890ee7ff85e9cdec007f1a77fcfd
---
M bridges/bridge_softmix.c
1 file changed, 21 insertions(+), 13 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index f0a3fb4..ed88b7c 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -1318,6 +1318,12 @@
 			break;
 		}
 	}
+
+	/* After the report is integrated we reset this to 0 in case they stop producing
+	 * REMB reports.
+	 */
+	sc->remb.br_mantissa = 0;
+	sc->remb.br_exp = 0;
 }
 
 static void remb_send_report(struct ast_bridge_channel *bridge_channel, struct softmix_channel *sc)
@@ -1328,20 +1334,18 @@
 		return;
 	}
 
-	/* If we have a new bitrate then use it for the REMB, if not we use the previous
-	 * one until we know otherwise. This way the bitrate doesn't drop to 0 all of a sudden.
+	/* We always do this calculation as even when the bitrate is zero the browser
+	 * still prefers it to be accurate instead of lying.
 	 */
-	if (sc->remb_collector->bitrate) {
-		sc->remb_collector->feedback.remb.br_mantissa = sc->remb_collector->bitrate;
-		sc->remb_collector->feedback.remb.br_exp = 0;
+	sc->remb_collector->feedback.remb.br_mantissa = sc->remb_collector->bitrate;
+	sc->remb_collector->feedback.remb.br_exp = 0;
 
-		/* The mantissa only has 18 bits available, so while it exceeds them we bump
-		 * up the exp.
-		 */
-		while (sc->remb_collector->feedback.remb.br_mantissa > 0x3ffff) {
-			sc->remb_collector->feedback.remb.br_mantissa = sc->remb_collector->feedback.remb.br_mantissa >> 1;
-			sc->remb_collector->feedback.remb.br_exp++;
-		}
+	/* The mantissa only has 18 bits available, so while it exceeds them we bump
+	 * up the exp.
+	 */
+	while (sc->remb_collector->feedback.remb.br_mantissa > 0x3ffff) {
+		sc->remb_collector->feedback.remb.br_mantissa = sc->remb_collector->feedback.remb.br_mantissa >> 1;
+		sc->remb_collector->feedback.remb.br_exp++;
 	}
 
 	for (i = 0; i < AST_VECTOR_SIZE(&bridge_channel->stream_map.to_bridge); ++i) {
@@ -2062,6 +2066,7 @@
 	struct ast_bridge_channel *participant;
 	struct ast_vector_int media_types;
 	int nths[AST_MEDIA_TYPE_END] = {0};
+	int idx;
 
 	switch (bridge->softmix.video_mode.mode) {
 	case AST_BRIDGE_VIDEO_MODE_NONE:
@@ -2080,7 +2085,10 @@
 	 * When channels end up getting added back in they'll reuse their existing
 	 * collector and won't need to allocate a new one (unless they were just added).
 	 */
-	AST_VECTOR_RESET(&softmix_data->remb_collectors, ao2_cleanup);
+	for (idx = 0; idx < AST_VECTOR_SIZE(&softmix_data->remb_collectors); ++idx) {
+		ao2_cleanup(AST_VECTOR_GET(&softmix_data->remb_collectors, idx));
+		AST_VECTOR_REPLACE(&softmix_data->remb_collectors, idx, NULL);
+	}
 
 	/* First traversal: re-initialize all of the participants' stream maps */
 	AST_LIST_TRAVERSE(&bridge->channels, participant, entry) {

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

Gerrit-Project: asterisk
Gerrit-Branch: 15.4
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia9dc9c150043890ee7ff85e9cdec007f1a77fcfd
Gerrit-Change-Number: 8855
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180611/75e26492/attachment.html>