[svn-commits] kpfleming: branch 1.6.2 r182528 - in /branches/1.6.2: ./ include/asterisk/ main/

SVN commits to the Digium repositories svn-commits at lists.digium.com
Tue Mar 17 09:40:48 CDT 2009


Author: kpfleming
Date: Tue Mar 17 09:40:44 2009
New Revision: 182528

URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=182528
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.2/   (props changed)
    branches/1.6.2/include/asterisk/channel.h
    branches/1.6.2/main/channel.c
    branches/1.6.2/main/features.c

Propchange: branches/1.6.2/
------------------------------------------------------------------------------
--- trunk-merged (original)
+++ trunk-merged Tue Mar 17 09:40:44 2009
@@ -1,1 +1,1 @@
-/trunk:1-182359,182408,182450
+/trunk:1-182359,182408,182450,182525

Modified: branches/1.6.2/include/asterisk/channel.h
URL: http://svn.digium.com/svn-view/asterisk/branches/1.6.2/include/asterisk/channel.h?view=diff&rev=182528&r1=182527&r2=182528
==============================================================================
--- branches/1.6.2/include/asterisk/channel.h (original)
+++ branches/1.6.2/include/asterisk/channel.h Tue Mar 17 09:40:44 2009
@@ -1008,12 +1008,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.2/main/channel.c
URL: http://svn.digium.com/svn-view/asterisk/branches/1.6.2/main/channel.c?view=diff&rev=182528&r1=182527&r2=182528
==============================================================================
--- branches/1.6.2/main/channel.c (original)
+++ branches/1.6.2/main/channel.c Tue Mar 17 09:40:44 2009
@@ -1700,8 +1700,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;
 
@@ -1733,15 +1732,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));
@@ -1749,42 +1783,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.2/main/features.c
URL: http://svn.digium.com/svn-view/asterisk/branches/1.6.2/main/features.c?view=diff&rev=182528&r1=182527&r2=182528
==============================================================================
--- branches/1.6.2/main/features.c (original)
+++ branches/1.6.2/main/features.c Tue Mar 17 09:40:44 2009
@@ -2477,8 +2477,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 svn-commits mailing list