[svn-commits] kharwell: trunk r433447 - in /trunk: ./ apps/ include/asterisk/ main/

SVN commits to the Digium repositories svn-commits at lists.digium.com
Thu Mar 26 12:13:28 CDT 2015


Author: kharwell
Date: Thu Mar 26 12:13:26 2015
New Revision: 433447

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=433447
Log:
app_confbridge: file playback blocks dtmf

Attempting to execute DTMF in a confbridge while file playback (prompt,
announcement, etc) is occurring is not allowed. You have to wait until
the sound file has completed before entering DTMF. This patch fixes it
so that app_confbridge now monitors for dtmf key presses during menu
driven file playback. If a key is pressed playback stops and it executes
the matched menu option.

ASTERISK-24864 #close
Reported by: Steve Pitts
Review: https://reviewboard.asterisk.org/r/4510/
........

Merged revisions 433445 from http://svn.asterisk.org/svn/asterisk/branches/11
........

Merged revisions 433446 from http://svn.asterisk.org/svn/asterisk/branches/13

Modified:
    trunk/   (props changed)
    trunk/apps/app_confbridge.c
    trunk/include/asterisk/bridge_channel.h
    trunk/main/bridge_channel.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-13-merged' - no diff available.

Modified: trunk/apps/app_confbridge.c
URL: http://svnview.digium.com/svn/asterisk/trunk/apps/app_confbridge.c?view=diff&rev=433447&r1=433446&r2=433447
==============================================================================
--- trunk/apps/app_confbridge.c (original)
+++ trunk/apps/app_confbridge.c Thu Mar 26 12:13:26 2015
@@ -723,6 +723,52 @@
 	return 0;
 }
 
+/* \brief Playback the given filename and monitor for any dtmf interrupts.
+ *
+ * This function is used to playback sound files on a given channel and optionally
+ * allow dtmf interrupts to occur.
+ *
+ * If the optional bridge_channel parameter is given then sound file playback
+ * is played on that channel and dtmf interruptions are allowed. However, if
+ * bridge_channel is not set then the channel parameter is expected to be set
+ * instead and non interruptible playback is played on that channel.
+ *
+ * \param bridge_channel Bridge channel to play file on
+ * \param channel Optional channel to play file on if bridge_channel not given
+ * \param filename The file name to playback
+ *
+ * \retval -1 failure during playback, 0 on file was fully played, 1 on dtmf interrupt.
+ */
+static int play_file(struct ast_bridge_channel *bridge_channel, struct ast_channel *channel,
+		     const char *filename)
+{
+	struct ast_channel *chan;
+	const char *stop_digits;
+	int digit;
+
+	if (bridge_channel) {
+		chan = bridge_channel->chan;
+		stop_digits = AST_DIGIT_ANY;
+	} else {
+		chan = channel;
+		stop_digits = AST_DIGIT_NONE;
+	}
+
+	digit = ast_stream_and_wait(chan, filename, stop_digits);
+	if (digit < 0) {
+		ast_log(LOG_WARNING, "Failed to playback file '%s' to channel\n", filename);
+		return -1;
+	}
+
+	if (digit > 0) {
+		ast_stopstream(bridge_channel->chan);
+		ast_bridge_channel_feature_digit_add(bridge_channel, digit);
+		return 1;
+	}
+
+	return 0;
+}
+
 /*!
  * \internal
  * \brief Complain if the given sound file does not exist.
@@ -745,11 +791,13 @@
  *
  * \param conference Conference bridge to peek at
  * \param user Optional Caller
+ * \param bridge_channel The bridged channel involved
  *
  * \note if caller is NULL, the announcment will be sent to all participants in the conference.
  * \return Returns 0 on success, -1 if the user hung up
  */
