[Asterisk-code-review] bridge softmix / app confbridge: Add support for REMB combin... (asterisk[master])

George Joseph asteriskteam at digium.com
Wed Apr 18 15:37:46 CDT 2018


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

Change subject: bridge_softmix / app_confbridge: Add support for REMB combining.
......................................................................

bridge_softmix / app_confbridge: Add support for REMB combining.

This change adds the ability for multiple REMB reports in
bridge_softmix to be combined according to a configured
behavior into a single report. This single report is sent
back to the sender of video, which adjusts the encoding bitrate
to be at or below the bitrate of the report. The available
behaviors are: lowest, highest, and average. Lowest uses the
lowest received bitrate. Highest uses the highest received
bitrate. Average goes through the received bitrates adding
them to the previous average and creates a new average.

Other behaviors can be added in the future and the existing
average one may be adjusted, but this provides the foundation
to do so.

Support for configuring which behavior to use has been
added to app_confbridge.

ASTERISK-27804

Change-Id: I9eafe4e7c1f72d67074a8d6acb26bfcf19322b66
---
M apps/app_confbridge.c
M apps/confbridge/conf_config_parser.c
M apps/confbridge/include/confbridge.h
M bridges/bridge_softmix.c
M bridges/bridge_softmix/include/bridge_softmix_internal.h
M configs/samples/confbridge.conf.sample
M include/asterisk/bridge.h
M main/bridge.c
8 files changed, 376 insertions(+), 3 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index d8407d8..25cf275 100644
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -1544,6 +1544,13 @@
 			ast_bridge_set_sfu_video_mode(conference->bridge);
 			ast_bridge_set_video_update_discard(conference->bridge, conference->b_profile.video_update_discard);
 			ast_bridge_set_remb_send_interval(conference->bridge, conference->b_profile.remb_send_interval);
+			if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE)) {
+				ast_brige_set_remb_behavior(conference->bridge, AST_BRIDGE_VIDEO_SFU_REMB_AVERAGE);
+			} else if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_REMB_BEHAVIOR_LOWEST)) {
+				ast_brige_set_remb_behavior(conference->bridge, AST_BRIDGE_VIDEO_SFU_REMB_LOWEST);
+			} else if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST)) {
+				ast_brige_set_remb_behavior(conference->bridge, AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST);
+			}
 		}
 
 		/* Link it into the conference bridges container */
diff --git a/apps/confbridge/conf_config_parser.c b/apps/confbridge/conf_config_parser.c
index f9d7483..c143e39 100644
--- a/apps/confbridge/conf_config_parser.c
+++ b/apps/confbridge/conf_config_parser.c
@@ -470,6 +470,27 @@
 						better quality for all receivers.
 					</para></description>
 				</configOption>
+				<configOption name="remb_behavior" default="average">
+					<synopsis>Sets how REMB reports are generated from multiple sources</synopsis>
+					<description><para>
+						Sets how REMB reports are combined from multiple sources to form one. A REMB report
+						consists of information about the receiver estimated maximum bitrate. As a source
+						stream may be forwarded to multiple receivers the reports must be combined into
+						a single one which is sent to the sender.</para>
+						<enumlist>
+							<enum name="average">
+								<para>The average of all estimated maximum bitrates is taken and sent
+								to the sender.</para>
+							</enum>
+							<enum name="lowest">
+								<para>The lowest estimated maximum bitrate is forwarded to the sender.</para>
+							</enum>
+							<enum name="highest">
+								<para>The highest estimated maximum bitrate is forwarded to the sender.</para>
+							</enum>
+						</enumlist>
+					</description>
+				</configOption>
 				<configOption name="template">
 					<synopsis>When using the CONFBRIDGE dialplan function, use a bridge profile as a template for creating a new temporary profile</synopsis>
 				</configOption>
@@ -1675,6 +1696,23 @@
 	ast_cli(a->fd,"Video Update Discard: %u\n", b_profile.video_update_discard);
 	ast_cli(a->fd,"REMB Send Interval: %u\n", b_profile.remb_send_interval);
 
