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


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@433447 65c4cc65-6c06-0410-ace0-fbb531ad65f3

3 files changed, 147 insertions, 52 deletions

diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index 0f1b977fa..cefc1337c 100644
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -723,6 +723,52 @@ static int conf_start_record(struct confbridge_conference *conference)
 	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 @@ static int sound_file_exists(const char *filename)
  *
  * \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 @@ static int announce_user_count(struct confbridge_conference *conference, struct
 	} 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 @@ static int announce_user_count(struct confbridge_conference *conference, struct
 			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 @@ static struct confbridge_conference *join_conference_bridge(const char *conferen
 
 	/* 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 @@ static struct confbridge_conference *join_conference_bridge(const char *conferen
 		 * 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 @@ confbridge_cleanup:
 }
 
 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 @@ static int action_toggle_mute(struct confbridge_conference *conference,
 		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 @@ static int action_kick_last(struct confbridge_conference *conference,
 	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 @@ static int action_kick_last(struct confbridge_conference *conference,
 	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 @@ static int execute_menu_entry(struct confbridge_conference *conference,
 	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 @@ static int execute_menu_entry(struct confbridge_conference *conference,
 			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 @@ static int execute_menu_entry(struct confbridge_conference *conference,
 				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);
diff --git a/include/asterisk/bridge_channel.h b/include/asterisk/bridge_channel.h
index c53848511..66f9be6e5 100644
--- a/include/asterisk/bridge_channel.h
+++ b/include/asterisk/bridge_channel.h
@@ -656,11 +656,26 @@ int ast_bridge_channel_write_park(struct ast_bridge_channel *bridge_channel, con
 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.
@@ -668,6 +683,10 @@ void ast_bridge_channel_kick(struct ast_bridge_channel *bridge_channel, int caus
  * \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
  */
 void ast_bridge_channel_feature_digit(struct ast_bridge_channel *bridge_channel, int digit);
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index e9f1ca0bd..29e7cf0f6 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -1510,6 +1510,51 @@ static void testsuite_notify_feature_success(struct ast_channel *chan, const cha
 #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 @@ void ast_bridge_channel_feature_digit(struct ast_bridge_channel *bridge_channel,
 	}
 
 	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 @@ void ast_bridge_channel_feature_digit(struct ast_bridge_channel *bridge_channel,
 			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 @@ void ast_bridge_channel_feature_digit(struct ast_bridge_channel *bridge_channel,
 			 */
 			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;
+			}
+
+			/* 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;
 			}
-			return;
 		}
-	} else {
+	}
+
+	if (!digit) {
 		ast_debug(1, "DTMF feature string collection on %p(%s) timed out\n",
 			bridge_channel, ast_channel_name(bridge_channel->chan));
 	}