[asterisk-commits] russell: branch 1.4 r89790 - in /branches/1.4/main: autoservice.c pbx.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Nov 27 15:45:51 CST 2007


Author: russell
Date: Tue Nov 27 15:45:51 2007
New Revision: 89790

URL: http://svn.digium.com/view/asterisk?view=rev&rev=89790
Log:
Merge changes from team/russell/autoservice_1.4

This set of changes fixes an issue that was reported to me on IRC yesterday.
The user, d1mas, was using chan_zap for incoming calls and was having DTMF
recognition issues in some situations.  Specifically, he noticed that the 
problem occurred when using DISA or WaitExten.  He also noticed that when 
using Read, the problem did not occur.  His system also used DUNDi for 
dialplan lookups.

So, he theorized that if the DUNDi lookups blocked for some period of time,
that audio from the zap channel could get lost.  If the audio got lost, then
it wouldn't be run through the DTMF detector, and digits could get lost.
He was correct, and the following set of changes fixes the problem.  However,
the changes go a little bit further than what was necessary to fix this exact
problem.

1) I updated pbx_extension_helper() to autoservice the associated channel to
   handle cases where extension lookups may take a long time.  This would
   normally be a dialplan switch that does some lookup over the network, such
   as the DUNDi or IAX2 switches.

   This ensures that even while a DUNDi lookup is blocking, the channel will be
   continuously serviced.

2) I made a change to the autoservice code.  This is actually something that
   has bothered me for a long time.  When a channel is in autoservice, _all_
   frames get thrown away.  However, some frames really shouldn't be thrown
   away.  The most notable examples are signalling (CONTROL) frames, and DTMF.

   So, this patch queues up important frames while a channel is in autoservice.
   When autoservice is stopped on the channel, the queued up frames get stuck
   back on the channel so that they can get processed instead of thrown away.

3) I made another change to the autoservice code to handle the case where
   autoservice is started on channels recursively.

   Previously, you could call ast_autoservice_start() multiple times on a
   channel, and it would stop the first time ast_autoservice_stop() gets
   called.  Now, it will ensure that autoservice doesn't actually stop until
   the final call to ast_autoservice_stop().

Modified:
    branches/1.4/main/autoservice.c
    branches/1.4/main/pbx.c

Modified: branches/1.4/main/autoservice.c
URL: http://svn.digium.com/view/asterisk/branches/1.4/main/autoservice.c?view=diff&rev=89790&r1=89789&r2=89790
==============================================================================
--- branches/1.4/main/autoservice.c (original)
+++ branches/1.4/main/autoservice.c Tue Nov 27 15:45:51 2007
@@ -54,6 +54,11 @@
 
 struct asent {
 	struct ast_channel *chan;
+	/*! This gets incremented each time autoservice gets started on the same
+	 *  channel.  It will ensure that it doesn't actually get stopped until 
+	 *  it gets stopped for the last time. */
+	unsigned int use_count;
+	AST_LIST_HEAD_NOLOCK(, ast_frame) dtmf_frames;
 	AST_LIST_ENTRY(asent) list;
 };
 
@@ -83,8 +88,42 @@
 
 		chan = ast_waitfor_n(mons, x, &ms);
 		if (chan) {
-			/* Read and ignore anything that occurs */
 			struct ast_frame *f = ast_read(chan);
+			
+			/* Do not add a default entry in this switch statement.  Each new
+			 * frame type should be addressed directly as to whether it should
+			 * be queued up or not. */
+			switch (f->frametype) {
+			/* Save these frames */
+			case AST_FRAME_DTMF_BEGIN:
+			case AST_FRAME_DTMF_END:
+			case AST_FRAME_CONTROL:
+			case AST_FRAME_TEXT:
+			case AST_FRAME_IMAGE:
+			case AST_FRAME_HTML:
+			{
+				struct ast_frame *dup_f;
+
+				AST_LIST_LOCK(&aslist);
+				AST_LIST_TRAVERSE(&aslist, as, list) {
+					if (as->chan != chan)
+						continue;
+					if ((dup_f = ast_frdup(f)))
+						AST_LIST_INSERT_TAIL(&as->dtmf_frames, dup_f, frame_list);
+				}
+				AST_LIST_UNLOCK(&aslist);
+			}
+
+			/* Throw these frames away */
+			case AST_FRAME_VOICE:
+			case AST_FRAME_VIDEO:
+			case AST_FRAME_NULL:
+			case AST_FRAME_IAX:
+			case AST_FRAME_CNG:
+			case AST_FRAME_MODEM:
+				break;
+			}
+
 			if (f)
 				ast_frfree(f);
 		}
@@ -97,17 +136,21 @@
 {
 	int res = -1;
 	struct asent *as;
+
 	AST_LIST_LOCK(&aslist);
 
 	/* Check if the channel already has autoservice */
 	AST_LIST_TRAVERSE(&aslist, as, list) {
-		if (as->chan == chan)
+		if (as->chan == chan) {
+			as->use_count++;
 			break;
+		}
 	}
 
 	/* If not, start autoservice on channel */
 	if (!as && (as = ast_calloc(1, sizeof(*as)))) {
 		as->chan = chan;
+		as->use_count = 1;
 		AST_LIST_INSERT_HEAD(&aslist, as, list);
 		res = 0;
 		if (asthread == AST_PTHREADT_NULL) { /* need start the thread */
@@ -122,7 +165,9 @@
 				pthread_kill(asthread, SIGURG);
 		}
 	}
+
 	AST_LIST_UNLOCK(&aslist);
+
 	return res;
 }
 
