bridge_holding/app_bridgewait: Add new entertainment options

This patch adds more entertainment options to holding bridges and the
bridge_wait application. Also, holding bridges will now use music on
hold as the default entertainment option instead of none. The
parameters for app_bridgewait have changed to (role, options) from
the previous (options) and the options themselves have changed as
well (entertainment options are now contained in an enumerator, role
specification is handled by the role parameter, etc)

(closes issue ASTERISK-21923)
Reported by: Matthew Jordan
Review: https://reviewboard.asterisk.org/r/2679/


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

3 files changed, 143 insertions, 51 deletions

diff --git a/apps/app_bridgewait.c b/apps/app_bridgewait.c
index 53165f545..356e62530 100644
--- a/apps/app_bridgewait.c
+++ b/apps/app_bridgewait.c
@@ -54,26 +54,44 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 			Put a call into the holding bridge.
 		</synopsis>
 		<syntax>
+			<parameter name="role" required="false">
+				<para>Defines the channel's purpose for entering the holding bridge. Values are case sensitive.
+				</para>
+				<enumlist>
+					<enum name="participant">
+						<para>The channel will enter the holding bridge to be placed on hold
+						until it is removed from the bridge for some reason. (default)</para>
+					</enum>
+					<enum name="announcer">
+						<para>The channel will enter the holding bridge to make announcements
+						to channels that are currently in the holding bridge. While an
+						announcer is present, holding for the participants will be
+						suspended.</para>
+					</enum>
+				</enumlist>
+			</parameter>
 			<parameter name="options">
 				<optionlist>
-					<option name="A">
-						<para>The channel will join the holding bridge as an
-						announcer</para>
-					</option>
 					<option name="m">
-						<argument name="class" required="false" />
-						<para>Play music on hold to the entering channel while it is
-						on hold. If the <emphasis>class</emphasis> is included, then
-						that class of music on hold will take priority over the
-						channel default.</para>
+						<argument name="class" required="true" />
+						<para>The specified MOH class will be used/suggested for
+						music on hold operations. This option will only be useful for
+						entertainment modes that use it (m and h).</para>
 					</option>
-					<option name="r">
-						<para>Play a ringing tone to the entering channel while it is
-						on hold.</para>
+					<option name="e">
+						<para>Which entertainment mechanism should be used while on hold
+						in the holding bridge. Only the first letter is read.</para>
+						<enumlist>
+							<enum name="m"><para>Play music on hold (default)</para></enum>
+							<enum name="r"><para>Ring without pause</para></enum>
+							<enum name="s"><para>Generate silent audio</para></enum>
+							<enum name="h"><para>Put the channel on hold</para></enum>
+							<enum name="n"><para>No entertainment</para></enum>
+						</enumlist>
 					</option>
 					<option name="S">
 						<argument name="duration" required="true" />
-						<para>Automatically end the hold and return to the PBX after
+						<para>Automatically exit the bridge and return to the PBX after
 						<emphasis>duration</emphasis> seconds.</para>
 					</option>
 				</optionlist>
@@ -87,12 +105,8 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 	</application>
  ***/
 /* BUGBUG Add bridge name/id parameter to specify which holding bridge to join (required) */
-/* BUGBUG Add h(moh-class) option to put channel on hold using AST_CONTROL_HOLD/AST_CONTROL_UNHOLD while in bridge */
-/* BUGBUG Add s option to send silence media frames to channel while in bridge (uses a silence generator) */
-/* BUGBUG Add n option to send no media to channel while in bridge (Channel should not be answered yet) */
 /* BUGBUG The channel may or may not be answered with the r option. */
 /* BUGBUG You should not place an announcer into a holding bridge with unanswered channels. */
-/* BUGBUG Not supplying any option flags will assume the m option with the default music class. */
 
 static char *app = "BridgeWait";
 static struct ast_bridge *holding_bridge;
