[Asterisk-code-review] bridge softmix: Add SDP "label" attribute to streams (asterisk[master])

Joshua Colp asteriskteam at digium.com
Thu Oct 25 07:45:24 CDT 2018


Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/10485 )

Change subject: bridge_softmix:  Add SDP "label" attribute to streams
......................................................................

bridge_softmix:  Add SDP "label" attribute to streams

Adding the "label" attribute used for participant info correlation
was previously done in app_confbridge but it wasn't working
correctly because it didn't have knowledge about which video
streams belonged to which channel.  Only bridge_softmix has that
data so now it's set when the bridge topology is changed.

ASTERISK-28107

Change-Id: Ieddeca5799d710cad083af3fcc3e677fa2a2a499
---
M apps/app_confbridge.c
M apps/confbridge/confbridge_manager.c
M bridges/bridge_softmix.c
M include/asterisk/bridge.h
M main/bridge.c
5 files changed, 55 insertions(+), 63 deletions(-)

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



diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index edb7e03..5936400 100644
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -1568,6 +1568,10 @@
 			}
 		}
 
+		if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_ENABLE_EVENTS)) {
+			ast_bridge_set_send_sdp_label(conference->bridge, 1);
+		}
+
 		/* Link it into the conference bridges container */
 		if (!ao2_link(conference_bridges, conference)) {
 			ao2_ref(conference, -1);
diff --git a/apps/confbridge/confbridge_manager.c b/apps/confbridge/confbridge_manager.c
index a7f2fce..e88bbc2 100644
--- a/apps/confbridge/confbridge_manager.c
+++ b/apps/confbridge/confbridge_manager.c
@@ -355,56 +355,6 @@
 	return pack_bridge_and_channels(json_bridge, json_channel, msg);
 }
 
-enum label_direction {
-	LABEL_DIRECTION_SRC,
-	LABEL_DIRECTION_DEST,
-};
-
-static struct ast_stream *get_stream(struct ast_stream_topology *topology,
-	enum ast_media_type m_type)
-{
-	int count;
-	int i;
-
-	count = ast_stream_topology_get_count(topology);
-	if (count < 0) {
-		return NULL;
-	}
-
-	for (i = 0; i < count; i++) {
-		struct ast_stream *s;
-		enum ast_stream_state s_state;
-		enum ast_media_type s_type;
-
-		s = ast_stream_topology_get_stream(topology, i);
-		s_state = ast_stream_get_state(s);
-		s_type = ast_stream_get_type(s);
-		if (s_type == m_type
-			&& (s_state == AST_STREAM_STATE_SENDRECV || s_state == AST_STREAM_STATE_RECVONLY)) {
-			return s;
-		}
-	}
-
-	return NULL;
-}
-
-static void set_media_labels(struct confbridge_conference *conference,
-	struct ast_channel *src_chan, struct ast_channel *dest_chan, enum label_direction dir)
-{
-	struct ast_stream_topology *topology;
-	struct ast_stream *stream;
-	struct ast_channel *chan = dir == LABEL_DIRECTION_SRC ? dest_chan : src_chan;
-
-	if (!chan) {
-		return;
-	}
-	topology = ast_channel_get_stream_topology(chan);
-	stream = get_stream(topology, AST_MEDIA_TYPE_VIDEO);
-	if (stream) {
-		ast_stream_set_metadata(stream, "SDP:LABEL", ast_channel_uniqueid(chan));
-	}
-}
-
 static void send_message(const char *msg_name, char *conf_name, struct ast_json *json_object,
 	struct ast_channel *chan)
 {
@@ -508,7 +458,6 @@
 				continue;
 			}
 
-			set_media_labels(conference, chan, user->chan, LABEL_DIRECTION_SRC);
 			target_json_channel = channel_to_json(target_snapshot, extras, NULL);
 			ao2_ref(target_snapshot, -1);
 
@@ -538,8 +487,6 @@
 			continue;
 		}
 
