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

[Joshua Colp]

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

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(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index 31b7226..a0b1474 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -440,21 +440,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)
 {
@@ -470,8 +455,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);
@@ -498,8 +481,6 @@
 		return;
 	}
 	bridge_channel->tech_pvt = NULL;
-
-	softmix_src_change(bridge_channel);
 
 	/* Drop mutex lock */
 	ast_mutex_destroy(&sc->lock);
@@ -694,6 +675,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/4314
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I27fec098b32e7c8718b4b65f3fd5fa73527968bf
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>