@@ -100,22 +114,21 @@ static struct ast_bridge *holding_bridge;
 AST_MUTEX_DEFINE_STATIC(bridgewait_lock);
 
 enum bridgewait_flags {
-	MUXFLAG_PLAYMOH = (1 << 0),
-	MUXFLAG_RINGING = (1 << 1),
+	MUXFLAG_MOHCLASS = (1 << 0),
+	MUXFLAG_ENTERTAINMENT = (1 << 1),
 	MUXFLAG_TIMEOUT = (1 << 2),
-	MUXFLAG_ANNOUNCER = (1 << 3),
 };
 
 enum bridgewait_args {
+	OPT_ARG_ENTERTAINMENT,
 	OPT_ARG_MOHCLASS,
 	OPT_ARG_TIMEOUT,
 	OPT_ARG_ARRAY_SIZE, /* Always the last element of the enum */
 };
 
 AST_APP_OPTIONS(bridgewait_opts, {
-	AST_APP_OPTION('A', MUXFLAG_ANNOUNCER),
-	AST_APP_OPTION('r', MUXFLAG_RINGING),
-	AST_APP_OPTION_ARG('m', MUXFLAG_PLAYMOH, OPT_ARG_MOHCLASS),
+	AST_APP_OPTION_ARG('e', MUXFLAG_ENTERTAINMENT, OPT_ARG_ENTERTAINMENT),
+	AST_APP_OPTION_ARG('m', MUXFLAG_MOHCLASS, OPT_ARG_MOHCLASS),
 	AST_APP_OPTION_ARG('S', MUXFLAG_TIMEOUT, OPT_ARG_TIMEOUT),
 });
 
@@ -147,18 +160,38 @@ static int apply_option_timeout(struct ast_bridge_features *features, char *dura
 	return 0;
 }
 
-static void apply_option_moh(struct ast_channel *chan, char *class_arg)
+static int apply_option_moh(struct ast_channel *chan, const char *class_arg)
 {
-	ast_channel_set_bridge_role_option(chan, "holding_participant", "idle_mode", "musiconhold");
-	ast_channel_set_bridge_role_option(chan, "holding_participant", "moh_class", class_arg);
+	return ast_channel_set_bridge_role_option(chan, "holding_participant", "moh_class", class_arg);
 }
 
-static void apply_option_ringing(struct ast_channel *chan)
+static int apply_option_entertainment(struct ast_channel *chan, const char *entertainment_arg)
 {
-	ast_channel_set_bridge_role_option(chan, "holding_participant", "idle_mode", "ringing");
+	char entertainment = entertainment_arg[0];
+	switch (entertainment) {
+	case 'm':
+		return ast_channel_set_bridge_role_option(chan, "holding_participant", "idle_mode", "musiconhold");
+	case 'r':
+		return ast_channel_set_bridge_role_option(chan, "holding_participant", "idle_mode", "ringing");
+	case 's':
+		return ast_channel_set_bridge_role_option(chan, "holding_participant", "idle_mode", "silence");
+	case 'h':
+		return ast_channel_set_bridge_role_option(chan, "holding_participant", "idle_mode", "hold");
+	case 'n':
+		return ast_channel_set_bridge_role_option(chan, "holding_participant", "idle_mode", "none");
+	default:
+		ast_log(LOG_ERROR, "Invalid argument for BridgeWait entertainment '%s'\n", entertainment_arg);
+		return -1;
+	}
 }
 