-		set_media_labels(conference, chan, user->chan, LABEL_DIRECTION_DEST);
-
 		json_object = pack_snapshots(obj->bridge, obj->channel, extras, NULL, msg);
 
 		if (!json_object) {
diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index b11fccc..cf61340 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -498,7 +498,7 @@
 }
 
 static int append_source_streams(struct ast_stream_topology *dest,
-	const char *channel_name,
+	const char *channel_name, const char *sdp_label,
 	const struct ast_stream_topology *source)
 {
 	int i;
@@ -523,6 +523,12 @@
 		if (!stream_clone) {
 			return -1;
 		}
+
+		/* Sends an "a:label" attribute in the SDP for participant event correlation */
+		if (!ast_strlen_zero(sdp_label)) {
+			ast_stream_set_metadata(stream_clone, "SDP:LABEL", sdp_label);
+		}
+
 		if (ast_stream_topology_append_stream(dest, stream_clone) < 0) {
 			ast_stream_free(stream_clone);
 			return -1;
@@ -585,9 +591,11 @@
  * \param joiner The channel that is joining the softmix bridge
  * \param participants The current participants in the softmix bridge
  */
-static void sfu_topologies_on_join(struct ast_bridge_channel *joiner, struct ast_bridge_channels_list *participants)
+static void sfu_topologies_on_join(struct ast_bridge *bridge,
+	struct ast_bridge_channel *joiner)
 {
 	struct ast_stream_topology *joiner_video = NULL;
+	struct ast_bridge_channels_list *participants = &bridge->channels;
 	struct ast_bridge_channel *participant;
 	int res;
 	struct softmix_channel *sc;
@@ -600,7 +608,9 @@
 	sc = joiner->tech_pvt;
 
 	ast_channel_lock(joiner->chan);
-	res = append_source_streams(joiner_video, ast_channel_name(joiner->chan), ast_channel_get_stream_topology(joiner->chan));
+	res = append_source_streams(joiner_video, ast_channel_name(joiner->chan),
+		bridge->softmix.send_sdp_label ? ast_channel_uniqueid(joiner->chan) : NULL,
+		ast_channel_get_stream_topology(joiner->chan));
 	sc->topology = ast_stream_topology_clone(ast_channel_get_stream_topology(joiner->chan));
 	ast_channel_unlock(joiner->chan);
 
@@ -614,7 +624,8 @@
 		}
 		ast_channel_lock(participant->chan);
 		res = append_source_streams(sc->topology, ast_channel_name(participant->chan),
-				ast_channel_get_stream_topology(participant->chan));
+			bridge->softmix.send_sdp_label ? ast_channel_uniqueid(participant->chan) : NULL,
+			ast_channel_get_stream_topology(participant->chan));
 		ast_channel_unlock(participant->chan);
 		if (res) {
 			goto cleanup;
@@ -704,7 +715,7 @@
 		bridge_channel, 0, set_binaural, pos_id, is_announcement);
 
 	if (bridge->softmix.video_mode.mode == AST_BRIDGE_VIDEO_MODE_SFU) {
-		sfu_topologies_on_join(bridge_channel, &bridge->channels);
+		sfu_topologies_on_join(bridge, bridge_channel);
 	}
 
 	softmix_poke_thread(softmix_data);
@@ -1063,9 +1074,11 @@
 	return 0;
 }
 
-static void sfu_topologies_on_source_change(struct ast_bridge_channel *source, struct ast_bridge_channels_list *participants)
+static void sfu_topologies_on_source_change(struct ast_bridge *bridge,
+	struct ast_bridge_channel *source)
 {
 	struct ast_stream_topology *source_video = NULL;
+	struct ast_bridge_channels_list *participants = &bridge->channels;
 	struct ast_bridge_channel *participant;
 	int res;
 
@@ -1075,7 +1088,9 @@
 	}
 
 	ast_channel_lock(source->chan);
