[Asterisk-code-review] app confbridge: Use the SDP 'label' attribute to correlate ... (asterisk[15])

George Joseph asteriskteam at digium.com
Tue Jul 10 13:34:02 CDT 2018


George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/9377


Change subject: app_confbridge:  Use the SDP 'label' attribute to correlate users
......................................................................

app_confbridge:  Use the SDP 'label' attribute to correlate users

Previously, the msid "label" attribute was used to correlate
participant info but because streams could be reused, the msid
wasn't being updated correctly when someone left the bridge and
another joined.

Now, instead of looking for the msid attribute on a channel's streams,
app_confbridge sets an "SDP:LABEL" attribute on the stream which
res_pjsip_sdp_rtp looks for.  If it finds it, it adds a "label"
attribute to the current sdp.

Change-Id: I6cbaa87fb59a2e0688d956e72d2d09e4ac20d5a5
---
M apps/confbridge/confbridge_manager.c
M res/res_pjsip_sdp_rtp.c
2 files changed, 18 insertions(+), 43 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/77/9377/1

diff --git a/apps/confbridge/confbridge_manager.c b/apps/confbridge/confbridge_manager.c
index a101530..51112ba 100644
--- a/apps/confbridge/confbridge_manager.c
+++ b/apps/confbridge/confbridge_manager.c
@@ -388,34 +388,18 @@
 	return NULL;
 }
 
-static struct ast_json *get_media_labels(struct confbridge_conference *conference,
+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;
-	const char *curr_a_label;
-	const char *a_label = NULL;
-	const char *v_label = NULL;
-	struct ast_json *labels = ast_json_array_create();
+	struct ast_channel *chan = dir == LABEL_DIRECTION_SRC ? dest_chan : src_chan;
 
-	if (!labels) {
-		return NULL;
-	}
-
-	topology = ast_channel_get_stream_topology(dir == LABEL_DIRECTION_SRC ? src_chan : dest_chan);
-	stream = get_stream(topology, AST_MEDIA_TYPE_AUDIO);
-	curr_a_label = stream ? ast_stream_get_metadata(stream, "MSID:LABEL") : NULL;
-	a_label = curr_a_label ?: conference->bridge->uniqueid;
-	ast_json_array_append(labels, ast_json_string_create(a_label));
-
-	topology = ast_channel_get_stream_topology(dir == LABEL_DIRECTION_SRC ? dest_chan : src_chan);
+	topology = ast_channel_get_stream_topology(chan);
 	stream = get_stream(topology, AST_MEDIA_TYPE_VIDEO);
-	v_label = stream ? ast_stream_get_metadata(stream, "MSID:LABEL") : NULL;
-	if (v_label) {
-		ast_json_array_append(labels, ast_json_string_create(v_label));
+	if (stream) {
+		ast_stream_set_metadata(stream, "SDP:LABEL", ast_channel_uniqueid(chan));
 	}
-
-	return ast_json_pack("{s: o }", "media_source_track_labels", labels);
 }
 
 static void send_message(const char *msg_name, char *conf_name, struct ast_json *json_object,
@@ -505,7 +489,6 @@
 	ao2_lock(conference);
 	AST_LIST_TRAVERSE(&conference->active_list, user, list) {
 		struct ast_json *json_object;
-		struct ast_json* source_json_labels = NULL;
 
 		/*
 		 * If the msg type is join, we need to capture all targets channel info so we can
@@ -514,7 +497,6 @@
 		if (source_send_events && stasis_message_type(msg) == confbridge_join_type()) {
 			struct ast_channel_snapshot *target_snapshot;
 			struct ast_json *target_json_channel;
-			struct ast_json *target_json_labels;
 
 			target_snapshot = ast_channel_snapshot_get_latest(ast_channel_uniqueid(user->chan));
 			if (!target_snapshot) {
@@ -523,17 +505,15 @@
 				continue;
 			}
 
-			target_json_labels = get_media_labels(conference, chan, user->chan, LABEL_DIRECTION_SRC);
-			target_json_channel = channel_to_json(target_snapshot, extras, target_json_labels);
+			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);
-			ast_json_unref(target_json_labels);
 
 			if (!json_channels) {
 				json_channels = ast_json_array_create();
 				if (!json_channels) {
 					ast_log(LOG_ERROR, "Unable to allocate json array\n");
 					ast_json_unref(target_json_channel);
-					ast_json_unref(target_json_labels);
 					return;
 				}
 			}
@@ -555,11 +535,9 @@
 			continue;
 		}
 
-		source_json_labels = get_media_labels(conference, chan, user->chan, LABEL_DIRECTION_DEST);
-		ast_json_object_update(extras, source_json_labels);
+		set_media_labels(conference, chan, user->chan, LABEL_DIRECTION_DEST);
 
-		json_object = pack_snapshots(obj->bridge, obj->channel, extras, source_json_labels, msg);
-		ast_json_unref(source_json_labels);
+		json_object = pack_snapshots(obj->bridge, obj->channel, extras, NULL, msg);
 
 		if (!json_object) {
 			ast_log(LOG_ERROR, "Unable to convert %s message to json\n", msg_name);
diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index c9c0f17..d435d8f 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1100,6 +1100,7 @@
 	pj_str_t stmp;
 	pjmedia_sdp_attr *attr;
 	char msid[(AST_UUID_STR_LEN * 2) + 2];
+	const char *stream_label = ast_stream_get_metadata(stream, "SDP:LABEL");
 
 	if (!session->endpoint->media.webrtc) {
 		return;
@@ -1119,19 +1120,7 @@
 	}
 
 	if (ast_strlen_zero(session_media->label)) {
-		/*
-		 * If this stream has already been assigned a label, use it.
-		 * This will ensure that a confbridge participant is known by
-		 * the same label by all other participants.
-		 */
-		const char *stream_label = ast_stream_get_metadata(stream, "MSID:LABEL");
-
-		if (!ast_strlen_zero(stream_label)) {
-			ast_copy_string(session_media->label, stream_label, sizeof(session_media->label));
-		} else {
 			ast_uuid_generate_str(session_media->label, sizeof(session_media->label));
-			ast_stream_set_metadata(stream, "MSID:LABEL", session_media->label);
-		}
 	}
 
 	snprintf(msid, sizeof(msid), "%s %s", session_media->mslabel, session_media->label);
@@ -1139,6 +1128,14 @@
 		ast_codec_media_type2str(ast_stream_get_type(stream)), msid);
 	attr = pjmedia_sdp_attr_create(pool, "msid", pj_cstr(&stmp, msid));
 	pjmedia_sdp_attr_add(&media->attr_count, media->attr, attr);
+
+	/* 'label' must come after 'msid' */
+	if (!ast_strlen_zero(stream_label)) {
+		ast_debug(3, "Stream Label: %p %s %s\n", stream,
+			ast_codec_media_type2str(ast_stream_get_type(stream)), stream_label);
+		attr = pjmedia_sdp_attr_create(pool, "label", pj_cstr(&stmp, stream_label));
+		pjmedia_sdp_attr_add(&media->attr_count, media->attr, attr);
+	}
 }
 
 static void add_rtcp_fb_to_stream(struct ast_sip_session *session,

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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6cbaa87fb59a2e0688d956e72d2d09e4ac20d5a5
Gerrit-Change-Number: 9377
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180710/419926e1/attachment-0001.html>


More information about the asterisk-code-review mailing list