+	switch (b_profile.flags
+		& (BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE | BRIDGE_OPT_REMB_BEHAVIOR_LOWEST
+			| BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST)) {
+	case BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE:
+		ast_cli(a->fd, "REMB Behavior:           average\n");
+		break;
+	case BRIDGE_OPT_REMB_BEHAVIOR_LOWEST:
+		ast_cli(a->fd, "REMB Behavior:           lowest\n");
+		break;
+	case BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST:
+		ast_cli(a->fd, "REMB Behavior:           highest\n");
+		break;
+	default:
+		ast_assert(0);
+		break;
+	}
+
 	ast_cli(a->fd,"sound_only_person:    %s\n", conf_get_sound(CONF_SOUND_ONLY_PERSON, b_profile.sounds));
 	ast_cli(a->fd,"sound_only_one:       %s\n", conf_get_sound(CONF_SOUND_ONLY_ONE, b_profile.sounds));
 	ast_cli(a->fd,"sound_has_joined:     %s\n", conf_get_sound(CONF_SOUND_HAS_JOINED, b_profile.sounds));
@@ -2011,6 +2049,30 @@
 	return 0;
 }
 
+static int remb_behavior_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)
+{
+	struct bridge_profile *b_profile = obj;
+
+	if (strcasecmp(var->name, "remb_behavior")) {
+		return -1;
+	}
+
+	ast_clear_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE |
+		BRIDGE_OPT_REMB_BEHAVIOR_LOWEST |
+		BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST);
+
+	if (!strcasecmp(var->value, "average")) {
+		ast_set_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE);
+	} else if (!strcasecmp(var->value, "lowest")) {
+		ast_set_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_LOWEST);
+	} else if (!strcasecmp(var->value, "highest")) {
+		ast_set_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST);
+	} else {
+		return -1;
+	}
+	return 0;
+}
+
 static int user_template_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)
 {
 	struct user_profile *u_profile = obj;
@@ -2245,6 +2307,7 @@
 	aco_option_register_custom(&cfg_info, "sound_", ACO_PREFIX, bridge_types, NULL, sound_option_handler, 0);
 	aco_option_register(&cfg_info, "video_update_discard", ACO_EXACT, bridge_types, "2000", OPT_UINT_T, 0, FLDSET(struct bridge_profile, video_update_discard));
 	aco_option_register(&cfg_info, "remb_send_interval", ACO_EXACT, bridge_types, "0", OPT_UINT_T, 0, FLDSET(struct bridge_profile, remb_send_interval));
+	aco_option_register_custom(&cfg_info, "remb_behavior", ACO_EXACT, bridge_types, "average", remb_behavior_handler, 0);
 	/* This option should only be used with the CONFBRIDGE dialplan function */
 	aco_option_register_custom(&cfg_info, "template", ACO_EXACT, bridge_types, NULL, bridge_template_handler, 0);
 
diff --git a/apps/confbridge/include/confbridge.h b/apps/confbridge/include/confbridge.h
index c2f8f9a..0a0a571 100644
--- a/apps/confbridge/include/confbridge.h
+++ b/apps/confbridge/include/confbridge.h
@@ -76,6 +76,9 @@
 	BRIDGE_OPT_RECORD_FILE_TIMESTAMP = (1 << 5), /*!< Set if the record file should have a timestamp appended */
 	BRIDGE_OPT_BINAURAL_ACTIVE = (1 << 6), /*!< Set if binaural convolution is activated */
 	BRIDGE_OPT_VIDEO_SRC_SFU = (1 << 7), /*!< Selective forwarding unit */
+	BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE = (1 << 8), /*!< The average of all REMB reports is sent to the sender */
+	BRIDGE_OPT_REMB_BEHAVIOR_LOWEST = (1 << 9), /*!< The lowest estimated maximum bitrate is sent to the sender */
+	BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST = (1 << 10), /*!< The highest estimated maximum bitrate is sent to the sender */
 };
 
 enum conf_menu_action_id {
diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index 16e1fb8..f0a3fb4 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -69,6 +69,15 @@
 #define SOFTBRIDGE_VIDEO_DEST_LEN strlen(SOFTBRIDGE_VIDEO_DEST_PREFIX)
 #define SOFTBRIDGE_VIDEO_DEST_SEPARATOR '_'
 
+struct softmix_remb_collector {
+	/*! The frame which will be given to each source stream */
+	struct ast_frame frame;
+	/*! The REMB to send to the source which is collecting REMB reports */
+	struct ast_rtp_rtcp_feedback feedback;
+	/*! The maximum bitrate */
+	unsigned int bitrate;
+};
+
 struct softmix_stats {
 	/*! Each index represents a sample rate used above the internal rate. */
 	unsigned int sample_rates[16];
@@ -768,6 +777,10 @@
 
 	ast_stream_topology_free(sc->topology);
 
+	ao2_cleanup(sc->remb_collector);
+
+	AST_VECTOR_FREE(&sc->video_sources);
+
 	/* Drop mutex lock */
 	ast_mutex_destroy(&sc->lock);
 
@@ -1160,6 +1173,39 @@
 
 /*!
  * \internal
+ * \brief Determine what to do with an RTCP frame.
+ * \since 15.4.0
+ *
+ * \param bridge Which bridge is getting the frame
+ * \param bridge_channel Which channel is writing the frame.
+ * \param frame What is being written.
+ */
+static void softmix_bridge_write_rtcp(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
+{
+	struct ast_rtp_rtcp_feedback *feedback = frame->data.ptr;
+	struct softmix_channel *sc = bridge_channel->tech_pvt;
+
+	/* We only care about REMB reports right now. In the future we may be able to use sender or
+	 * receiver reports to further tweak things, but not yet.
+	 */
+	if (frame->subclass.integer != AST_RTP_RTCP_PSFB || feedback->fmt != AST_RTP_RTCP_FMT_REMB ||
+		bridge->softmix.video_mode.mode != AST_BRIDGE_VIDEO_MODE_SFU ||
+		!bridge->softmix.video_mode.mode_data.sfu_data.remb_send_interval) {
+		return;
+	}
+
+	/* REMB is the total estimated maximum bitrate across all streams within the session, so we store
+	 * only the latest report and use it everywhere.
+	 */
+	ast_mutex_lock(&sc->lock);
+	sc->remb = feedback->remb;
+	ast_mutex_unlock(&sc->lock);
+
+	return;
+}
+
+/*!
+ * \internal
  * \brief Determine what to do with a frame written into the bridge.
  * \since 12.0.0
  *
@@ -1204,6 +1250,9 @@
 	case AST_FRAME_CONTROL:
 		res = softmix_bridge_write_control(bridge, bridge_channel, frame);
 		break;
+	case AST_FRAME_RTCP:
+		softmix_bridge_write_rtcp(bridge, bridge_channel, frame);
+		break;
 	case AST_FRAME_BRIDGE_ACTION:
 		res = ast_bridge_queue_everyone_else(bridge, bridge_channel, frame);
 		break;
@@ -1217,6 +1266,104 @@
 	}
 
 	return res;
+}
+
+static void remb_collect_report(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel,
+	struct softmix_bridge_data *softmix_data, struct softmix_channel *sc)
+{
+	int i;
+	unsigned int bitrate;
+
+	/* If there are no video sources that we are a receiver of then we have noone to
+	 * report REMB to.
+	 */
+	if (!AST_VECTOR_SIZE(&sc->video_sources)) {
+		return;
+	}
+
+	/* We evenly divide the available maximum bitrate across the video sources
+	 * to this receiver so each source gets an equal slice.
+	 */
+	bitrate = (sc->remb.br_mantissa << sc->remb.br_exp) / AST_VECTOR_SIZE(&sc->video_sources);
+
+	/* If this receiver has no bitrate yet ignore it */
+	if (!bitrate) {
+		return;
+	}
+
+	for (i = 0; i < AST_VECTOR_SIZE(&sc->video_sources); ++i) {
+		struct softmix_remb_collector *collector;
+
+		/* The collector will always exist if a video source is in our list */
+		collector = AST_VECTOR_GET(&softmix_data->remb_collectors, AST_VECTOR_GET(&sc->video_sources, i));
+
+		if (!collector->bitrate) {
+			collector->bitrate = bitrate;
+			continue;
+		}
+
+		switch (bridge->softmix.video_mode.mode_data.sfu_data.remb_behavior) {
+		case AST_BRIDGE_VIDEO_SFU_REMB_AVERAGE:
+			collector->bitrate = (collector->bitrate + bitrate) / 2;
+			break;
+		case AST_BRIDGE_VIDEO_SFU_REMB_LOWEST:
+			if (bitrate < collector->bitrate) {
+				collector->bitrate = bitrate;
+			}
+			break;
+		case AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST:
+			if (bitrate > collector->bitrate) {
+				collector->bitrate = bitrate;
+			}
+			break;
+		}
+	}
+}
+
+static void remb_send_report(struct ast_bridge_channel *bridge_channel, struct softmix_channel *sc)
+{
+	int i;
+
+	if (!sc->remb_collector) {
+		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.
+	 */
+	if (sc->remb_collector->bitrate) {
+		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++;
+		}
+	}
+
+	for (i = 0; i < AST_VECTOR_SIZE(&bridge_channel->stream_map.to_bridge); ++i) {
+		int bridge_num = AST_VECTOR_GET(&bridge_channel->stream_map.to_bridge, i);
+
+		/* If this stream is not being provided to the bridge there can be no receivers of it
+		 * so therefore no REMB reports.
+		 */
+		if (bridge_num == -1) {
+			continue;
+		}
+
+		/* We need to update the frame with this stream, or else it won't be
+		 * properly routed. We don't use the actual channel stream identifier as
+		 * the bridging core will do the translation from bridge stream identifier to
+		 * channel stream identifier.
+		 */
+		sc->remb_collector->frame.stream_num = bridge_num;
+		ast_bridge_channel_queue_frame(bridge_channel, &sc->remb_collector->frame);
+	}
+
+	sc->remb_collector->bitrate = 0;
 }
 
 static void gather_softmix_stats(struct softmix_stats *stats,
@@ -1440,6 +1587,7 @@
 		struct ast_format *cur_slin = ast_format_cache_get_slin_by_rate(softmix_data->internal_rate);
 		unsigned int softmix_samples = SOFTMIX_SAMPLES(softmix_data->internal_rate, softmix_data->internal_mixing_interval);
 		unsigned int softmix_datalen = SOFTMIX_DATALEN(softmix_data->internal_rate, softmix_data->internal_mixing_interval);
+		int remb_update = 0;
 
 		if (softmix_datalen > MAX_DATALEN) {
 			/* This should NEVER happen, but if it does we need to know about it. Almost
@@ -1478,6 +1626,14 @@
 		check_binaural_position_change(bridge, softmix_data);
 #endif
 
+		/* If we need to do a REMB update to all video sources then do so */
+		if (bridge->softmix.video_mode.mode == AST_BRIDGE_VIDEO_MODE_SFU &&
+			bridge->softmix.video_mode.mode_data.sfu_data.remb_send_interval &&
+			ast_tvdiff_ms(ast_tvnow(), softmix_data->last_remb_update) > bridge->softmix.video_mode.mode_data.sfu_data.remb_send_interval) {
+			remb_update = 1;
+			softmix_data->last_remb_update = ast_tvnow();
+		}
+
 		/* Go through pulling audio from each factory that has it available */
 		AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) {
 			struct softmix_channel *sc = bridge_channel->tech_pvt;
@@ -1511,6 +1667,9 @@
 						ast_channel_name(bridge_channel->chan));
 #endif
 				mixing_array.used_entries++;
+			}
+			if (remb_update) {
+				remb_collect_report(bridge, bridge_channel, softmix_data, sc);
 			}
 			ast_mutex_unlock(&sc->lock);
 		}
@@ -1562,6 +1721,10 @@
 
 			/* A frame is now ready for the channel. */
 			ast_bridge_channel_queue_frame(bridge_channel, &sc->write_frame);
+
+			if (remb_update) {
+				remb_send_report(bridge_channel, sc);
+			}
 		}
 
 		update_all_rates = 0;