-static int announce_user_count(struct confbridge_conference *conference, struct confbridge_user *user)
+static int announce_user_count(struct confbridge_conference *conference, struct confbridge_user *user,
+			       struct ast_bridge_channel *bridge_channel)
 {
 	const char *other_in_party = conf_get_sound(CONF_SOUND_OTHER_IN_PARTY, conference->b_profile.sounds);
 	const char *only_one = conf_get_sound(CONF_SOUND_ONLY_ONE, conference->b_profile.sounds);
@@ -761,9 +809,7 @@
 	} else if (conference->activeusers == 2) {
 		if (user) {
 			/* Eep, there is one other person */
-			if (ast_stream_and_wait(user->chan,
-				only_one,
-				"")) {
+			if (play_file(bridge_channel, user->chan, only_one) < 0) {
 				return -1;
 			}
 		} else {
@@ -780,9 +826,7 @@
 			if (ast_say_number(user->chan, conference->activeusers - 1, "", ast_channel_language(user->chan), NULL)) {
 				return -1;
 			}
-			if (ast_stream_and_wait(user->chan,
-				other_in_party,
-				"")) {
+			if (play_file(bridge_channel, user->chan, other_in_party) < 0) {
 				return -1;
 			}
 		} else if (sound_file_exists(there_are) && sound_file_exists(other_in_party)) {
@@ -1301,7 +1345,7 @@
 
 	/* Announce number of users if need be */
 	if (ast_test_flag(&user->u_profile, USER_OPT_ANNOUNCEUSERCOUNT)) {
-		if (announce_user_count(conference, user)) {
+		if (announce_user_count(conference, user, NULL)) {
 			leave_conference(user);
 			return NULL;
 		}
@@ -1316,7 +1360,7 @@
 		 * joined the conference yet.
 		 */
 		ast_autoservice_start(user->chan);
-		user_count_res = announce_user_count(conference, NULL);
+		user_count_res = announce_user_count(conference, NULL, NULL);
 		ast_autoservice_stop(user->chan);
 		if (user_count_res) {
 			leave_conference(user);
@@ -1818,7 +1862,8 @@
 }
 
 static int action_toggle_mute(struct confbridge_conference *conference,
-	struct confbridge_user *user)
+			      struct confbridge_user *user,
+			      struct ast_bridge_channel *bridge_channel)
 {
 	int mute;
 
@@ -1841,10 +1886,9 @@
 		send_unmute_event(user, conference);
 	}
 
-	return ast_stream_and_wait(user->chan, (mute ?
+	return play_file(bridge_channel, NULL, (mute ?
 		conf_get_sound(CONF_SOUND_MUTED, user->b_profile.sounds) :
-		conf_get_sound(CONF_SOUND_UNMUTED, user->b_profile.sounds)),
-		"");
+		conf_get_sound(CONF_SOUND_UNMUTED, user->b_profile.sounds))) < 0;
 }
 
 static int action_toggle_mute_participants(struct confbridge_conference *conference, struct confbridge_user *user)
@@ -1976,9 +2020,8 @@
 	int isadmin = ast_test_flag(&user->u_profile, USER_OPT_ADMIN);
 
 	if (!isadmin) {
-		ast_stream_and_wait(bridge_channel->chan,
-			conf_get_sound(CONF_SOUND_ERROR_MENU, user->b_profile.sounds),
-			"");
+		play_file(bridge_channel, NULL,
+			  conf_get_sound(CONF_SOUND_ERROR_MENU, user->b_profile.sounds));
 		ast_log(LOG_WARNING, "Only admin users can use the kick_last menu action. Channel %s of conf %s is not an admin.\n",
 			ast_channel_name(bridge_channel->chan),
 			conference->name);
@@ -1989,9 +2032,8 @@
 	if (((last_user = AST_LIST_LAST(&conference->active_list)) == user)
 		|| (ast_test_flag(&last_user->u_profile, USER_OPT_ADMIN))) {
 		ao2_unlock(conference);
-		ast_stream_and_wait(bridge_channel->chan,
-			conf_get_sound(CONF_SOUND_ERROR_MENU, user->b_profile.sounds),
-			"");
+		play_file(bridge_channel, NULL,
+			  conf_get_sound(CONF_SOUND_ERROR_MENU, user->b_profile.sounds));
 	} else if (last_user && !last_user->kicked) {
 		last_user->kicked = 1;
 		pbx_builtin_setvar_helper(last_user->chan, "CONFBRIDGE_RESULT", "KICKED");
@@ -2059,7 +2101,7 @@
 	AST_LIST_TRAVERSE(&menu_entry->actions, menu_action, action) {
 		switch (menu_action->id) {
 		case MENU_ACTION_TOGGLE_MUTE:
-			res |= action_toggle_mute(conference, user);
+			res |= action_toggle_mute(conference, user, bridge_channel);
 			break;
 		case MENU_ACTION_ADMIN_TOGGLE_MUTE_PARTICIPANTS:
 			if (!isadmin) {
@@ -2068,7 +2110,7 @@
 			action_toggle_mute_participants(conference, user);
 			break;
 		case MENU_ACTION_PARTICIPANT_COUNT:
-			announce_user_count(conference, user);
+			announce_user_count(conference, user, bridge_channel);
 			break;
 		case MENU_ACTION_PLAYBACK:
 			if (!stop_prompts) {
@@ -2119,12 +2161,10 @@
 				break;
 			}
 			conference->locked = (!conference->locked ? 1 : 0);
-			res |= ast_stream_and_wait(bridge_channel->chan,
+			res |= play_file(bridge_channel, NULL,
 				(conference->locked ?
 				conf_get_sound(CONF_SOUND_LOCKED_NOW, user->b_profile.sounds) :
-				conf_get_sound(CONF_SOUND_UNLOCKED_NOW, user->b_profile.sounds)),
-				"");
-
+				conf_get_sound(CONF_SOUND_UNLOCKED_NOW, user->b_profile.sounds))) < 0;
 			break;
 		case MENU_ACTION_ADMIN_KICK_LAST:
 			res |= action_kick_last(conference, bridge_channel, user);

Modified: trunk/include/asterisk/bridge_channel.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/bridge_channel.h?view=diff&rev=433447&r1=433446&r2=433447
==============================================================================
--- trunk/include/asterisk/bridge_channel.h (original)
+++ trunk/include/asterisk/bridge_channel.h Thu Mar 26 12:13:26 2015
@@ -656,17 +656,36 @@
 void ast_bridge_channel_kick(struct ast_bridge_channel *bridge_channel, int cause);
 
 /*!
+ * \brief Add a DTMF digit to the collected digits.
+ * \since 13.3.0
+ *
+ * \param bridge_channel Channel that received a DTMF digit.
+ * \param digit DTMF digit to add to collected digits
+ *
+ * \note Neither the bridge nor the bridge_channel locks should be held
+ * when entering this function.
+ *
+ * \note This is can only be called from within DTMF bridge hooks.
+ */
+void ast_bridge_channel_feature_digit_add(struct ast_bridge_channel *bridge_channel, int digit);
+
+/*!
  * \brief Add a DTMF digit to the collected digits to match against DTMF features.
  * \since 12.8.0
  *
  * \param bridge_channel Channel that received a DTMF digit.
  * \param digit DTMF digit to add to collected digits or 0 for timeout event.
+ * \param clear_digits clear the digits array prior to calling hooks
  *
  * \note Neither the bridge nor the bridge_channel locks should be held
  * when entering this function.
  *
  * \note This is intended to be called by bridge hooks and the
  * bridge channel thread.
+ *
+ * \note This is intended to be called by non-DTMF bridge hooks and the bridge
+ * channel thread.  Calling from a DTMF bridge hook can potentially cause
+ * unbounded recursion.
  *
  * \return Nothing
  */

Modified: trunk/main/bridge_channel.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/bridge_channel.c?view=diff&rev=433447&r1=433446&r2=433447
==============================================================================
--- trunk/main/bridge_channel.c (original)
+++ trunk/main/bridge_channel.c Thu Mar 26 12:13:26 2015
@@ -1510,6 +1510,51 @@
 #endif /* TEST_FRAMEWORK */
 }
 
+static int bridge_channel_feature_digit_add(
+	struct ast_bridge_channel *bridge_channel, int digit, size_t dtmf_len)
+{
+	if (dtmf_len < ARRAY_LEN(bridge_channel->dtmf_hook_state.collected) - 1) {
+		/* Add the new digit to the DTMF string so we can do our matching */
+		bridge_channel->dtmf_hook_state.collected[dtmf_len++] = digit;
+		bridge_channel->dtmf_hook_state.collected[dtmf_len] = '\0';
+
+		ast_debug(1, "DTMF feature string on %p(%s) is now '%s'\n",
+			  bridge_channel, ast_channel_name(bridge_channel->chan),
+			  bridge_channel->dtmf_hook_state.collected);
+	}
+
+	return dtmf_len;
+}
+
+static unsigned int bridge_channel_feature_digit_timeout(struct ast_bridge_channel *bridge_channel)
+{
+	unsigned int digit_timeout;
+	struct ast_features_general_config *gen_cfg;
+
+	/* Determine interdigit timeout */
+	ast_channel_lock(bridge_channel->chan);
+	gen_cfg = ast_get_chan_features_general_config(bridge_channel->chan);
+	ast_channel_unlock(bridge_channel->chan);
+
+	if (!gen_cfg) {
+		ast_log(LOG_ERROR, "Unable to retrieve features configuration.\n");
+		return 3000; /* Pick a reasonable failsafe timeout in ms */
+	}
+
+	digit_timeout = gen_cfg->featuredigittimeout;
+	ao2_ref(gen_cfg, -1);
+
+	return digit_timeout;
+}
+
+void ast_bridge_channel_feature_digit_add(struct ast_bridge_channel *bridge_channel, int digit)
+{
+	if (digit) {
+		bridge_channel_feature_digit_add(
+			bridge_channel, digit, strlen(bridge_channel->dtmf_hook_state.collected));
+	}
+}
+
 void ast_bridge_channel_feature_digit(struct ast_bridge_channel *bridge_channel, int digit)
 {
 	struct ast_bridge_features *features = bridge_channel->features;
@@ -1527,17 +1572,10 @@
 	}
 
 	if (digit) {
-		/* There should always be room for the new digit. */
-		ast_assert(dtmf_len < ARRAY_LEN(bridge_channel->dtmf_hook_state.collected) - 1);
-
-		/* Add the new digit to the DTMF string so we can do our matching */
-		bridge_channel->dtmf_hook_state.collected[dtmf_len++] = digit;
-		bridge_channel->dtmf_hook_state.collected[dtmf_len] = '\0';
-
-		ast_debug(1, "DTMF feature string on %p(%s) is now '%s'\n",
-			bridge_channel, ast_channel_name(bridge_channel->chan),
-			bridge_channel->dtmf_hook_state.collected);
-
+		dtmf_len = bridge_channel_feature_digit_add(bridge_channel, digit, dtmf_len);
+	}
+
+	while (digit) {
 		/* See if a DTMF feature hook matches or can match */
 		hook = ao2_find(features->dtmf_hooks, bridge_channel->dtmf_hook_state.collected,
 			OBJ_SEARCH_PARTIAL_KEY);
@@ -1545,25 +1583,12 @@
 			ast_debug(1, "No DTMF feature hooks on %p(%s) match '%s'\n",
 				bridge_channel, ast_channel_name(bridge_channel->chan),
 				bridge_channel->dtmf_hook_state.collected);
+			break;
 		} else if (dtmf_len != strlen(hook->dtmf.code)) {
 			unsigned int digit_timeout;
-			struct ast_features_general_config *gen_cfg;
-
 			/* Need more digits to match */
 			ao2_ref(hook, -1);
-
-			/* Determine interdigit timeout */
-			ast_channel_lock(bridge_channel->chan);
-			gen_cfg = ast_get_chan_features_general_config(bridge_channel->chan);
-			ast_channel_unlock(bridge_channel->chan);
-			if (!gen_cfg) {
-				ast_log(LOG_ERROR, "Unable to retrieve features configuration.\n");
-				digit_timeout = 3000; /* Pick a reasonable failsafe timeout in ms */
-			} else {
-				digit_timeout = gen_cfg->featuredigittimeout;
-				ao2_ref(gen_cfg, -1);
-			}
-
+			digit_timeout = bridge_channel_feature_digit_timeout(bridge_channel);
 			bridge_channel->dtmf_hook_state.interdigit_timeout =
 				ast_tvadd(ast_tvnow(), ast_samp2tv(digit_timeout, 1000));
 			return;
@@ -1612,10 +1637,21 @@
 			 */
 			if (bridge_channel->chan && ast_check_hangup_locked(bridge_channel->chan)) {
 				ast_bridge_channel_kick(bridge_channel, 0);
+				bridge_channel->dtmf_hook_state.collected[0] = '\0';
+				return;
 			}
-			return;
-		}
-	} else {
+
+			/* if there is dtmf that has been collected then loop back through,
+			   but set digit to -1 so it doesn't try to do an add since the dtmf
+			   is already in the buffer */
+			dtmf_len = strlen(bridge_channel->dtmf_hook_state.collected);
+			if (!dtmf_len) {
+				return;
+			}
+		}
+	}
+
+	if (!digit) {
 		ast_debug(1, "DTMF feature string collection on %p(%s) timed out\n",
 			bridge_channel, ast_channel_name(bridge_channel->chan));
 	}




More information about the svn-commits mailing list