[asterisk-commits] russell: branch russell/autoservice_1.4 r89620 - /team/russell/autoservice_1....
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Mon Nov 26 17:13:48 CST 2007
Author: russell
Date: Mon Nov 26 17:13:48 2007
New Revision: 89620
URL: http://svn.digium.com/view/asterisk?view=rev&rev=89620
Log:
This morning, d1mas brought up an issue he was having in #asterisk-bugs on IRC.
He was noting that when calling in to his system on a Zap channel and executing
DISA or WaitExten, that some digits would get lost when dialed quickly. He was
using DUNDi in his setup, so every digit required a DUNDi query. However, if
he took DUNDi out of the picture, he had no problems at all. So, we started
talking about potential issues that could cause this to happen.
D1mas theorized that if the DUNDi lookup blocked for some period of time and
audio from the zap channel got lost, then it wouldn't be run through the DSP
and digits could get lost. He is certainly right *if* audio data doesn't get
buffered. So, question:
- If something blocks and some period of time (say, a second, for example)
goes by without calling zt_read, is that audio lost? Or will it all get
read with repeated calls to zt_read after the blocking period is over?
I wrote a patch similar to the one in this commit and gave it for him to try on
his system. He reported that it solved his problem. Regardless of the answer
to my question about zaptel, I think that this patch is the "right thing" to do.
This patch has a few parts.
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:
team/russell/autoservice_1.4/main/autoservice.c
team/russell/autoservice_1.4/main/pbx.c
Modified: team/russell/autoservice_1.4/main/autoservice.c
URL: http://svn.digium.com/view/asterisk/team/russell/autoservice_1.4/main/autoservice.c?view=diff&rev=89620&r1=89619&r2=89620
==============================================================================
--- team/russell/autoservice_1.4/main/autoservice.c (original)
+++ team/russell/autoservice_1.4/main/autoservice.c Mon Nov 26 17:13:48 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: team/russell/autoservice_1.4/main/pbx.c
URL: http://svn.digium.com/view/asterisk/team/russell/autoservice_1.4/main/pbx.c?view=diff&rev=89620&r1=89619&r2=89620
==============================================================================
--- team/russell/autoservice_1.4/main/pbx.c (original)
+++ team/russell/autoservice_1.4/main/pbx.c Mon Nov 26 17:13:48 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