@@ -1688,6 +1851,8 @@
 	}
 	ast_mutex_destroy(&softmix_data->lock);
 	ast_cond_destroy(&softmix_data->cond);
+	AST_VECTOR_RESET(&softmix_data->remb_collectors, ao2_cleanup);
+	AST_VECTOR_FREE(&softmix_data->remb_collectors);
 	ast_free(softmix_data);
 }
 
@@ -1717,6 +1882,8 @@
 	softmix_data->default_sample_size = SOFTMIX_SAMPLES(softmix_data->internal_rate,
 			softmix_data->internal_mixing_interval);
 #endif
+
+	AST_VECTOR_INIT(&softmix_data->remb_collectors, 0);
 
 	bridge->tech_pvt = softmix_data;
 
@@ -1814,12 +1981,67 @@
 
 			stream = ast_stream_topology_get_stream(topology, i);
 			if (is_video_dest(stream, source_channel_name, source_stream_name)) {
+				struct softmix_channel *sc = participant->tech_pvt;
+
 				AST_VECTOR_REPLACE(&participant->stream_map.to_channel, bridge_stream_position, i);
+				AST_VECTOR_APPEND(&sc->video_sources, bridge_stream_position);
 				break;
 			}
 		}
 		ast_channel_unlock(participant->chan);
 		ast_bridge_channel_unlock(participant);
