diff options
author | kpfleming <kpfleming@f38db490-d61c-443f-a65b-d21fe96a405b> | 2009-03-17 14:39:16 +0000 |
---|---|---|
committer | kpfleming <kpfleming@f38db490-d61c-443f-a65b-d21fe96a405b> | 2009-03-17 14:39:16 +0000 |
commit | ceac57beff960f2e16655b56b4bf05d3858714e9 (patch) | |
tree | 3f00c6f607ae26230e819769dbb0424c8cb614cb | |
parent | cf8cb2de056d5b67a2ceb97615cf0ebcd3703357 (diff) |
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/
........
git-svn-id: http://svn.digium.com/svn/asterisk/branches/1.6.0@182526 f38db490-d61c-443f-a65b-d21fe96a405b
-rw-r--r-- | include/asterisk/channel.h | 51 | ||||
-rw-r--r-- | main/channel.c | 129 | ||||
-rw-r--r-- | main/features.c | 7 |
3 files changed, 151 insertions, 36 deletions
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h index d42da81b0..8bda3ac5c 100644 --- a/include/asterisk/channel.h +++ b/include/asterisk/channel.h @@ -981,12 +981,61 @@ void ast_channel_setwhentohangup(struct ast_channel *chan, time_t offset); * 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 diff --git a/main/channel.c b/main/channel.c index 110486d57..43c1766a2 100644 --- a/main/channel.c +++ b/main/channel.c @@ -1687,8 +1687,7 @@ int ast_hangup(struct ast_channel *chan) 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 @@ int __ast_answer(struct ast_channel *chan, unsigned int delay, int cdr_answer) 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,43 +1770,72 @@ int __ast_answer(struct ast_channel *chan, unsigned int delay, int cdr_answer) 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; + } + + if (done) { break; } - ast_frfree(f); } - } - 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; } diff --git a/main/features.c b/main/features.c index 7e7edc78b..0f238a528 100644 --- a/main/features.c +++ b/main/features.c @@ -2209,8 +2209,11 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast 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)); |