[asterisk-commits] rmudgett: branch 10 r365320 - in /branches/10: ./ channels/chan_local.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri May 4 11:29:10 CDT 2012


Author: rmudgett
Date: Fri May  4 11:28:06 2012
New Revision: 365320

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=365320
Log:
Fix local channel chains optimizing themselves out of a call.

* Made chan_local.c:check_bridge() check the return value of
ast_channel_masquerade().  In long chains of local channels, the
masquerade occasionally fails to get setup because there is another
masquerade already setup on an adjacent local channel in the chain.

* Made the outgoing local channel (the ;2 channel) flush one voice or
video frame per optimization attempt.

* Made sure that the outgoing local channel also does not have any frames
in its queue before the masquerade.

* Made do the masquerade immediately to minimize the chance that the
outgoing channel queue does not get any new frames added and thus
unconditionally flushed.

* Made block indication -1 (Stop tones) event when the local channel is
going to optimize itself out.  When the call is answered, a chain of local
channels pass down a -1 indication for each bridge.  This blizzard of -1
events really slows down the optimization process.

(closes issue ASTERISK-16711)
Reported by: Alec Davis
Tested by: rmudgett, Alec Davis
Review: https://reviewboard.asterisk.org/r/1894/
........

Merged revisions 365313 from http://svn.asterisk.org/svn/asterisk/branches/1.8

Modified:
    branches/10/   (props changed)
    branches/10/channels/chan_local.c

Propchange: branches/10/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.

Modified: branches/10/channels/chan_local.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/channels/chan_local.c?view=diff&rev=365320&r1=365319&r2=365320
==============================================================================
--- branches/10/channels/chan_local.c (original)
+++ branches/10/channels/chan_local.c Fri May  4 11:28:06 2012
@@ -475,17 +475,16 @@
  *
  * \note it is assummed p is locked and reffed before entering this function
  */