+	}
+}
+
+/*!
+ * \brief Allocate a REMB collector
+ *
+ * \retval non-NULL success
+ * \retval NULL failure
+ */
+static struct softmix_remb_collector *remb_collector_alloc(void)
+{
+	struct softmix_remb_collector *collector;
+
+	collector = ao2_alloc_options(sizeof(*collector), NULL, AO2_ALLOC_OPT_LOCK_NOLOCK);
+	if (!collector) {
+		return NULL;
+	}
+
+	collector->frame.frametype = AST_FRAME_RTCP;
+	collector->frame.subclass.integer = AST_RTP_RTCP_PSFB;
+	collector->feedback.fmt = AST_RTP_RTCP_FMT_REMB;
+	collector->frame.data.ptr = &collector->feedback;
+	collector->frame.datalen = sizeof(collector->feedback);
+
+	return collector;
+}
+
+/*!
+ * \brief Setup REMB collection for a particular bridge stream and channel.
+ *
+ * \param bridge The bridge
+ * \param bridge_channel Channel that is collecting REMB information
+ * \param bridge_stream_position The slot in the bridge where source video comes from
+ */
+static void remb_enable_collection(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel,
+	size_t bridge_stream_position)
+{
+	struct softmix_channel *sc = bridge_channel->tech_pvt;
+	struct softmix_bridge_data *softmix_data = bridge->tech_pvt;
+
+	if (!sc->remb_collector) {
+		sc->remb_collector = remb_collector_alloc();
+		if (!sc->remb_collector) {
+			/* This is not fatal. Things will still continue to work but we won't
+			 * produce a REMB report to the sender.
+			 */
+			return;
+		}
+	}
+
+	if (AST_VECTOR_REPLACE(&softmix_data->remb_collectors, bridge_stream_position, ao2_bump(sc->remb_collector))) {
+		ao2_ref(sc->remb_collector, -1);
 	}
 }
 