@@ -130,11 +175,22 @@
 {
 	int res = -1;
 	struct asent *as;
+	AST_LIST_HEAD_NOLOCK(, ast_frame) dtmf_frames;
+	struct ast_frame *f;
+	int removed = 1;
+
+	AST_LIST_HEAD_INIT_NOLOCK(&dtmf_frames);
 
 	AST_LIST_LOCK(&aslist);
 	AST_LIST_TRAVERSE_SAFE_BEGIN(&aslist, as, list) {	
 		if (as->chan == chan) {
+			as->use_count--;
+			if (as->use_count) {
+				removed = 0;
+				break;
+			}
 			AST_LIST_REMOVE_CURRENT(&aslist, list);
+			AST_LIST_APPEND_LIST(&dtmf_frames, &as->dtmf_frames, frame_list);
 			free(as);
 			if (!chan->_softhangup)
 				res = 0;
@@ -143,12 +199,22 @@
 	}
 	AST_LIST_TRAVERSE_SAFE_END
 
-	if (asthread != AST_PTHREADT_NULL) 
+	if (removed && asthread != AST_PTHREADT_NULL) 
 		pthread_kill(asthread, SIGURG);
+
 	AST_LIST_UNLOCK(&aslist);
 
+	if (!removed)
+		return 0;
+
 	/* Wait for it to un-block */
-	while(ast_test_flag(chan, AST_FLAG_BLOCKING))
+	while (ast_test_flag(chan, AST_FLAG_BLOCKING))
 		usleep(1000);
+
+	while ((f = AST_LIST_REMOVE_HEAD(&dtmf_frames, frame_list))) {
+		ast_queue_frame(chan, f);
+		ast_frfree(f);
+	}
+
 	return res;
 }

Modified: branches/1.4/main/pbx.c
URL: http://svn.digium.com/view/asterisk/branches/1.4/main/pbx.c?view=diff&rev=89790&r1=89789&r2=89790
==============================================================================
--- branches/1.4/main/pbx.c (original)
+++ branches/1.4/main/pbx.c Tue Nov 27 15:45:51 2007
@@ -1764,7 +1764,8 @@
 	pbx_substitute_variables_helper(c, e->data, passdata, datalen - 1);
 }
 
-/*! \brief The return value depends on the action:
+/*! 
+ * \brief The return value depends on the action:
  *
  * E_MATCH, E_CANMATCH, E_MATCHMORE require a real match,
  *	and return 0 on failure, -1 on match;
@@ -1772,6 +1773,12 @@
  *	the priority on success, ... XXX
  * E_SPAWN, spawn an application,
  *	and return 0 on success, -1 on failure.
+ *
+ * \note The channel is auto-serviced in this function, because doing an extension
+ * match may block for a long time.  For example, if the lookup has to use a network
+ * dialplan switch, such as DUNDi or IAX2, it may take a while.  However, the channel
+ * auto-service code will queue up any important signalling frames to be processed
+ * after this is done.
  */
 static int pbx_extension_helper(struct ast_channel *c, struct ast_context *con,
 	const char *context, const char *exten, int priority,
@@ -1785,21 +1792,26 @@
 
 	int matching_action = (action == E_MATCH || action == E_CANMATCH || action == E_MATCHMORE);
 
+	ast_autoservice_start(c);
+
 	ast_mutex_lock(&conlock);
 	e = pbx_find_extension(c, con, &q, context, exten, priority, label, callerid, action);
 	if (e) {
 		if (matching_action) {
 			ast_mutex_unlock(&conlock);
+			ast_autoservice_stop(c);
 			return -1;	/* success, we found it */
 		} else if (action == E_FINDLABEL) { /* map the label to a priority */
 			res = e->priority;
 			ast_mutex_unlock(&conlock);
+			ast_autoservice_stop(c);
 			return res;	/* the priority we were looking for */
 		} else {	/* spawn */
 			app = pbx_findapp(e->app);
 			ast_mutex_unlock(&conlock);
 			if (!app) {
 				ast_log(LOG_WARNING, "No application '%s' for extension (%s, %s, %d)\n", e->app, context, exten, priority);
+				ast_autoservice_stop(c);
 				return -1;
 			}
 			if (c->context != context)
@@ -1835,17 +1847,20 @@
 					"AppData: %s\r\n"
 					"Uniqueid: %s\r\n",
 					c->name, c->context, c->exten, c->priority, app->name, passdata, c->uniqueid);
+			ast_autoservice_stop(c);
 			return pbx_exec(c, app, passdata);	/* 0 on success, -1 on failure */
 		}
 	} else if (q.swo) {	/* not found here, but in another switch */
 		ast_mutex_unlock(&conlock);
-		if (matching_action)
+		if (matching_action) {
+			ast_autoservice_stop(c);
 			return -1;
-		else {
+		} else {
 			if (!q.swo->exec) {
 				ast_log(LOG_WARNING, "No execution engine for switch %s\n", q.swo->name);
 				res = -1;
 			}
+			ast_autoservice_stop(c);
 			return q.swo->exec(c, q.foundcontext ? q.foundcontext : context, exten, priority, callerid, q.data);
 		}
 	} else {	/* not found anywhere, see what happened */
@@ -1871,6 +1886,8 @@
 			if (option_debug)
 				ast_log(LOG_DEBUG, "Shouldn't happen!\n");
 		}
+
+		ast_autoservice_stop(c);
 
 		return (matching_action) ? 0 : -1;
 	}




More information about the asterisk-commits mailing list