[asterisk-commits] jpeeler: trunk r187211 - in /trunk: include/asterisk/ main/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Apr 8 16:00:43 CDT 2009


Author: jpeeler
Date: Wed Apr  8 16:00:39 2009
New Revision: 187211

URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=187211
Log:
Add timer for features so that backup bridge config can go away

The biggest change done here was elimination of the backup_config for use with
features. Previously, the bridging code upon detecting a feature would set the
start time of the bridge to the start time of the feature. Then after the 
feature had either expired or timed out the start time would be reset to the
true bridge start time from the backup_config. Now, the time differences are
calculated with respect to the newly added feature_start_time timeval instead.

There should be no behavior changes from the previous functionality aside from
the bridge timing being unaffected by either valid or partial feature matches.
Previously the timing would be increased by the length of time configured for
featuredigittimeout, which was probably never noticed.

(closes issue #14503)
Reported by: KNK
Tested by: jpeeler

Review: http://reviewboard.digium.com/r/179/

Modified:
    trunk/include/asterisk/channel.h
    trunk/main/channel.c
    trunk/main/features.c

Modified: trunk/include/asterisk/channel.h
URL: http://svn.digium.com/svn-view/asterisk/trunk/include/asterisk/channel.h?view=diff&rev=187211&r1=187210&r2=187211
==============================================================================
--- trunk/include/asterisk/channel.h (original)
+++ trunk/include/asterisk/channel.h Wed Apr  8 16:00:39 2009
@@ -738,6 +738,7 @@
 	struct ast_flags features_callee;
 	struct timeval start_time;
 	struct timeval nexteventts;
+	struct timeval feature_start_time;
 	long feature_timer;
 	long timelimit;
 	long play_warning;
@@ -745,7 +746,6 @@
 	const char *warning_sound;
 	const char *end_sound;
 	const char *start_sound;
-	int firstpass;
 	unsigned int flags;
 	void (* end_bridge_callback)(void *);   /*!< A callback that is called after a bridge attempt */
 	void *end_bridge_callback_data;         /*!< Data passed to the callback */

Modified: trunk/main/channel.c
URL: http://svn.digium.com/svn-view/asterisk/trunk/main/channel.c?view=diff&rev=187211&r1=187210&r2=187211
==============================================================================
--- trunk/main/channel.c (original)
+++ trunk/main/channel.c Wed Apr  8 16:00:39 2009
@@ -4814,7 +4814,7 @@
 
 static enum ast_bridge_result ast_generic_bridge(struct ast_channel *c0, struct ast_channel *c1,
 						 struct ast_bridge_config *config, struct ast_frame **fo,
-						 struct ast_channel **rc, struct timeval bridge_end)
+						 struct ast_channel **rc)
 {
 	/* Copy voice back and forth between the two channels. */
 	struct ast_channel *cs[3];
@@ -4856,13 +4856,17 @@
 			res = AST_BRIDGE_RETRY;
 			break;
 		}