@@ -1835,6 +2057,8 @@
  */
 static void softmix_bridge_stream_topology_changed(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
 {
+	struct softmix_bridge_data *softmix_data = bridge->tech_pvt;
+	struct softmix_channel *sc;
 	struct ast_bridge_channel *participant;
 	struct ast_vector_int media_types;
 	int nths[AST_MEDIA_TYPE_END] = {0};
@@ -1852,11 +2076,22 @@
 
 	AST_VECTOR_INIT(&media_types, AST_MEDIA_TYPE_END);
 
+	/* The bridge stream identifiers may change, so reset the mapping for them.
+	 * 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);
+
 	/* First traversal: re-initialize all of the participants' stream maps */
 	AST_LIST_TRAVERSE(&bridge->channels, participant, entry) {
 		ast_bridge_channel_lock(participant);
+
 		AST_VECTOR_RESET(&participant->stream_map.to_channel, AST_VECTOR_ELEM_CLEANUP_NOOP);
 		AST_VECTOR_RESET(&participant->stream_map.to_bridge, AST_VECTOR_ELEM_CLEANUP_NOOP);
+
+		sc = participant->tech_pvt;
+		AST_VECTOR_RESET(&sc->video_sources, AST_VECTOR_ELEM_CLEANUP_NOOP);
+
 		ast_bridge_channel_unlock(participant);
 	}
 
@@ -1897,7 +2132,12 @@
 			if (is_video_source(stream)) {
 				AST_VECTOR_APPEND(&media_types, AST_MEDIA_TYPE_VIDEO);
 				AST_VECTOR_REPLACE(&participant->stream_map.to_bridge, i, AST_VECTOR_SIZE(&media_types) - 1);
-				AST_VECTOR_REPLACE(&participant->stream_map.to_channel, AST_VECTOR_SIZE(&media_types) - 1, -1);
+				/*
+				 * There are cases where we need to bidirectionally send frames, such as for REMB reports
+				 * so we also map back to the channel.
+				 */
+				AST_VECTOR_REPLACE(&participant->stream_map.to_channel, AST_VECTOR_SIZE(&media_types) - 1, i);
+				remb_enable_collection(bridge, participant, AST_VECTOR_SIZE(&media_types) - 1);
 				/*
 				 * Unlock the channel and participant to prevent
 				 * potential deadlock in map_source_to_destinations().
diff --git a/bridges/bridge_softmix/include/bridge_softmix_internal.h b/bridges/bridge_softmix/include/bridge_softmix_internal.h
index f842acb..3aa9091 100644
--- a/bridges/bridge_softmix/include/bridge_softmix_internal.h
+++ b/bridges/bridge_softmix/include/bridge_softmix_internal.h
@@ -50,6 +50,8 @@
 #include "asterisk/astobj2.h"
 #include "asterisk/timing.h"
 #include "asterisk/translate.h"
+#include "asterisk/rtp_engine.h"
+#include "asterisk/vector.h"
 
 #ifdef BINAURAL_RENDERING
 #include <fftw3.h>
@@ -124,6 +126,8 @@
 	int energy_average;
 };
 
+struct softmix_remb_collector;
+
 /*! \brief Structure which contains per-channel mixing information */
 struct softmix_channel {
 	/*! Lock to protect this structure */
@@ -169,6 +173,12 @@
 	struct video_follow_talker_data video_talker;
 	/*! The ideal stream topology for the channel */
 	struct ast_stream_topology *topology;
+	/*! The latest REMB report from this participant */
+	struct ast_rtp_rtcp_feedback_remb remb;
+	/*! The REMB collector for this channel, collects REMB from all video receivers */
+	struct softmix_remb_collector *remb_collector;
+	/*! The bridge streams which are feeding us video sources */
+	AST_VECTOR(, int) video_sources;
 };
 
 struct softmix_bridge_data {
@@ -202,6 +212,10 @@
 	unsigned int binaural_init;
 	/*! The last time a video update was sent into the bridge */
 	struct timeval last_video_update;
+	/*! The last time a REMB frame was sent to each source of video */
+	struct timeval last_remb_update;
+	/*! Per-bridge stream REMB collectors, which flow back to video source */
+	AST_VECTOR(, struct softmix_remb_collector *) remb_collectors;
 };
 
 struct softmix_mixing_array {
diff --git a/configs/samples/confbridge.conf.sample b/configs/samples/confbridge.conf.sample
index 4028593..8b276cd 100644
--- a/configs/samples/confbridge.conf.sample
+++ b/configs/samples/confbridge.conf.sample
@@ -239,6 +239,10 @@
                            ; A REMB frame contains receiver estimated maximum bitrate information. By creating a combined
                            ; frame and sending it to the sources of video the sender can be influenced on what bitrate
                            ; they choose allowing a better experience for the receivers. This defaults to 0, or disabled.
+;remb_behavior=average     ; How the combined REMB report for an SFU video bridge is constructed. If set to "average" then
+                           ; the estimated maximum bitrate of each receiver is used to construct an average bitrate. If
+                           ; set to "lowest" the lowest maximum bitrate is forwarded to the sender. If set to "highest"
+                           ; the highest maximum bitrate is forwarded to the sender. This defaults to "average".
 
 ; All sounds in the conference are customizable using the bridge profile options below.
 ; Simply state the option followed by the filename or full path of the filename after
diff --git a/include/asterisk/bridge.h b/include/asterisk/bridge.h
index c96cefb..3584085 100644
--- a/include/asterisk/bridge.h
+++ b/include/asterisk/bridge.h
@@ -126,6 +126,24 @@
 	struct ast_channel *chan_old_vsrc;
 };
 
+/*! \brief REMB report behaviors */
+enum ast_bridge_video_sfu_remb_behavior {
+	/*! The average of all reports is sent to the sender */
+	AST_BRIDGE_VIDEO_SFU_REMB_AVERAGE = 0,
+	/*! The lowest reported bitrate is forwarded to the sender */
+	AST_BRIDGE_VIDEO_SFU_REMB_LOWEST,
+	/*! The highest reported bitrate is forwarded to the sender */
+	AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST,
+};
+
+/*! \brief This is used for selective forwarding unit configuration */
+struct ast_bridge_video_sfu_data {
+	/*! The interval at which a REMB report is generated and sent */
+	unsigned int remb_send_interval;
+	/*! How the combined REMB report is generated */
+	enum ast_bridge_video_sfu_remb_behavior remb_behavior;
+};
+
 /*! \brief Data structure that defines a video source mode */
 struct ast_bridge_video_mode {
 	enum ast_bridge_video_mode_type mode;
@@ -133,9 +151,10 @@
 	union {
 		struct ast_bridge_video_single_src_data single_src_data;
 		struct ast_bridge_video_talker_src_data talker_src_data;
+		struct ast_bridge_video_sfu_data sfu_data;
 	} mode_data;
+	/*! The minimum interval between video updates */
 	unsigned int video_update_discard;
-	unsigned int remb_send_interval;
 };
 
 /*!
@@ -917,10 +936,22 @@
  *
  * \param bridge Bridge to set the REMB send interval on
  * \param remb_send_interval The REMB send interval
+ *
+ * \note This can only be called when the bridge has been set to the SFU video mode.
  */
 void ast_bridge_set_remb_send_interval(struct ast_bridge *bridge, unsigned int remb_send_interval);
 
 /*!
+ * \brief Set the REMB report generation behavior on a bridge
+ *
+ * \param bridge Bridge to set the REMB behavior on
+ * \param behavior How REMB reports are generated
+ *
+ * \note This can only be called when the bridge has been set to the SFU video mode.
+ */
+void ast_brige_set_remb_behavior(struct ast_bridge *bridge, enum ast_bridge_video_sfu_remb_behavior behavior);
+
+/*!
  * \brief Update information about talker energy for talker src video mode.
  */
 void ast_bridge_update_talker_src_video_mode(struct ast_bridge *bridge, struct ast_channel *chan, int talker_energy, int is_keyfame);
diff --git a/main/bridge.c b/main/bridge.c
index 8795081..2b347fd 100644
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -3852,8 +3852,19 @@
 
 void ast_bridge_set_remb_send_interval(struct ast_bridge *bridge, unsigned int remb_send_interval)
 {
+	ast_assert(bridge->softmix.video_mode.mode == AST_BRIDGE_VIDEO_MODE_SFU);
+
 	ast_bridge_lock(bridge);
-	bridge->softmix.video_mode.remb_send_interval = remb_send_interval;
+	bridge->softmix.video_mode.mode_data.sfu_data.remb_send_interval = remb_send_interval;
+	ast_bridge_unlock(bridge);
+}
+
+void ast_brige_set_remb_behavior(struct ast_bridge *bridge, enum ast_bridge_video_sfu_remb_behavior behavior)
+{
+	ast_assert(bridge->softmix.video_mode.mode == AST_BRIDGE_VIDEO_MODE_SFU);
+
+	ast_bridge_lock(bridge);
+	bridge->softmix.video_mode.mode_data.sfu_data.remb_behavior = behavior;
 	ast_bridge_unlock(bridge);
 }
 

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9eafe4e7c1f72d67074a8d6acb26bfcf19322b66
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 3
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Matthew Fredrickson <creslin 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/20180418/59e341f9/attachment-0001.html>


More information about the asterisk-code-review mailing list