-static void check_bridge(struct local_pvt *p)
-{
-	struct ast_channel_monitor *tmp;
-	struct ast_channel *chan = NULL;
-	struct ast_channel *bridged_chan = NULL;
+static void check_bridge(struct ast_channel *ast, struct local_pvt *p)
+{
+	struct ast_channel *owner;
+	struct ast_channel *chan;
+	struct ast_channel *bridged_chan;
+	struct ast_frame *f;
 
 	/* Do a few conditional checks early on just to see if this optimization is possible */
-	if (ast_test_flag(p, LOCAL_NO_OPTIMIZATION)) {
-		return;
-	}
-	if (ast_test_flag(p, LOCAL_ALREADY_MASQED) || !p->chan || !p->owner) {
+	if (ast_test_flag(p, LOCAL_NO_OPTIMIZATION | LOCAL_ALREADY_MASQED)
+		|| !p->chan || !p->owner) {
 		return;
 	}
 
@@ -501,7 +500,9 @@
 	/* since we had to unlock p to get the bridged chan, validate our
 	 * data once again and verify the bridged channel is what we expect
 	 * it to be in order to perform this optimization */
-	if (ast_test_flag(p, LOCAL_ALREADY_MASQED) || !p->owner || !p->chan || (p->chan->_bridge != bridged_chan)) {
+	if (ast_test_flag(p, LOCAL_NO_OPTIMIZATION | LOCAL_ALREADY_MASQED)
+		|| !p->chan || !p->owner
+		|| (p->chan->_bridge != bridged_chan)) {
 		return;
 	}
 
@@ -510,73 +511,114 @@
 	   frames on the owner channel (because they would be transferred to the
 	   outbound channel during the masquerade)
 	*/
-	if (p->chan->_bridge /* Not ast_bridged_channel!  Only go one step! */ && AST_LIST_EMPTY(&p->owner->readq)) {
-		/* Masquerade bridged channel into owner */
-		/* Lock everything we need, one by one, and give up if
-		   we can't get everything.  Remember, we'll get another
-		   chance in just a little bit */
-		if (!ast_channel_trylock(p->chan->_bridge)) {
-			if (!ast_check_hangup(p->chan->_bridge)) {
-				if (!ast_channel_trylock(p->owner)) {
-					if (!ast_check_hangup(p->owner)) {
-						if (p->owner->monitor && !p->chan->_bridge->monitor) {
-							/* If a local channel is being monitored, we don't want a masquerade
-							 * to cause the monitor to go away. Since the masquerade swaps the monitors,
-							 * pre-swapping the monitors before the masquerade will ensure that the monitor
-							 * ends up where it is expected.
-							 */
-							tmp = p->owner->monitor;
-							p->owner->monitor = p->chan->_bridge->monitor;
-							p->chan->_bridge->monitor = tmp;
-						}
-						if (p->chan->audiohooks) {
-							struct ast_audiohook_list *audiohooks_swapper;
-							audiohooks_swapper = p->chan->audiohooks;
-							p->chan->audiohooks = p->owner->audiohooks;
-							p->owner->audiohooks = audiohooks_swapper;
-						}
-
-						/* If any Caller ID was set, preserve it after masquerade like above. We must check
-						 * to see if Caller ID was set because otherwise we'll mistakingly copy info not
-						 * set from the dialplan and will overwrite the real channel Caller ID. The reason
-						 * for this whole preswapping action is because the Caller ID is set on the channel
-						 * thread (which is the to be masqueraded away local channel) before both local
-						 * channels are optimized away.
-						 */
-						if (p->owner->caller.id.name.valid || p->owner->caller.id.number.valid
-							|| p->owner->caller.id.subaddress.valid || p->owner->caller.ani.name.valid
-							|| p->owner->caller.ani.number.valid || p->owner->caller.ani.subaddress.valid) {
-							struct ast_party_caller tmp;
-							tmp = p->owner->caller;
-							p->owner->caller = p->chan->_bridge->caller;
-							p->chan->_bridge->caller = tmp;
-						}
-						if (p->owner->redirecting.from.name.valid || p->owner->redirecting.from.number.valid
-							|| p->owner->redirecting.from.subaddress.valid || p->owner->redirecting.to.name.valid
-							|| p->owner->redirecting.to.number.valid || p->owner->redirecting.to.subaddress.valid) {
-							struct ast_party_redirecting tmp;
-							tmp = p->owner->redirecting;
-							p->owner->redirecting = p->chan->_bridge->redirecting;
-							p->chan->_bridge->redirecting = tmp;
-						}
-						if (p->owner->dialed.number.str || p->owner->dialed.subaddress.valid) {
-							struct ast_party_dialed tmp;
-							tmp = p->owner->dialed;
-							p->owner->dialed = p->chan->_bridge->dialed;
-							p->chan->_bridge->dialed = tmp;
-						}
-
-
-						ast_app_group_update(p->chan, p->owner);
-						ast_channel_masquerade(p->owner, p->chan->_bridge);
-						ast_set_flag(p, LOCAL_ALREADY_MASQED);
-					}
-					ast_channel_unlock(p->owner);
-				}
-			}
-			ast_channel_unlock(p->chan->_bridge);
-		}
-	}
+	if (!p->chan->_bridge /* Not ast_bridged_channel!  Only go one step! */
+		|| !AST_LIST_EMPTY(&p->owner->readq)
+		|| ast != p->chan /* Sanity check (should always be false) */) {
+		return;
+	}
+
+	/* Masquerade bridged channel into owner */
+	/* Lock everything we need, one by one, and give up if
+	   we can't get everything.  Remember, we'll get another
+	   chance in just a little bit */
+	if (ast_channel_trylock(p->chan->_bridge)) {
+		return;
+	}
+	if (ast_check_hangup(p->chan->_bridge) || ast_channel_trylock(p->owner)) {
+		ast_channel_unlock(p->chan->_bridge);
+		return;
+	}
+
+	/*
+	 * At this point we have 4 locks:
+	 * p, p->chan (same as ast), p->chan->_bridge, p->owner
+	 *
+	 * Flush a voice or video frame on the outbound channel to make
+	 * the queue empty faster so we can get optimized out.
+	 */
+	f = AST_LIST_FIRST(&p->chan->readq);
+	if (f && (f->frametype == AST_FRAME_VOICE || f->frametype == AST_FRAME_VIDEO)) {
+		AST_LIST_REMOVE_HEAD(&p->chan->readq, frame_list);
+		ast_frfree(f);
+		f = AST_LIST_FIRST(&p->chan->readq);
+	}
+
+	if (f
+		|| ast_check_hangup(p->owner)
+		|| ast_channel_masquerade(p->owner, p->chan->_bridge)) {
+		ast_channel_unlock(p->owner);
+		ast_channel_unlock(p->chan->_bridge);
+		return;
+	}
+
+	/* Masquerade got setup. */
+	ast_debug(4, "Masquerading %s <- %s\n",
+		p->owner->name, p->chan->_bridge->name);
+	if (p->owner->monitor && !p->chan->_bridge->monitor) {
+		struct ast_channel_monitor *tmp;
+
+		/* If a local channel is being monitored, we don't want a masquerade
+		 * to cause the monitor to go away. Since the masquerade swaps the monitors,
+		 * pre-swapping the monitors before the masquerade will ensure that the monitor
+		 * ends up where it is expected.
+		 */
+		tmp = p->owner->monitor;
+		p->owner->monitor = p->chan->_bridge->monitor;
+		p->chan->_bridge->monitor = tmp;
+	}
+	if (p->chan->audiohooks) {
+		struct ast_audiohook_list *audiohooks_swapper;
+		audiohooks_swapper = p->chan->audiohooks;
+		p->chan->audiohooks = p->owner->audiohooks;
+		p->owner->audiohooks = audiohooks_swapper;
+	}
+
+	/* If any Caller ID was set, preserve it after masquerade like above. We must check
+	 * to see if Caller ID was set because otherwise we'll mistakingly copy info not
+	 * set from the dialplan and will overwrite the real channel Caller ID. The reason
+	 * for this whole preswapping action is because the Caller ID is set on the channel
+	 * thread (which is the to be masqueraded away local channel) before both local
+	 * channels are optimized away.
+	 */
+	if (p->owner->caller.id.name.valid || p->owner->caller.id.number.valid
+		|| p->owner->caller.id.subaddress.valid || p->owner->caller.ani.name.valid
+		|| p->owner->caller.ani.number.valid || p->owner->caller.ani.subaddress.valid) {
+		struct ast_party_caller tmp;
+
+		tmp = p->owner->caller;
+		p->owner->caller = p->chan->_bridge->caller;
+		p->chan->_bridge->caller = tmp;
+	}
+	if (p->owner->redirecting.from.name.valid || p->owner->redirecting.from.number.valid
+		|| p->owner->redirecting.from.subaddress.valid || p->owner->redirecting.to.name.valid
+		|| p->owner->redirecting.to.number.valid || p->owner->redirecting.to.subaddress.valid) {
+		struct ast_party_redirecting tmp;
+
+		tmp = p->owner->redirecting;
+		p->owner->redirecting = p->chan->_bridge->redirecting;
+		p->chan->_bridge->redirecting = tmp;
+	}
+	if (p->owner->dialed.number.str || p->owner->dialed.subaddress.valid) {
+		struct ast_party_dialed tmp;
+
+		tmp = p->owner->dialed;
+		p->owner->dialed = p->chan->_bridge->dialed;
+		p->chan->_bridge->dialed = tmp;
+	}
+	ast_app_group_update(p->chan, p->owner);
+	ast_set_flag(p, LOCAL_ALREADY_MASQED);
+
+	ast_channel_unlock(p->owner);
+	ast_channel_unlock(p->chan->_bridge);
+
+	/* Do the masquerade now. */
+	owner = ast_channel_ref(p->owner);
+	ao2_unlock(p);
+	ast_channel_unlock(ast);
+	ast_do_masquerade(owner);
+	ast_channel_unref(owner);
+	ast_channel_lock(ast);
+	ao2_lock(p);
 }
 
 static struct ast_frame  *local_read(struct ast_channel *ast)
@@ -599,14 +641,16 @@
 	ao2_lock(p);
 	isoutbound = IS_OUTBOUND(ast, p);
 
-	if (isoutbound && f && (f->frametype == AST_FRAME_VOICE || f->frametype == AST_FRAME_VIDEO)) {
-		check_bridge(p);
+	if (isoutbound
+		&& (f->frametype == AST_FRAME_VOICE || f->frametype == AST_FRAME_VIDEO)) {
+		check_bridge(ast, p);
 	}
 
 	if (!ast_test_flag(p, LOCAL_ALREADY_MASQED)) {
 		res = local_queue_frame(p, isoutbound, f, ast, 1);
 	} else {
-		ast_debug(1, "Not posting to queue since already masked on '%s'\n", ast->name);
+		ast_debug(1, "Not posting to '%s' queue since already masqueraded out\n",
+			ast->name);
 		res = 0;
 	}
 	ao2_unlock(p);
@@ -700,11 +744,20 @@
 	} else {
 		/* Queue up a frame representing the indication as a control frame */
 		ao2_lock(p);
-		isoutbound = IS_OUTBOUND(ast, p);
-		f.subclass.integer = condition;
-		f.data.ptr = (void*)data;
-		f.datalen = datalen;
-		res = local_queue_frame(p, isoutbound, &f, ast, 1);
+		/*
+		 * Block -1 stop tones events if we are to be optimized out.  We
+		 * don't need a flurry of these events on a local channel chain
+		 * when initially connected to slow the optimization process.
+		 */
+		if (0 <= condition || ast_test_flag(p, LOCAL_NO_OPTIMIZATION)) {
+			isoutbound = IS_OUTBOUND(ast, p);
+			f.subclass.integer = condition;
+			f.data.ptr = (void *) data;
+			f.datalen = datalen;
+			res = local_queue_frame(p, isoutbound, &f, ast, 1);
+		} else {
+			ast_debug(4, "Blocked indication %d\n", condition);
+		}
 		ao2_unlock(p);
 	}
 




More information about the asterisk-commits mailing list