-static int process_options(struct ast_channel *chan, struct ast_flags *flags, char **opts, struct ast_bridge_features *features)
+enum wait_bridge_roles {
+	ROLE_PARTICIPANT = 0,
+	ROLE_ANNOUNCER,
+	ROLE_INVALID,
+};
+
+static int process_options(struct ast_channel *chan, struct ast_flags *flags, char **opts, struct ast_bridge_features *features, enum wait_bridge_roles role)
 {
 	if (ast_test_flag(flags, MUXFLAG_TIMEOUT)) {
 		if (apply_option_timeout(features, opts[OPT_ARG_TIMEOUT])) {
@@ -166,30 +199,59 @@ static int process_options(struct ast_channel *chan, struct ast_flags *flags, ch
 		}
 	}
 
-	if (ast_test_flag(flags, MUXFLAG_ANNOUNCER)) {
-		/* Announcer specific stuff */
-		ast_channel_add_bridge_role(chan, "announcer");
-	} else {
-		/* Non Announcer specific stuff */
-		ast_channel_add_bridge_role(chan, "holding_participant");
+	switch (role) {
+	case ROLE_PARTICIPANT:
+		if (ast_channel_add_bridge_role(chan, "holding_participant")) {
+			return -1;
+		}
+
+		if (ast_test_flag(flags, MUXFLAG_MOHCLASS)) {
+			if (apply_option_moh(chan, opts[OPT_ARG_MOHCLASS])) {
+				return -1;
+			}
+		}
 
-		if (ast_test_flag(flags, MUXFLAG_PLAYMOH)) {
-			apply_option_moh(chan, opts[OPT_ARG_MOHCLASS]);
-		} else if (ast_test_flag(flags, MUXFLAG_RINGING)) {
-			apply_option_ringing(chan);
+		if (ast_test_flag(flags, MUXFLAG_ENTERTAINMENT)) {
+			if (apply_option_entertainment(chan, opts[OPT_ARG_ENTERTAINMENT])) {
+				return -1;
+			}
 		}
+
+		break;
+	case ROLE_ANNOUNCER:
+		if (ast_channel_add_bridge_role(chan, "announcer")) {
+			return -1;
+		}
+		break;
+	case ROLE_INVALID:
+		ast_assert(0);
+		return -1;
 	}
 
 	return 0;
 }
 
+static enum wait_bridge_roles validate_role(const char *role)
+{
+	if (!strcmp(role, "participant")) {
+		return ROLE_PARTICIPANT;
+	} else if (!strcmp(role, "announcer")) {
+		return ROLE_ANNOUNCER;
+	} else {
+		return ROLE_INVALID;
+	}
+}
+
 static int bridgewait_exec(struct ast_channel *chan, const char *data)
 {
 	struct ast_bridge_features chan_features;
 	struct ast_flags flags = { 0 };
 	char *parse;
+	enum wait_bridge_roles role = ROLE_PARTICIPANT;
+	char *opts[OPT_ARG_ARRAY_SIZE] = { NULL, };
 
 	AST_DECLARE_APP_ARGS(args,
+		AST_APP_ARG(role);
 		AST_APP_ARG(options);
 		AST_APP_ARG(other);		/* Any remaining unused arguments */
 	);
@@ -209,18 +271,26 @@ static int bridgewait_exec(struct ast_channel *chan, const char *data)
 	parse = ast_strdupa(data);
 	AST_STANDARD_APP_ARGS(args, parse);
 
+	if (!ast_strlen_zero(args.role)) {
+		role = validate_role(args.role);
+		if (role == ROLE_INVALID) {
+			ast_log(LOG_ERROR, "Requested waiting bridge role '%s' is invalid.\n", args.role);
+			return -1;
+		}
+	}
+
 	if (ast_bridge_features_init(&chan_features)) {
 		ast_bridge_features_cleanup(&chan_features);
 		return -1;
 	}
 
 	if (args.options) {
-		char *opts[OPT_ARG_ARRAY_SIZE] = { NULL, };
 		ast_app_parse_options(bridgewait_opts, &flags, opts, args.options);
-		if (process_options(chan, &flags, opts, &chan_features)) {
-			ast_bridge_features_cleanup(&chan_features);
-			return -1;
-		}
+	}
+
+	if (process_options(chan, &flags, opts, &chan_features, role)) {
+		ast_bridge_features_cleanup(&chan_features);
+		return -1;
 	}
 
 	ast_bridge_join(holding_bridge, chan, NULL, &chan_features, NULL, 0);
diff --git a/bridges/bridge_holding.c b/bridges/bridge_holding.c
index bd288c9bd..43dde5bdf 100644
--- a/bridges/bridge_holding.c
+++ b/bridges/bridge_holding.c
@@ -54,18 +54,18 @@ enum role_flags {
 	HOLDING_ROLE_ANNOUNCER = (1 << 1),
 };
 
-/* BUGBUG Add IDLE_MODE_HOLD option to put channel on hold using AST_CONTROL_HOLD/AST_CONTROL_UNHOLD while in bridge */
-/* BUGBUG Add IDLE_MODE_SILENCE to send silence media frames to channel while in bridge (uses a silence generator) */
-/* BUGBUG A channel without the holding_participant role will assume IDLE_MODE_MOH with the default music class. */
 enum idle_modes {
 	IDLE_MODE_NONE = 0,
 	IDLE_MODE_MOH,
 	IDLE_MODE_RINGING,
+	IDLE_MODE_SILENCE,
+	IDLE_MODE_HOLD,
 };
 
 /*! \brief Structure which contains per-channel role information */
 struct holding_channel {
 	struct ast_flags holding_roles;
+	struct ast_silence_generator *silence_generator;
 	enum idle_modes idle_mode;
 };
 
@@ -85,6 +85,15 @@ static void participant_stop_hold_audio(struct ast_bridge_channel *bridge_channe
 		break;
 	case IDLE_MODE_NONE:
 		break;
+	case IDLE_MODE_SILENCE:
+		if (hc->silence_generator) {
+			ast_channel_stop_silence_generator(bridge_channel->chan, hc->silence_generator);
+			hc->silence_generator = NULL;
+		}
+		break;
+	case IDLE_MODE_HOLD:
+		ast_bridge_channel_queue_control_data(bridge_channel, AST_CONTROL_UNHOLD, NULL, 0);
+		break;
 	}
 }
 
@@ -103,6 +112,7 @@ static void participant_start_hold_audio(struct ast_bridge_channel *bridge_chann
 {
 	struct holding_channel *hc = bridge_channel->tech_pvt;
 	const char *moh_class;
+	size_t moh_length;
 
 	if (!hc) {
 		return;
@@ -118,6 +128,14 @@ static void participant_start_hold_audio(struct ast_bridge_channel *bridge_chann
 		break;
 	case IDLE_MODE_NONE:
 		break;
+	case IDLE_MODE_SILENCE:
+		hc->silence_generator = ast_channel_start_silence_generator(bridge_channel->chan);
+		break;
+	case IDLE_MODE_HOLD:
+		moh_class = ast_bridge_channel_get_role_option(bridge_channel, "holding_participant", "moh_class");
+		moh_length = moh_class ? strlen(moh_class + 1) : 0;
+		ast_bridge_channel_queue_control_data(bridge_channel, AST_CONTROL_HOLD, moh_class, moh_length);
+		break;
 	}
 }
 
@@ -133,11 +151,17 @@ static void handle_participant_join(struct ast_bridge_channel *bridge_channel, s
 	}
 
 	if (ast_strlen_zero(idle_mode)) {
-		hc->idle_mode = IDLE_MODE_NONE;
+		hc->idle_mode = IDLE_MODE_MOH;
 	} else if (!strcmp(idle_mode, "musiconhold")) {
 		hc->idle_mode = IDLE_MODE_MOH;
 	} else if (!strcmp(idle_mode, "ringing")) {
 		hc->idle_mode = IDLE_MODE_RINGING;
+	} else if (!strcmp(idle_mode, "none")) {
+		hc->idle_mode = IDLE_MODE_NONE;
+	} else if (!strcmp(idle_mode, "silence")) {
+		hc->idle_mode = IDLE_MODE_SILENCE;
+	} else if (!strcmp(idle_mode, "hold")) {
+		hc->idle_mode = IDLE_MODE_HOLD;
 	} else {
 		ast_debug(2, "channel %s idle mode '%s' doesn't match any expected idle mode\n", ast_channel_name(us), idle_mode);
 	}
diff --git a/main/bridging_roles.c b/main/bridging_roles.c
index 079cbdb33..a50ade406 100644
--- a/main/bridging_roles.c
+++ b/main/bridging_roles.c
@@ -371,9 +371,7 @@ int ast_channel_set_bridge_role_option(struct ast_channel *channel, const char *
 		return 0;
 	}
 
-	setup_bridge_role_option(role, option, value);
-
-	return 0;
+	return setup_bridge_role_option(role, option, value);
 }
 
 int ast_bridge_channel_has_role(struct ast_bridge_channel *bridge_channel, const char *role_name)