-		if (bridge_end.tv_sec) {
-			to = ast_tvdiff_ms(bridge_end, ast_tvnow());
+		if (config->nexteventts.tv_sec) {
+			to = ast_tvdiff_ms(config->nexteventts, ast_tvnow());
 			if (to <= 0) {
-				if (config->timelimit) {
+				if (config->timelimit && !config->feature_timer && !ast_test_flag(config, AST_FEATURE_WARNING_ACTIVE)) {
 					res = AST_BRIDGE_RETRY;
 					/* generic bridge ending to play warning */
 					ast_set_flag(config, AST_FEATURE_WARNING_ACTIVE);
+				} else if (config->feature_timer) {
+					/* feature timer expired - make sure we do not play warning */
+					ast_clear_flag(config, AST_FEATURE_WARNING_ACTIVE);
+					res = AST_BRIDGE_RETRY;
 				} else {
 					res = AST_BRIDGE_COMPLETE;
 				}
@@ -4939,8 +4943,8 @@
 			/* monitored dtmf causes exit from bridge */
 			int monitored_source = (who == c0) ? watch_c0_dtmf : watch_c1_dtmf;
 
-			if (monitored_source && 
-				(f->frametype == AST_FRAME_DTMF_END || 
+			if (monitored_source &&
+				(f->frametype == AST_FRAME_DTMF_END ||
 				f->frametype == AST_FRAME_DTMF_BEGIN)) {
 				*fo = f;
 				*rc = who;
@@ -4994,13 +4998,13 @@
 {
 	manager_event(EVENT_FLAG_CALL, "Bridge",
 			"Bridgestate: %s\r\n"
-		     "Bridgetype: %s\r\n"
-		      "Channel1: %s\r\n"
-		      "Channel2: %s\r\n"
-		      "Uniqueid1: %s\r\n"
-		      "Uniqueid2: %s\r\n"
-		      "CallerID1: %s\r\n"
-		      "CallerID2: %s\r\n",
+			"Bridgetype: %s\r\n"
+			"Channel1: %s\r\n"
+			"Channel2: %s\r\n"
+			"Uniqueid1: %s\r\n"
+			"Uniqueid2: %s\r\n"
+			"CallerID1: %s\r\n"
+			"CallerID2: %s\r\n",
 			onoff ? "Link" : "Unlink",
 			type == 1 ? "core" : "native",
 			c0->name, c1->name, c0->uniqueid, c1->uniqueid, 
@@ -5079,7 +5083,6 @@
 	struct ast_channel *who = NULL;
 	enum ast_bridge_result res = AST_BRIDGE_COMPLETE;
 	int nativefailed=0;
-	int firstpass;
 	int o0nativeformats;
 	int o1nativeformats;
 	long time_left_ms=0;
@@ -5103,21 +5106,17 @@
 		return -1;
 
 	*fo = NULL;
-	firstpass = config->firstpass;
-	config->firstpass = 0;
-
-	if (ast_tvzero(config->start_time))
+
+	if (ast_tvzero(config->start_time)) {
 		config->start_time = ast_tvnow();
-	time_left_ms = config->timelimit;
-
-	caller_warning = ast_test_flag(&config->features_caller, AST_FEATURE_PLAY_WARNING);
-	callee_warning = ast_test_flag(&config->features_callee, AST_FEATURE_PLAY_WARNING);
-
-	if (config->start_sound && firstpass) {
-		if (caller_warning)
-			bridge_playfile(c0, c1, config->start_sound, time_left_ms / 1000);
-		if (callee_warning)
-			bridge_playfile(c1, c0, config->start_sound, time_left_ms / 1000);
+		if (config->start_sound) {
+			if (caller_warning) {
+				bridge_playfile(c0, c1, config->start_sound, config->timelimit / 1000);
+			}
+			if (callee_warning) {
+				bridge_playfile(c1, c0, config->start_sound, config->timelimit / 1000);
+			}
+		}
 	}
 
 	/* Keep track of bridge */
@@ -5129,11 +5128,26 @@
 	o1nativeformats = c1->nativeformats;
 
 	if (config->feature_timer && !ast_tvzero(config->nexteventts)) {
-		config->nexteventts = ast_tvadd(config->start_time, ast_samp2tv(config->feature_timer, 1000));
-	} else if (config->timelimit && firstpass) {
+		config->nexteventts = ast_tvadd(config->feature_start_time, ast_samp2tv(config->feature_timer, 1000));
+	} else if (config->timelimit) {
+		time_left_ms = config->timelimit - ast_tvdiff_ms(ast_tvnow(), config->start_time);
+		caller_warning = ast_test_flag(&config->features_caller, AST_FEATURE_PLAY_WARNING);
+		callee_warning = ast_test_flag(&config->features_callee, AST_FEATURE_PLAY_WARNING);
 		config->nexteventts = ast_tvadd(config->start_time, ast_samp2tv(config->timelimit, 1000));
-		if (caller_warning || callee_warning)
-			config->nexteventts = ast_tvsub(config->nexteventts, ast_samp2tv(config->play_warning, 1000));
+		if ((caller_warning || callee_warning) && config->play_warning) {
+			long next_warn = config->play_warning;
+			if (time_left_ms < config->play_warning) {
+				/* At least one warning was played, which means we are returning after feature */
+				long warns_passed = (config->play_warning - time_left_ms) / config->warning_freq;
+				/* It is 'warns_passed * warning_freq' NOT '(warns_passed + 1) * warning_freq',
+					because nexteventts will be updated once again in the 'if (!to)' block */
+				next_warn = config->play_warning - warns_passed * config->warning_freq;
+			}
+			config->nexteventts = ast_tvsub(config->nexteventts, ast_samp2tv(next_warn, 1000));
+		}
+	} else {
+		config->nexteventts.tv_sec = 0;
+		config->nexteventts.tv_usec = 0;
 	}
 
 	if (!c0->tech->send_digit_begin)
@@ -5189,10 +5203,12 @@
 					if (callee_warning)
 						bridge_playfile(c1, c0, config->warning_sound, t);
 				}
-				if (config->warning_freq && (time_left_ms > (config->warning_freq + 5000)))
+
+				if (config->warning_freq && (time_left_ms > (config->warning_freq + 5000))) {
 					config->nexteventts = ast_tvadd(config->nexteventts, ast_samp2tv(config->warning_freq, 1000));
-				else
+				} else {
 					config->nexteventts = ast_tvadd(config->start_time, ast_samp2tv(config->timelimit, 1000));
+				}
 			}
 			ast_clear_flag(config, AST_FEATURE_WARNING_ACTIVE);
 		}
@@ -5237,7 +5253,6 @@
 			ast_set_flag(c0, AST_FLAG_NBRIDGE);
 			ast_set_flag(c1, AST_FLAG_NBRIDGE);
 			if ((res = c0->tech->bridge(c0, c1, config->flags, fo, rc, to)) == AST_BRIDGE_COMPLETE) {
-				/* \todo  XXX here should check that cid_num is not NULL */
 				manager_event(EVENT_FLAG_CALL, "Unlink",
 					      "Channel1: %s\r\n"
 					      "Channel2: %s\r\n"
@@ -5245,7 +5260,7 @@
 					      "Uniqueid2: %s\r\n"
 					      "CallerID1: %s\r\n"
 					      "CallerID2: %s\r\n",
-					      c0->name, c1->name, c0->uniqueid, c1->uniqueid, c0->cid.cid_num, c1->cid.cid_num);
+					      c0->name, c1->name, c0->uniqueid, c1->uniqueid, S_OR(c0->cid.cid_num, "<unknown>"), S_OR(c1->cid.cid_num, "<unknown>"));
 				ast_debug(1, "Returning from native bridge, channels: %s, %s\n", c0->name, c1->name);
 
 				ast_clear_flag(c0, AST_FLAG_NBRIDGE);
@@ -5291,7 +5306,7 @@
 
 		update_bridge_vars(c0, c1);
 
-		res = ast_generic_bridge(c0, c1, config, fo, rc, config->nexteventts);
+		res = ast_generic_bridge(c0, c1, config, fo, rc);
 		if (res != AST_BRIDGE_RETRY) {
 			break;
 		} else if (config->feature_timer) {
@@ -5310,7 +5325,6 @@
 	c0->_bridge = NULL;
 	c1->_bridge = NULL;
 
-	/* \todo  XXX here should check that cid_num is not NULL */
 	manager_event(EVENT_FLAG_CALL, "Unlink",
 		      "Channel1: %s\r\n"
 		      "Channel2: %s\r\n"
@@ -5318,7 +5332,7 @@
 		      "Uniqueid2: %s\r\n"
 		      "CallerID1: %s\r\n"
 		      "CallerID2: %s\r\n",
-		      c0->name, c1->name, c0->uniqueid, c1->uniqueid, c0->cid.cid_num, c1->cid.cid_num);
+		      c0->name, c1->name, c0->uniqueid, c1->uniqueid, S_OR(c0->cid.cid_num, "<unknown>"), S_OR(c1->cid.cid_num, "<unknown>"));
 	ast_debug(1, "Bridge stops bridging channels %s and %s\n", c0->name, c1->name);
 
 	return res;

Modified: trunk/main/features.c
URL: http://svn.digium.com/svn-view/asterisk/trunk/main/features.c?view=diff&rev=187211&r1=187210&r2=187211
==============================================================================
--- trunk/main/features.c (original)
+++ trunk/main/features.c Wed Apr  8 16:00:39 2009
@@ -2509,7 +2509,6 @@
 	int hadfeatures=0;
 	int autoloopflag;
 	struct ast_option_header *aoh;
-	struct ast_bridge_config backup_config;
 	struct ast_cdr *bridge_cdr = NULL;
 	struct ast_cdr *orig_peer_cdr = NULL;
 	struct ast_cdr *chan_cdr = pick_unlocked_cdr(chan->cdr); /* the proper chan cdr, if there are forked cdrs */
@@ -2517,10 +2516,6 @@
 	struct ast_cdr *new_chan_cdr = NULL; /* the proper chan cdr, if there are forked cdrs */
 	struct ast_cdr *new_peer_cdr = NULL; /* the proper chan cdr, if there are forked cdrs */
 
-	memset(&backup_config, 0, sizeof(backup_config));
-
-	config->start_time = ast_tvnow();
-
 	if (chan && peer) {
 		pbx_builtin_setvar_helper(chan, "BRIDGEPEER", peer->name);
 		pbx_builtin_setvar_helper(peer, "BRIDGEPEER", chan->name);
@@ -2541,7 +2536,7 @@
 	if (monitor_ok) {
 		const char *monitor_exec;
 		struct ast_channel *src = NULL;
-		if (!monitor_app) { 
+		if (!monitor_app) {
 			if (!(monitor_app = pbx_findapp("Monitor")))
 				monitor_ok=0;
 		}
@@ -2556,7 +2551,6 @@
 	}
 
 	set_config_flags(chan, peer, config);
-	config->firstpass = 1;
 
 	/* Answer if need be */
 	if (chan->_state != AST_STATE_UP) {
@@ -2635,8 +2629,8 @@
 		   feature_timer. Not good!
 		*/
 		if (config->feature_timer && (!f || f->frametype == AST_FRAME_DTMF_END)) {
-			/* Update time limit for next pass */
-			diff = ast_tvdiff_ms(ast_tvnow(), config->start_time);
+			/* Update feature timer for next pass */
+			diff = ast_tvdiff_ms(ast_tvnow(), config->feature_start_time);
 			if (res == AST_BRIDGE_RETRY) {
 				/* The feature fully timed out but has not been updated. Skip
 				 * the potential round error from the diff calculation and
@@ -2647,18 +2641,7 @@
 			}
 
 			if (hasfeatures) {
-				/* Running on backup config, meaning a feature might be being
-				   activated, but that's no excuse to keep things going 
-				   indefinitely! */
-				if (backup_config.feature_timer && ((backup_config.feature_timer -= diff) <= 0)) {
-					ast_debug(1, "Timed out, realtime this time!\n");
-					config->feature_timer = 0;
-					who = chan;
-					if (f)
-						ast_frfree(f);
-					f = NULL;
-					res = 0;
-				} else if (config->feature_timer <= 0) {
+				if (config->feature_timer <= 0) {
 					/* Not *really* out of time, just out of time for
 					   digits to come in for features. */
 					ast_debug(1, "Timed out for feature!\n");
@@ -2674,9 +2657,8 @@
 						ast_frfree(f);
 					hasfeatures = !ast_strlen_zero(chan_featurecode) || !ast_strlen_zero(peer_featurecode);
 					if (!hasfeatures) {
-						/* Restore original (possibly time modified) bridge config */
-						memcpy(config, &backup_config, sizeof(struct ast_bridge_config));
-						memset(&backup_config, 0, sizeof(backup_config));
+						/* No more digits expected - reset the timer */
+						config->feature_timer = 0;
 					}
 					hadfeatures = hasfeatures;
 					/* Continue as we were */
@@ -2700,13 +2682,14 @@
 			}
 		}
 		if (res < 0) {
-			if (!ast_test_flag(chan, AST_FLAG_ZOMBIE) && !ast_test_flag(peer, AST_FLAG_ZOMBIE) && !ast_check_hangup(chan) && !ast_check_hangup(peer))
+			if (!ast_test_flag(chan, AST_FLAG_ZOMBIE) && !ast_test_flag(peer, AST_FLAG_ZOMBIE) && !ast_check_hangup(chan) && !ast_check_hangup(peer)) {
 				ast_log(LOG_WARNING, "Bridge failed on channels %s and %s\n", chan->name, peer->name);
+			}
 			goto before_you_go;
 		}
 		
 		if (!f || (f->frametype == AST_FRAME_CONTROL &&
-				(f->subclass == AST_CONTROL_HANGUP || f->subclass == AST_CONTROL_BUSY || 
+				(f->subclass == AST_CONTROL_HANGUP || f->subclass == AST_CONTROL_BUSY ||
 					f->subclass == AST_CONTROL_CONGESTION))) {
 			res = -1;
 			break;
@@ -2756,7 +2739,7 @@
 			/* Get rid of the frame before we start doing "stuff" with the channels */
 			ast_frfree(f);
 			f = NULL;
-			config->feature_timer = backup_config.feature_timer;
+			config->feature_timer = 0;
 			res = feature_interpret(chan, peer, config, featurecode, sense);
 			switch(res) {
 			case AST_FEATURE_RETURN_PASSDIGITS:
@@ -2768,30 +2751,21 @@
 			}
 			if (res >= AST_FEATURE_RETURN_PASSDIGITS) {
 				res = 0;
-			} else 
+			} else {
 				break;
+			}
 			hasfeatures = !ast_strlen_zero(chan_featurecode) || !ast_strlen_zero(peer_featurecode);
 			if (hadfeatures && !hasfeatures) {
-				/* Restore backup */
-				memcpy(config, &backup_config, sizeof(struct ast_bridge_config));
-				memset(&backup_config, 0, sizeof(struct ast_bridge_config));
+				/* Feature completed or timed out */
+				config->feature_timer = 0;
 			} else if (hasfeatures) {
-				if (!hadfeatures) {
-					/* Backup configuration */
-					memcpy(&backup_config, config, sizeof(struct ast_bridge_config));
-					/* Setup temporary config options */
-					config->play_warning = 0;
-					ast_clear_flag(&(config->features_caller), AST_FEATURE_PLAY_WARNING);
-					ast_clear_flag(&(config->features_callee), AST_FEATURE_PLAY_WARNING);
-					config->warning_freq = 0;
-					config->warning_sound = NULL;
-					config->end_sound = NULL;
-					config->start_sound = NULL;
-					config->firstpass = 0;
+				if (config->timelimit) {
+					/* No warning next time - we are waiting for future */
+					ast_set_flag(config, AST_FEATURE_WARNING_ACTIVE);
 				}
-				config->start_time = ast_tvnow();
+				config->feature_start_time = ast_tvnow();
 				config->feature_timer = featuredigittimeout;
-				ast_debug(1, "Set time limit to %ld\n", config->feature_timer);
+				ast_debug(1, "Set feature timer to %ld\n", config->feature_timer);
 			}
 		}
 		if (f)




More information about the asterisk-commits mailing list