From d11b6386a5e69cb881a1fe251609c902212b495a Mon Sep 17 00:00:00 2001 From: "Kevin P. Fleming" Date: Tue, 17 Mar 2009 14:38:11 +0000 Subject: 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: https://origsvn.digium.com/svn/asterisk/trunk@182525 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- main/channel.c | 129 ++++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 96 insertions(+), 33 deletions(-) (limited to 'main/channel.c') diff --git a/main/channel.c b/main/channel.c index f4ab05f95..3520efc22 100644 --- a/main/channel.c +++ b/main/channel.c @@ -1700,8 +1700,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; @@ -1733,15 +1732,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)); @@ -1749,43 +1783,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; } -- cgit v1.2.3