[asterisk-commits] kpfleming: branch 1.6.0 r182526 - in /branches/1.6.0: ./ include/asterisk/ main/
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Tue Mar 17 09:39:19 CDT 2009
Author: kpfleming
Date: Tue Mar 17 09:39:16 2009
New Revision: 182526
URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=182526
Log:
Merged revisions 182525 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk
........
r182525 | kpfleming | 2009-03-17 09:38:11 -0500 (Tue, 17 Mar 2009) | 11 lines
Improve behavior of ast_answer() to not lose incoming frames
ast_answer(), when supplied a delay before returning to the caller, use ast_safe_sleep() to implement the delay. Unfortunately during this time any incoming frames are discarded, which is problematic for T.38 re-INVITES and other sorts of channel operations.
When a delay is not passed to ast_answer(), it still delays for up to 500 milliseconds, waiting for media to arrive. Again, though, it discards any control frames, or non-voice media frames.
This patch rectifies this situation, by storing all incoming frames during the delay period on a list, and then requeuing them onto the channel before returning to the caller.
http://reviewboard.digium.com/r/196/
........
Modified:
branches/1.6.0/ (props changed)
branches/1.6.0/include/asterisk/channel.h
branches/1.6.0/main/channel.c
branches/1.6.0/main/features.c
Propchange: branches/1.6.0/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.
Modified: branches/1.6.0/include/asterisk/channel.h
URL: http://svn.digium.com/svn-view/asterisk/branches/1.6.0/include/asterisk/channel.h?view=diff&rev=182526&r1=182525&r2=182526
==============================================================================
--- branches/1.6.0/include/asterisk/channel.h (original)
+++ branches/1.6.0/include/asterisk/channel.h Tue Mar 17 09:39:16 2009
@@ -981,12 +981,61 @@
* This function answers a channel and handles all necessary call
* setup functions.
*
- * \note The channel passed does not need to be locked.
+ * \note The channel passed does not need to be locked, but is locked
+ * by the function when needed.
+ *
+ * \note This function will wait up to 500 milliseconds for media to
+ * arrive on the channel before returning to the caller, so that the
+ * caller can properly assume the channel is 'ready' for media flow.
*
* \retval 0 on success
* \retval non-zero on failure
*/
int ast_answer(struct ast_channel *chan);
+
+/*!
+ * \brief Answer a channel
+ *
+ * \param chan channel to answer
+ * \param cdr_answer flag to control whether any associated CDR should be marked as 'answered'
+ *
+ * This function answers a channel and handles all necessary call
+ * setup functions.
+ *
+ * \note The channel passed does not need to be locked, but is locked
+ * by the function when needed.
+ *
+ * \note Unlike ast_answer(), this function will not wait for media
+ * flow to begin. The caller should be careful before sending media
+ * to the channel before incoming media arrives, as the outgoing
+ * media may be lost.
+ *
+ * \retval 0 on success
+ * \retval non-zero on failure
+ */
+int ast_raw_answer(struct ast_channel *chan, int cdr_answer);
+
+/*!
+ * \brief Answer a channel, with a selectable delay before returning
+ *
+ * \param chan channel to answer
+ * \param delay maximum amount of time to wait for incoming media
+ * \param cdr_answer flag to control whether any associated CDR should be marked as 'answered'
+ *
+ * This function answers a channel and handles all necessary call
+ * setup functions.
+ *
+ * \note The channel passed does not need to be locked, but is locked
+ * by the function when needed.
+ *
+ * \note This function will wait up to 'delay' milliseconds for media to
+ * arrive on the channel before returning to the caller, so that the
+ * caller can properly assume the channel is 'ready' for media flow. If
+ * 'delay' is less than 500, the function will wait up to 500 milliseconds.
+ *
+ * \retval 0 on success
+ * \retval non-zero on failure
+ */
int __ast_answer(struct ast_channel *chan, unsigned int delay, int cdr_answer);
/*! \brief Make a call
Modified: branches/1.6.0/main/channel.c
URL: http://svn.digium.com/svn-view/asterisk/branches/1.6.0/main/channel.c?view=diff&rev=182526&r1=182525&r2=182526
==============================================================================
--- branches/1.6.0/main/channel.c (original)
+++ branches/1.6.0/main/channel.c Tue Mar 17 09:39:16 2009
@@ -1687,8 +1687,7 @@
return res;
}
-#define ANSWER_WAIT_MS 500
-int __ast_answer(struct ast_channel *chan, unsigned int delay, int cdr_answer)
+int ast_raw_answer(struct ast_channel *chan, int cdr_answer)
{
int res = 0;
@@ -1720,15 +1719,50 @@
ast_cdr_answer(chan->cdr);
}
ast_channel_unlock(chan);
- if (delay) {
- ast_safe_sleep(chan, delay);
- } else {
- struct ast_frame *f;
- int ms = ANSWER_WAIT_MS;
- while (1) {
- /* 500 ms was the original delay here, so now
- * we cap our waiting at 500 ms
- */
+ break;
+ case AST_STATE_UP:
+ /* Calling ast_cdr_answer when it it has previously been called
+ * is essentially a no-op, so it is safe.
+ */
+ if (cdr_answer) {
+ ast_cdr_answer(chan->cdr);
+ }
+ break;
+ default:
+ break;
+ }
+
+ ast_indicate(chan, -1);
+ chan->visible_indication = 0;
+
+ return res;
+}
+
+int __ast_answer(struct ast_channel *chan, unsigned int delay, int cdr_answer)
+{
+ int res = 0;
+ enum ast_channel_state old_state;
+
+ old_state = chan->_state;
+ if ((res = ast_raw_answer(chan, cdr_answer))) {
+ return res;
+ }
+
+ switch (old_state) {
+ case AST_STATE_RINGING:
+ case AST_STATE_RING:
+ /* wait for media to start flowing, but don't wait any longer
+ * than 'delay' or 500 milliseconds, whichever is longer
+ */
+ do {
+ AST_LIST_HEAD_NOLOCK(, ast_frame) frames;
+ struct ast_frame *cur, *new;
+ int ms = MAX(delay, 500);
+ unsigned int done = 0;
+
+ AST_LIST_HEAD_INIT_NOLOCK(&frames);
+
+ for (;;) {
ms = ast_waitfor(chan, ms);
if (ms < 0) {
ast_log(LOG_WARNING, "Error condition occurred when polling channel %s for a voice frame: %s\n", chan->name, strerror(errno));
@@ -1736,42 +1770,71 @@
break;
}
if (ms == 0) {
- ast_debug(2, "Didn't receive a voice frame from %s within %d ms of answering. Continuing anyway\n", chan->name, ANSWER_WAIT_MS);
- res = 0;
+ ast_debug(2, "Didn't receive a media frame from %s within %d ms of answering. Continuing anyway\n", chan->name, MAX(delay, 500));
break;
}
- f = ast_read(chan);
- if (!f || (f->frametype == AST_FRAME_CONTROL && f->subclass == AST_CONTROL_HANGUP)) {
- if (f) {
- ast_frfree(f);
+ cur = ast_read(chan);
+ if (!cur || ((cur->frametype == AST_FRAME_CONTROL) &&
+ (cur->subclass == AST_CONTROL_HANGUP))) {
+ if (cur) {
+ ast_frfree(cur);
}
res = -1;
ast_debug(2, "Hangup of channel %s detected in answer routine\n", chan->name);
break;
}
- if (f->frametype == AST_FRAME_VOICE) {
- ast_frfree(f);
- res = 0;
+
+ if ((new = ast_frisolate(cur)) != cur) {
+ ast_frfree(cur);
+ }
+
+ AST_LIST_INSERT_TAIL(&frames, new, frame_list);
+
+ /* if a specific delay period was requested, continue
+ * until that delay has passed. don't stop just because
+ * incoming media has arrived.
+ */
+ if (delay) {
+ continue;
+ }
+
+ switch (new->frametype) {
+ /* all of these frametypes qualify as 'media' */
+ case AST_FRAME_VOICE:
+ case AST_FRAME_VIDEO:
+ case AST_FRAME_TEXT:
+ case AST_FRAME_DTMF_BEGIN:
+ case AST_FRAME_DTMF_END:
+ case AST_FRAME_IMAGE:
+ case AST_FRAME_HTML:
+ case AST_FRAME_MODEM:
+ done = 1;
+ break;
+ case AST_FRAME_CONTROL:
+ case AST_FRAME_IAX:
+ case AST_FRAME_NULL:
+ case AST_FRAME_CNG:
break;
}
- ast_frfree(f);
+
+ if (done) {
+ break;
+ }
}
- }
- break;
- case AST_STATE_UP:
- /* Calling ast_cdr_answer when it it has previously been called
- * is essentially a no-op, so it is safe.
- */
- if (cdr_answer) {
- ast_cdr_answer(chan->cdr);
- }
+
+ if (res == 0) {
+ ast_channel_lock(chan);
+ while ((cur = AST_LIST_REMOVE(&frames, AST_LIST_LAST(&frames), frame_list))) {
+ ast_queue_frame_head(chan, cur);
+ ast_frfree(cur);
+ }
+ ast_channel_unlock(chan);
+ }
+ } while (0);
break;
default:
break;
}
-
- ast_indicate(chan, -1);
- chan->visible_indication = 0;
return res;
}
Modified: branches/1.6.0/main/features.c
URL: http://svn.digium.com/svn-view/asterisk/branches/1.6.0/main/features.c?view=diff&rev=182526&r1=182525&r2=182526
==============================================================================
--- branches/1.6.0/main/features.c (original)
+++ branches/1.6.0/main/features.c Tue Mar 17 09:39:16 2009
@@ -2209,8 +2209,11 @@
config->firstpass = 1;
/* Answer if need be */
- if (ast_answer(chan))
- return -1;
+ if (chan->_state != AST_STATE_UP) {
+ if (ast_raw_answer(chan, 1)) {
+ return -1;
+ }
+ }
ast_copy_string(orig_channame,chan->name,sizeof(orig_channame));
ast_copy_string(orig_peername,peer->name,sizeof(orig_peername));
More information about the asterisk-commits
mailing list