-	res = append_source_streams(source_video, ast_channel_name(source->chan), ast_channel_get_stream_topology(source->chan));
+	res = append_source_streams(source_video, ast_channel_name(source->chan),
+		bridge->softmix.send_sdp_label ? ast_channel_uniqueid(source->chan) : NULL,
+		ast_channel_get_stream_topology(source->chan));
 	ast_channel_unlock(source->chan);
 	if (res) {
 		goto cleanup;
@@ -1198,7 +1213,7 @@
 		break;
 	case AST_CONTROL_STREAM_TOPOLOGY_SOURCE_CHANGED:
 		if (bridge->softmix.video_mode.mode == AST_BRIDGE_VIDEO_MODE_SFU) {
-			sfu_topologies_on_source_change(bridge_channel, &bridge->channels);
+			sfu_topologies_on_source_change(bridge, bridge_channel);
 		}
 		break;
 	default:
@@ -2383,7 +2398,7 @@
 		goto end;
 	}
 
-	if (append_source_streams(topology_alice, "PJSIP/Bob-00000001", topology_bob)) {
+	if (append_source_streams(topology_alice, "PJSIP/Bob-00000001", NULL, topology_bob)) {
 		ast_test_status_update(test, "Failed to append Bob's streams to Alice\n");
 		goto end;
 	}
@@ -2402,7 +2417,7 @@
 		goto end;
 	}
 
-	if (append_source_streams(topology_bob, "PJSIP/Alice-00000000", topology_alice)) {
+	if (append_source_streams(topology_bob, "PJSIP/Alice-00000000", NULL, topology_alice)) {
 		ast_test_status_update(test, "Failed to append Alice's streams to Bob\n");
 		goto end;
 	}
diff --git a/include/asterisk/bridge.h b/include/asterisk/bridge.h
index 3584085..f4b1df8 100644
--- a/include/asterisk/bridge.h
+++ b/include/asterisk/bridge.h
@@ -290,6 +290,11 @@
 	unsigned int internal_mixing_interval;
 	/*! TRUE if binaural convolve is activated in configuration. */
 	unsigned int binaural_active;
+	/*!
+	 * Add a "label" attribute to each stream in the SDP containing
+	 * the channel uniqueid.  Used for participant info correlation.
+	 */
+	unsigned int send_sdp_label;
 };
 
 AST_LIST_HEAD_NOLOCK(ast_bridge_channels_list, ast_bridge_channel);
@@ -986,6 +991,20 @@
 const char *ast_bridge_video_mode_to_string(enum ast_bridge_video_mode_type video_mode);
 
 /*!
+ * \brief Controls whether to send a "label" attribute in each stream in an SDP
+ * \since 16.1.0
+ *
+ * \param bridge The bridge
+ * \param send_sdp_label Whether to send the labels or not
+ *
+ * \note The label will contain the uniqueid of the channel related to the stream.
+ * This is used to allow the recipient to correlate the stream to the participant
+ * information events sent by app_confbridge.
+ * The bridge will be locked in this function.
+ */
+void ast_bridge_set_send_sdp_label(struct ast_bridge *bridge, unsigned int send_sdp_label);
+
+/*!
  * \brief Acquire the channel's bridge for transfer purposes.
  * \since 13.21.0
  *
diff --git a/main/bridge.c b/main/bridge.c
index db49180..d6e7a51 100644
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -4018,6 +4018,13 @@
 	}
 }
 
+void ast_bridge_set_send_sdp_label(struct ast_bridge *bridge, unsigned int send_sdp_label)
+{
+	ast_bridge_lock(bridge);
+	bridge->softmix.send_sdp_label = send_sdp_label;
+	ast_bridge_unlock(bridge);
+}
+
 static int channel_hash(const void *obj, int flags)
 {
 	const struct ast_channel *chan = obj;

-- 
To view, visit https://gerrit.asterisk.org/10485
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieddeca5799d710cad083af3fcc3e677fa2a2a499
Gerrit-Change-Number: 10485
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Joshua Colp <jcolp 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/20181025/0c95da07/attachment-0001.html>


More information about the asterisk-code-review mailing list