[Asterisk-code-review] bridges/bridge softmix: Remove SSRC changes on join/leave; u... (asterisk[14])

[Matt Jordan]

Matt Jordan has uploaded a new change for review. ( https://gerrit.asterisk.org/4304 )

Change subject: bridges/bridge_softmix: Remove SSRC changes on join/leave; update video source
......................................................................

bridges/bridge_softmix: Remove SSRC changes on join/leave; update video source

WebRTC clients really, really want to know the SSRC of the media they're
getting. Changing the SSRC is generally not a good thing.

bridge_softmix, starting in Asterisk 12, started changing the SSRC of
parties as they joined or left the bridge. With most phones, this isn't
a problem: phones just play back the stream they're getting. With WebRTC
clients, however, the SSRC is tied to a media stream that may be
negotiated. When a new SSRC just shows up, the media can be dropped.

As it turns out, the SSRC change shouldn't even be necessary. From the
perspective of the client, it's still talking to Asterisk with the same
media stream: why indicate that the far party has suddenly changed to a
different source of media?

This patch opts to just remove the SSRC changes. With this patch, video
clients that join/leave a softmix bridge actually get the video stream
instead of freaking out.

ASTERISK-26555

Change-Id: I27fec098b32e7c8718b4b65f3fd5fa73527968bf
---
M bridges/bridge_softmix.c
1 file changed, 9 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/04/4304/1

diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index 0991e28..1c35a23 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -442,21 +442,6 @@
 	}
 }
 
-/*!
- * \internal
- * \brief Indicate a source change to the channel.
- * \since 12.0.0
- *
- * \param bridge_channel Which channel source is changing.
- *
- * \retval 0 on success.
- * \retval -1 on error.
- */
-static int softmix_src_change(struct ast_bridge_channel *bridge_channel)
-{
-	return ast_bridge_channel_queue_control_data(bridge_channel, AST_CONTROL_SRCCHANGE, NULL, 0);
-}
-
 /*! \brief Function called when a channel is joined into the bridge */
 static int softmix_bridge_join(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
 {
@@ -472,8 +457,6 @@
 	if (!(sc = ast_calloc(1, sizeof(*sc)))) {
 		return -1;
 	}
-
-	softmix_src_change(bridge_channel);
 
 	/* Can't forget the lock */
 	ast_mutex_init(&sc->lock);
@@ -500,8 +483,6 @@
 		return;
 	}
 	bridge_channel->tech_pvt = NULL;
-
-	softmix_src_change(bridge_channel);
 
 	/* Drop mutex lock */
 	ast_mutex_destroy(&sc->lock);
@@ -696,6 +677,15 @@
 	 * XXX Softmix needs to use channel roles to determine what to
 	 * do with control frames.
 	 */
+
+	switch (frame->subclass.integer) {
+	case AST_CONTROL_VIDUPDATE:
+		ast_bridge_queue_everyone_else(bridge, NULL, frame);
+		break;
+	default:
+		break;
+	}
+
 	return 0;
 }
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I27fec098b32e7c8718b4b65f3fd5fa73527968bf
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Matt Jordan <mjordan at digium.com>