Fix several interrelated issues dealing with the holding bridge technology.

* Added an option flags parameter to interval hooks.  Interval hooks now
can specify if the callback will affect the media path or not.

* Added an option flags parameter to the bridge action custom callback.
The action callback now can specify if the callback will affect the media
path or not.

* Made the holding bridge technology reexamine the participant idle mode
option whenever the entertainment is restarted.

* Fixed app_agent_pool waiting agents needlessly starting and stopping MOH
every second by specifying the heartbeat interval hook as not affecting
the media path.

* Fixed app_agent_pool agent alert from restarting the MOH after the alert
beep.  The agent entertainment is now changed from MOH to silence after
the alert beep.

* Fixed holding bridge technology to defer starting the entertainment.  It
was previously a mixture of immediate and deferred.

* Fixed holding bridge technology to immediately stop the entertainment.
It was previously a mixture of immediate and deferred.  If the channel
left the bridging system, any deferred stopping was discarded before
taking effect.

* Miscellaneous holding bridge technology rework coding improvements.

Review: https://reviewboard.asterisk.org/r/2761/


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

apps/app_agent_pool.c

@@ -1202,7 +1202,7 @@ static int bridge_agent_hold_push(struct ast_bridge *self, struct ast_bridge_cha
 
 	/* Add heartbeat interval hook. */
 	ao2_ref(agent, +1);
-	if (ast_bridge_interval_hook(bridge_channel->features, 1000,
+	if (ast_bridge_interval_hook(bridge_channel->features, 0, 1000,
 		bridge_agent_hold_heartbeat, agent, __ao2_cleanup, AST_BRIDGE_HOOK_REMOVE_ON_PULL)) {
 		ao2_ref(agent, -1);
 		res = -1;
@@ -1696,6 +1696,13 @@ static void agent_alert(struct ast_bridge_channel *bridge_channel, const void *p
 		return;
 	}
 
+	/* Change holding bridge participant role's idle mode to silence */
+	ast_bridge_channel_lock_bridge(bridge_channel);
+	ast_bridge_channel_clear_roles(bridge_channel);
+	ast_channel_set_bridge_role_option(bridge_channel->chan, "holding_participant", "idle_mode", "silence");
+	ast_bridge_channel_establish_roles(bridge_channel);
+	ast_bridge_unlock(bridge_channel->bridge);
+
 	/* Alert the agent. */
 	agent_lock(agent);
 	playfile = ast_strdupa(agent->cfg->beep_sound);
@@ -1724,8 +1731,8 @@ static void agent_alert(struct ast_bridge_channel *bridge_channel, const void *p
 
 static int send_alert_to_agent(struct ast_bridge_channel *bridge_channel, const char *agent_id)
 {
-	return ast_bridge_channel_queue_callback(bridge_channel, agent_alert, agent_id,
+	return ast_bridge_channel_queue_callback(bridge_channel,
-		strlen(agent_id) + 1);
+		AST_BRIDGE_CHANNEL_CB_OPTION_MEDIA, agent_alert, agent_id, strlen(agent_id) + 1);
 }
 
 static int send_colp_to_agent(struct ast_bridge_channel *bridge_channel, struct ast_party_connected_line *connected)
@@ -1797,7 +1804,7 @@ static int agent_request_exec(struct ast_channel *chan, const char *data)
 
 	/* Add safety timeout hook. */
 	ao2_ref(agent, +1);
-	if (ast_bridge_interval_hook(&caller_features, CALLER_SAFETY_TIMEOUT_TIME,
+	if (ast_bridge_interval_hook(&caller_features, 0, CALLER_SAFETY_TIMEOUT_TIME,
 		caller_safety_timeout, agent, __ao2_cleanup, AST_BRIDGE_HOOK_REMOVE_ON_PULL)) {
 		ao2_ref(agent, -1);
 		ast_bridge_features_cleanup(&caller_features);

apps/app_bridgewait.c

@@ -223,7 +223,7 @@ static int apply_option_timeout(struct ast_bridge_features *features, char *dura
 	}
 
 	duration *= 1000;
-	if (ast_bridge_interval_hook(features, duration, bridgewait_timeout_callback,
+	if (ast_bridge_interval_hook(features, 0, duration, bridgewait_timeout_callback,
 		NULL, NULL, AST_BRIDGE_HOOK_REMOVE_ON_PULL)) {
 		ast_log(LOG_ERROR, "Timeout option 'S': Could not create timer.\n");
 		return -1;

bridges/bridge_builtin_interval_features.c

@@ -170,7 +170,8 @@ static int bridge_builtin_set_limits(struct ast_bridge_features *features,
 
 	/* Install limit hooks. */
 	ao2_ref(feature_limits, +1);
-	if (ast_bridge_interval_hook(features, feature_limits->duration,
+	if (ast_bridge_interval_hook(features, AST_BRIDGE_HOOK_TIMER_OPTION_MEDIA,
+		feature_limits->duration,
 		bridge_features_duration_callback, feature_limits, __ao2_cleanup, remove_flags)) {
 		ast_log(LOG_ERROR, "Failed to schedule the duration limiter to the bridge channel.\n");
 		ao2_ref(feature_limits, -1);
@@ -178,7 +179,7 @@ static int bridge_builtin_set_limits(struct ast_bridge_features *features,
 	}
 	if (!ast_strlen_zero(feature_limits->connect_sound)) {
 		ao2_ref(feature_limits, +1);
-		if (ast_bridge_interval_hook(features, 1,
+		if (ast_bridge_interval_hook(features, AST_BRIDGE_HOOK_TIMER_OPTION_MEDIA, 1,
 			bridge_features_connect_callback, feature_limits, __ao2_cleanup, remove_flags)) {
 			ast_log(LOG_WARNING, "Failed to schedule connect sound to the bridge channel.\n");
 			ao2_ref(feature_limits, -1);
@@ -186,7 +187,7 @@ static int bridge_builtin_set_limits(struct ast_bridge_features *features,
 	}
 	if (feature_limits->warning && feature_limits->warning < feature_limits->duration) {
 		ao2_ref(feature_limits, +1);
-		if (ast_bridge_interval_hook(features,
+		if (ast_bridge_interval_hook(features, AST_BRIDGE_HOOK_TIMER_OPTION_MEDIA,
 			feature_limits->duration - feature_limits->warning,
 			bridge_features_warning_callback, feature_limits, __ao2_cleanup, remove_flags)) {
 			ast_log(LOG_WARNING, "Failed to schedule warning sound playback to the bridge channel.\n");

bridges/bridge_holding.c

@@ -49,13 +49,13 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #include "asterisk/frame.h"
 #include "asterisk/musiconhold.h"
 
-enum role_flags {
+enum holding_roles {
-	HOLDING_ROLE_PARTICIPANT = (1 << 0),
+	HOLDING_ROLE_PARTICIPANT,
-	HOLDING_ROLE_ANNOUNCER = (1 << 1),
+	HOLDING_ROLE_ANNOUNCER,
 };
 
 enum idle_modes {
-	IDLE_MODE_NONE = 0,
+	IDLE_MODE_NONE,
 	IDLE_MODE_MOH,
 	IDLE_MODE_RINGING,
 	IDLE_MODE_SILENCE,
@@ -64,91 +64,62 @@ enum idle_modes {
 
 /*! \brief Structure which contains per-channel role information */
 struct holding_channel {
-	struct ast_flags holding_roles;
 	struct ast_silence_generator *silence_generator;
+	enum holding_roles role;
 	enum idle_modes idle_mode;
+	/*! TRUE if the entertainment is started. */
+	unsigned int entertainment_active:1;
 };
 
-static void participant_stop_hold_audio(struct ast_bridge_channel *bridge_channel)
+typedef void (*deferred_cb)(struct ast_bridge_channel *bridge_channel);
-{
-	struct holding_channel *hc = bridge_channel->tech_pvt;
-	if (!hc) {
-		return;
-	}
 
-	switch (hc->idle_mode) {
+struct deferred_data {
-	case IDLE_MODE_MOH:
+	/*! Deferred holding technology callback */
-		ast_moh_stop(bridge_channel->chan);
+	deferred_cb callback;
-		break;
+};
-	case IDLE_MODE_RINGING:
+
-		ast_bridge_channel_queue_control_data(bridge_channel, -1, NULL, 0);
+static void deferred_action(struct ast_bridge_channel *bridge_channel, const void *payload, size_t payload_size);
-		break;
+
-	case IDLE_MODE_NONE:
+/*!
-		break;
+ * \internal
-	case IDLE_MODE_SILENCE:
+ * \brief Defer an action to a bridge_channel.
-		if (hc->silence_generator) {
+ * \since 12.0.0
-			ast_channel_stop_silence_generator(bridge_channel->chan, hc->silence_generator);
+ *
-			hc->silence_generator = NULL;
+ * \param bridge_channel Which channel to operate on.
-		}
+ * \param callback action to defer.
-		break;
+ *
-	case IDLE_MODE_HOLD:
+ * \retval 0 on success.
-		ast_bridge_channel_queue_control_data(bridge_channel, AST_CONTROL_UNHOLD, NULL, 0);
+ * \retval -1 on error.
-		break;
+ */
+static int defer_action(struct ast_bridge_channel *bridge_channel, deferred_cb callback)
+{
+	struct deferred_data data = { .callback = callback };
+	int res;
+
+	res = ast_bridge_channel_queue_callback(bridge_channel, 0, deferred_action,
+		&data, sizeof(data));
+	if (res) {
+		ast_log(LOG_WARNING, "Bridge %s: Could not defer action on %s.\n",
+			bridge_channel->bridge->uniqueid, ast_channel_name(bridge_channel->chan));
 	}
+	return res;
 }
 
-static void participant_reaction_announcer_join(struct ast_bridge_channel *bridge_channel)
+/*!
+ * \internal
+ * \brief Setup participant idle mode from channel.
+ * \since 12.0.0
+ *
+ * \param bridge_channel Channel to setup idle mode.
+ *
+ * \return Nothing
+ */
+static void participant_idle_mode_setup(struct ast_bridge_channel *bridge_channel)
 {
-	struct ast_channel *chan;
-	chan = bridge_channel->chan;
-	participant_stop_hold_audio(bridge_channel);
-	if (ast_set_write_format_by_id(chan, AST_FORMAT_SLINEAR)) {
-		ast_log(LOG_WARNING, "Could not make participant %s compatible.\n", ast_channel_name(chan));
-	}
-}
-
-/* This should only be called on verified holding_participants. */
-static void participant_start_hold_audio(struct ast_bridge_channel *bridge_channel)
-{
-	struct holding_channel *hc = bridge_channel->tech_pvt;
-	const char *moh_class;
-	size_t moh_length;
-
-	if (!hc) {
-		return;
-	}
-
-	switch(hc->idle_mode) {
-	case IDLE_MODE_MOH:
-		moh_class = ast_bridge_channel_get_role_option(bridge_channel, "holding_participant", "moh_class");
-		ast_moh_start(bridge_channel->chan, ast_strlen_zero(moh_class) ? NULL : moh_class, NULL);
-		break;
-	case IDLE_MODE_RINGING:
-		ast_bridge_channel_queue_control_data(bridge_channel, AST_CONTROL_RINGING, NULL, 0);
-		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;
-	}
-}
-
-static void handle_participant_join(struct ast_bridge_channel *bridge_channel, struct ast_bridge_channel *announcer_channel)
-{
-	struct ast_channel *us = bridge_channel->chan;
-	struct holding_channel *hc = bridge_channel->tech_pvt;
 	const char *idle_mode = ast_bridge_channel_get_role_option(bridge_channel, "holding_participant", "idle_mode");
+	struct holding_channel *hc = bridge_channel->tech_pvt;
 
-
+	ast_assert(hc != NULL);
-	if (!hc) {
-		return;
-	}
 
 	if (ast_strlen_zero(idle_mode)) {
 		hc->idle_mode = IDLE_MODE_MOH;
@@ -163,16 +134,105 @@ static void handle_participant_join(struct ast_bridge_channel *bridge_channel, s
 	} 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);
+		/* Invalid idle mode requested. */
+		ast_debug(1, "channel %s idle mode '%s' doesn't match any defined idle mode\n",
+			ast_channel_name(bridge_channel->chan), idle_mode);
+		ast_assert(0);
 	}
+}
+
+static void participant_entertainment_stop(struct ast_bridge_channel *bridge_channel)
+{
+	struct holding_channel *hc = bridge_channel->tech_pvt;
+
+	ast_assert(hc != NULL);
+
+	if (!hc->entertainment_active) {
+		/* Already stopped */
+		return;
+	}
+	hc->entertainment_active = 0;
+
+	switch (hc->idle_mode) {
+	case IDLE_MODE_MOH:
+		ast_moh_stop(bridge_channel->chan);
+		break;
+	case IDLE_MODE_RINGING:
+		ast_indicate(bridge_channel->chan, -1);
+		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_indicate(bridge_channel->chan, AST_CONTROL_UNHOLD);
+		break;
+	}
+}
+
+static void participant_reaction_announcer_join(struct ast_bridge_channel *bridge_channel)
+{
+	struct ast_channel *chan;
+
+	chan = bridge_channel->chan;
+	participant_entertainment_stop(bridge_channel);
+	if (ast_set_write_format_by_id(chan, AST_FORMAT_SLINEAR)) {
+		ast_log(LOG_WARNING, "Could not make participant %s compatible.\n", ast_channel_name(chan));
+	}
+}
+
+/* This should only be called on verified holding_participants. */
+static void participant_entertainment_start(struct ast_bridge_channel *bridge_channel)
+{
+	struct holding_channel *hc = bridge_channel->tech_pvt;
+	const char *moh_class;
+	size_t moh_length;
+
+	ast_assert(hc != NULL);
+
+	if (hc->entertainment_active) {
+		/* Already started */
+		return;
+	}
+	hc->entertainment_active = 1;
+
+	participant_idle_mode_setup(bridge_channel);
+	switch(hc->idle_mode) {
+	case IDLE_MODE_MOH:
+		moh_class = ast_bridge_channel_get_role_option(bridge_channel, "holding_participant", "moh_class");
+		ast_moh_start(bridge_channel->chan, ast_strlen_zero(moh_class) ? NULL : moh_class, NULL);
+		break;
+	case IDLE_MODE_RINGING:
+		ast_indicate(bridge_channel->chan, AST_CONTROL_RINGING);
+		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_indicate_data(bridge_channel->chan, AST_CONTROL_HOLD, moh_class, moh_length);
+		break;
+	}
+}
+
+static void handle_participant_join(struct ast_bridge_channel *bridge_channel, struct ast_bridge_channel *announcer_channel)
+{
+	struct ast_channel *us = bridge_channel->chan;
 
 	/* If the announcer channel isn't present, we need to set up ringing, music on hold, or whatever. */
 	if (!announcer_channel) {
-		participant_start_hold_audio(bridge_channel);
+		defer_action(bridge_channel, participant_entertainment_start);
 		return;
 	}
 
-	/* If it is present though, we need to establish compatability. */
+	/* We need to get compatible with the announcer. */
 	if (ast_set_write_format_by_id(us, AST_FORMAT_SLINEAR)) {
 		ast_log(LOG_WARNING, "Could not make participant %s compatible.\n", ast_channel_name(us));
 	}
@@ -185,6 +245,8 @@ static int holding_bridge_join(struct ast_bridge *bridge, struct ast_bridge_chan
 	struct holding_channel *hc;
 	struct ast_channel *us = bridge_channel->chan; /* The joining channel */
 
+	ast_assert(bridge_channel->tech_pvt == NULL);
+
 	if (!(hc = ast_calloc(1, sizeof(*hc)))) {
 		return -1;
 	}
@@ -195,54 +257,44 @@ static int holding_bridge_join(struct ast_bridge *bridge, struct ast_bridge_chan
 	announcer_channel = bridge->tech_pvt;
 
 	if (ast_bridge_channel_has_role(bridge_channel, "announcer")) {
-		/* If another announcer already exists, scrap the holding channel struct so we know to ignore it in the future */
 		if (announcer_channel) {
+			/* Another announcer already exists. */
 			bridge_channel->tech_pvt = NULL;
 			ast_free(hc);
-			ast_log(LOG_WARNING, "A second announcer channel %s attempted to enter a holding bridge.\n",
+			ast_log(LOG_WARNING, "Bridge %s: Channel %s tried to be an announcer.  Bridge already has one.\n",
-				ast_channel_name(announcer_channel->chan));
+				bridge->uniqueid, ast_channel_name(bridge_channel->chan));
 			return -1;
 		}
 
 		bridge->tech_pvt = bridge_channel;
-		ast_set_flag(&hc->holding_roles, HOLDING_ROLE_ANNOUNCER);
+		hc->role = HOLDING_ROLE_ANNOUNCER;
 
 		/* The announcer should always be made compatible with signed linear */
 		if (ast_set_read_format_by_id(us, AST_FORMAT_SLINEAR)) {
 			ast_log(LOG_ERROR, "Could not make announcer %s compatible.\n", ast_channel_name(us));
 		}
 
-		/* Make everyone compatible. While we are at it we should stop music on hold and ringing. */
+		/* Make everyone listen to the announcer. */
 		AST_LIST_TRAVERSE(&bridge->channels, other_channel, entry) {
 			/* Skip the reaction if we are the channel in question */
 			if (bridge_channel == other_channel) {
 				continue;
 			}
-			participant_reaction_announcer_join(other_channel);
+			defer_action(other_channel, participant_reaction_announcer_join);
 		}
 
 		return 0;
 	}
 
-	/* If the entering channel isn't an announcer then we need to setup it's properties and put it in its holding state if necessary */
+	hc->role = HOLDING_ROLE_PARTICIPANT;
-	ast_set_flag(&hc->holding_roles, HOLDING_ROLE_PARTICIPANT);
 	handle_participant_join(bridge_channel, announcer_channel);
 	return 0;
 }
 
 static void participant_reaction_announcer_leave(struct ast_bridge_channel *bridge_channel)
 {
-	struct holding_channel *hc = bridge_channel->tech_pvt;
-
-	if (!hc) {
-		/* We are dealing with a channel that failed to join properly. Skip it. */
-		return;
-	}
-
 	ast_bridge_channel_restore_formats(bridge_channel);
-	if (ast_test_flag(&hc->holding_roles, HOLDING_ROLE_PARTICIPANT)) {
+	participant_entertainment_start(bridge_channel);
-		participant_start_hold_audio(bridge_channel);
-	}
 }
 
 static void holding_bridge_leave(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
@@ -254,27 +306,23 @@ static void holding_bridge_leave(struct ast_bridge *bridge, struct ast_bridge_ch
 		return;
 	}
 
-	if (!ast_test_flag(&hc->holding_roles, HOLDING_ROLE_ANNOUNCER)) {
+	switch (hc->role) {
-		/* It's not an announcer so nothing needs to react to its departure. Just free the tech_pvt. */
+	case HOLDING_ROLE_ANNOUNCER:
-		if (!bridge->tech_pvt) {
+		/* The announcer is leaving */
-			/* Since no announcer is in the channel, we may be playing MOH/ringing. Stop that. */
+		bridge->tech_pvt = NULL;
-			participant_stop_hold_audio(bridge_channel);
+
+		/* Reset the other channels back to moh/ringing. */
+		AST_LIST_TRAVERSE(&bridge->channels, other_channel, entry) {
+			defer_action(other_channel, participant_reaction_announcer_leave);
 		}
-		ast_free(hc);
+		break;
-		bridge_channel->tech_pvt = NULL;
+	default:
-		return;
+		/* Nothing needs to react to its departure. */
+		participant_entertainment_stop(bridge_channel);
+		break;
 	}
-
-	/* When the announcer leaves, the other channels should reset their formats and go back to moh/ringing */
-	AST_LIST_TRAVERSE(&bridge->channels, other_channel, entry) {
-		participant_reaction_announcer_leave(other_channel);
-	}
-
-	/* Since the announcer is leaving, we should clear the tech_pvt pointing to it */
-	bridge->tech_pvt = NULL;
-
-	ast_free(hc);
 	bridge_channel->tech_pvt = NULL;
+	ast_free(hc);
 }
 
 static int holding_bridge_write(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
@@ -287,57 +335,57 @@ static int holding_bridge_write(struct ast_bridge *bridge, struct ast_bridge_cha
 		return 0;
 	}
 
-	/* If we aren't an announcer, we never have any business writing anything. */
+	switch (hc->role) {
-	if (!ast_test_flag(&hc->holding_roles, HOLDING_ROLE_ANNOUNCER)) {
+	case HOLDING_ROLE_ANNOUNCER:
+		/* Write the frame to all other channels if any. */
+		ast_bridge_queue_everyone_else(bridge, bridge_channel, frame);
+		break;
+	default:
 		/* "Accept" the frame and discard it. */
-		return 0;
+		break;
 	}
 
-	/*
-	 * Ok, so we are the announcer.  Write the frame to all other
-	 * channels if any.
-	 */
-	ast_bridge_queue_everyone_else(bridge, bridge_channel, frame);
-
 	return 0;
 }
 
 static void holding_bridge_suspend(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
 {
-	struct holding_channel *hc = bridge_channel ? bridge_channel->tech_pvt : NULL;
+	struct holding_channel *hc = bridge_channel->tech_pvt;
 
 	if (!hc) {
 		return;
 	}
 
-	if (!ast_test_flag(&hc->holding_roles, HOLDING_ROLE_PARTICIPANT)) {
+	switch (hc->role) {
-		return;
+	case HOLDING_ROLE_PARTICIPANT:
+		participant_entertainment_stop(bridge_channel);
+		break;
+	default:
+		break;
 	}
-
-	participant_stop_hold_audio(bridge_channel);
 }
 
 static void holding_bridge_unsuspend(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
 {
-	struct holding_channel *hc = bridge_channel ? bridge_channel->tech_pvt : NULL;
+	struct holding_channel *hc = bridge_channel->tech_pvt;
 	struct ast_bridge_channel *announcer_channel = bridge->tech_pvt;
 
 	if (!hc) {
 		return;
 	}
 
-	/* If the bridge channel being unsuspended is not a participant, no need to do anything. */
+	switch (hc->role) {
-	if (!ast_test_flag(&hc->holding_roles, HOLDING_ROLE_PARTICIPANT)) {
+	case HOLDING_ROLE_PARTICIPANT:
-		return;
+		if (announcer_channel) {
+			/* There is an announcer channel in the bridge. */
+			break;
+		}
+		/* We need to restart the entertainment. */
+		participant_entertainment_start(bridge_channel);
+		break;
+	default:
+		break;
 	}
-
-	/* If there is currently an announcer channel in the bridge, there is also no need to do anything. */
-	if (announcer_channel) {
-		return;
-	}
-
-	/* Otherwise we need to resume the entertainment. */
-	participant_start_hold_audio(bridge_channel);
 }
 
 static struct ast_bridge_technology holding_bridge = {
@@ -351,6 +399,32 @@ static struct ast_bridge_technology holding_bridge = {
 	.unsuspend = holding_bridge_unsuspend,
 };
 
+/*!
+ * \internal
+ * \brief Deferred action to start/stop participant entertainment.
+ * \since 12.0.0
+ *
+ * \param bridge_channel Which channel to operate on.
+ * \param payload Data to pass to the callback. (NULL if none).
+ * \param payload_size Size of the payload if payload is non-NULL.  A number otherwise.
+ *
+ * \return Nothing
+ */
+static void deferred_action(struct ast_bridge_channel *bridge_channel, const void *payload, size_t payload_size)
+{
+	const struct deferred_data *data = payload;
+
+	ast_bridge_channel_lock_bridge(bridge_channel);
+	if (bridge_channel->bridge->technology != &holding_bridge
+		|| !bridge_channel->tech_pvt) {
+		/* Not valid anymore. */
+		ast_bridge_unlock(bridge_channel->bridge);
+		return;
+	}
+	data->callback(bridge_channel);
+	ast_bridge_unlock(bridge_channel->bridge);
+}
+
 static int unload_module(void)
 {
 	ast_format_cap_destroy(holding_bridge.format_capabilities);

include/asterisk/bridge_channel.h

@@ -530,11 +530,17 @@ int ast_bridge_channel_queue_playfile(struct ast_bridge_channel *bridge_channel,
  */
 typedef void (*ast_bridge_custom_callback_fn)(struct ast_bridge_channel *bridge_channel, const void *payload, size_t payload_size);
 
+enum ast_bridge_channel_custom_callback_option {
+	/*! The callback temporarily affects media. (Like a custom playfile.) */
+	AST_BRIDGE_CHANNEL_CB_OPTION_MEDIA = (1 << 0),
+};
+
 /*!
  * \brief Write a bridge action custom callback frame into the bridge.
  * \since 12.0.0
  *
  * \param bridge_channel Which channel is putting the frame into the bridge
+ * \param flags Custom callback option flags.
  * \param callback Custom callback run on a bridge channel.
  * \param payload Data to pass to the callback. (NULL if none).
  * \param payload_size Size of the payload if payload is non-NULL.  A number otherwise.
@@ -546,13 +552,16 @@ typedef void (*ast_bridge_custom_callback_fn)(struct ast_bridge_channel *bridge_
  * \retval 0 on success.
  * \retval -1 on error.
  */
-int ast_bridge_channel_write_callback(struct ast_bridge_channel *bridge_channel, ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size);
+int ast_bridge_channel_write_callback(struct ast_bridge_channel *bridge_channel,
+	enum ast_bridge_channel_custom_callback_option flags,
+	ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size);
 
 /*!
  * \brief Queue a bridge action custom callback frame onto the bridge channel.
  * \since 12.0.0
  *
  * \param bridge_channel Which channel to put the frame onto.
+ * \param flags Custom callback option flags.
  * \param callback Custom callback run on a bridge channel.
  * \param payload Data to pass to the callback. (NULL if none).
  * \param payload_size Size of the payload if payload is non-NULL.  A number otherwise.
@@ -564,7 +573,9 @@ int ast_bridge_channel_write_callback(struct ast_bridge_channel *bridge_channel,
  * \retval 0 on success.
  * \retval -1 on error.
  */
-int ast_bridge_channel_queue_callback(struct ast_bridge_channel *bridge_channel, ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size);
+int ast_bridge_channel_queue_callback(struct ast_bridge_channel *bridge_channel,
+	enum ast_bridge_channel_custom_callback_option flags,
+	ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size);
 
 /*!
  * \brief Have a bridge channel park a channel in the bridge

include/asterisk/bridge_features.h

@@ -208,6 +208,11 @@ struct ast_bridge_hook_dtmf {
 	struct ast_bridge_hook_dtmf_parms dtmf;
 };
 
+enum ast_bridge_hook_timer_option {
+	/*! The timer temporarily affects media. (Like a custom playfile.) */
+	AST_BRIDGE_HOOK_TIMER_OPTION_MEDIA = (1 << 0),
+};
+
 /*! Extra parameters for an interval timer hook. */
 struct ast_bridge_hook_timer_parms {
 	/*! Time at which the hook should actually trip */
@@ -218,6 +223,8 @@ struct ast_bridge_hook_timer_parms {
 	unsigned int interval;
 	/*! Sequence number for the hook to ensure expiration ordering */
 	unsigned int seqno;
+	/*! Option flags determining how callback is called. */
+	unsigned int flags;
 };
 
 /*! Timer specific hook. */
@@ -553,6 +560,7 @@ int ast_bridge_dtmf_hook(struct ast_bridge_features *features,
  * \brief Attach an interval hook to a bridge features structure
  *
  * \param features Bridge features structure
+ * \param flags Interval timer callback option flags.
  * \param interval The interval that the hook should execute at in milliseconds
  * \param callback Function to execute upon activation
  * \param hook_pvt Unique data
@@ -572,6 +580,7 @@ int ast_bridge_dtmf_hook(struct ast_bridge_features *features,
  * data may be provided to the hook_pvt parameter.
  */
 int ast_bridge_interval_hook(struct ast_bridge_features *features,
+	enum ast_bridge_hook_timer_option flags,
 	unsigned int interval,
 	ast_bridge_hook_callback callback,
 	void *hook_pvt,

main/bridge.c

@@ -1699,7 +1699,7 @@ int ast_bridge_kick(struct ast_bridge *bridge, struct ast_channel *chan)
 		return -1;
 	}
 
-	res = ast_bridge_channel_queue_callback(bridge_channel, kick_it, NULL, 0);
+	res = ast_bridge_channel_queue_callback(bridge_channel, 0, kick_it, NULL, 0);
 
 	ast_bridge_unlock(bridge);
 
@@ -2963,6 +2963,7 @@ int ast_bridge_talk_detector_hook(struct ast_bridge_features *features,
 }
 
 int ast_bridge_interval_hook(struct ast_bridge_features *features,
+	enum ast_bridge_hook_timer_option flags,
 	unsigned int interval,
 	ast_bridge_hook_callback callback,
 	void *hook_pvt,
@@ -2984,8 +2985,9 @@ int ast_bridge_interval_hook(struct ast_bridge_features *features,
 	}
 	hook->generic.type = AST_BRIDGE_HOOK_TYPE_TIMER;
 	hook->timer.interval = interval;
-	hook->timer.trip_time = ast_tvadd(ast_tvnow(), ast_samp2tv(hook->timer.interval, 1000));
+	hook->timer.trip_time = ast_tvadd(ast_tvnow(), ast_samp2tv(interval, 1000));
 	hook->timer.seqno = ast_atomic_fetchadd_int((int *) &features->interval_sequence, +1);
+	hook->timer.flags = flags;
 
 	ast_debug(1, "Putting interval hook %p with interval %u in the heap on features %p\n",
 		hook, hook->timer.interval, features);

main/bridge_channel.c

@@ -320,6 +320,87 @@ static int bridge_channel_write_frame(struct ast_bridge_channel *bridge_channel,
 	return 0;
 }
 
+/*!
+ * \internal
+ * \brief Suspend a channel from a bridge.
+ *
+ * \param bridge_channel Channel to suspend.
+ *
+ * \note This function assumes bridge_channel->bridge is locked.
+ *
+ * \return Nothing
+ */
+void bridge_channel_internal_suspend_nolock(struct ast_bridge_channel *bridge_channel)
+{
+	bridge_channel->suspended = 1;
+	if (bridge_channel->in_bridge) {
+		--bridge_channel->bridge->num_active;
+	}
+
+	/* Get technology bridge threads off of the channel. */
+	if (bridge_channel->bridge->technology->suspend) {
+		bridge_channel->bridge->technology->suspend(bridge_channel->bridge, bridge_channel);
+	}
+}
+
+/*!
+ * \internal
+ * \brief Suspend a channel from a bridge.
+ *
+ * \param bridge_channel Channel to suspend.
+ *
+ * \return Nothing
+ */
+static void bridge_channel_suspend(struct ast_bridge_channel *bridge_channel)
+{
+	ast_bridge_channel_lock_bridge(bridge_channel);
+	bridge_channel_internal_suspend_nolock(bridge_channel);
+	ast_bridge_unlock(bridge_channel->bridge);
+}
+
+/*!
+ * \internal
+ * \brief Unsuspend a channel from a bridge.
+ *
+ * \param bridge_channel Channel to unsuspend.
+ *
+ * \note This function assumes bridge_channel->bridge is locked.
+ *
+ * \return Nothing
+ */
+void bridge_channel_internal_unsuspend_nolock(struct ast_bridge_channel *bridge_channel)
+{
+	bridge_channel->suspended = 0;
+	if (bridge_channel->in_bridge) {
+		++bridge_channel->bridge->num_active;
+	}
+
+	/* Wake technology bridge threads to take care of channel again. */
+	if (bridge_channel->bridge->technology->unsuspend) {
+		bridge_channel->bridge->technology->unsuspend(bridge_channel->bridge, bridge_channel);
+	}
+
+	/* Wake suspended channel. */
+	ast_bridge_channel_lock(bridge_channel);
+	ast_cond_signal(&bridge_channel->cond);
+	ast_bridge_channel_unlock(bridge_channel);
+}
+
+/*!
+ * \internal
+ * \brief Unsuspend a channel from a bridge.
+ *
+ * \param bridge_channel Channel to unsuspend.
+ *
+ * \return Nothing
+ */
+static void bridge_channel_unsuspend(struct ast_bridge_channel *bridge_channel)
+{
+	ast_bridge_channel_lock_bridge(bridge_channel);
+	bridge_channel_internal_unsuspend_nolock(bridge_channel);
+	ast_bridge_unlock(bridge_channel->bridge);
+}
+
 /*!
  * \internal
  * \brief Queue an action frame onto the bridge channel with data.
@@ -680,6 +761,8 @@ struct bridge_custom_callback {
 	ast_bridge_custom_callback_fn callback;
 	/*! Size of the payload if it exists.  A number otherwise. */
 	size_t payload_size;
+	/*! Option flags determining how callback is called. */
+	unsigned int flags;
 	/*! Nonzero if the payload exists. */
 	char payload_exists;
 	/*! Payload to give to callback. */
@@ -691,14 +774,22 @@ struct bridge_custom_callback {
  * \brief Handle the do custom callback bridge action.
  * \since 12.0.0
  *
- * \param bridge_channel Which channel to run the application on.
+ * \param bridge_channel Which channel to call the callback on.
- * \param data Action frame data to run the application.
+ * \param data Action frame data to call the callback.
  *
  * \return Nothing
  */
 static void bridge_channel_do_callback(struct ast_bridge_channel *bridge_channel, struct bridge_custom_callback *data)
 {
+	if (ast_test_flag(data, AST_BRIDGE_CHANNEL_CB_OPTION_MEDIA)) {
+		bridge_channel_suspend(bridge_channel);
+		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
+	}
 	data->callback(bridge_channel, data->payload_exists ? data->payload : NULL, data->payload_size);
+	if (ast_test_flag(data, AST_BRIDGE_CHANNEL_CB_OPTION_MEDIA)) {
+		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
+		bridge_channel_unsuspend(bridge_channel);
+	}
 }
 
 /*!
@@ -706,7 +797,9 @@ static void bridge_channel_do_callback(struct ast_bridge_channel *bridge_channel
  * \brief Marshal a custom callback function to be called on a bridge_channel
  */
 static int payload_helper_cb(ast_bridge_channel_post_action_data post_it,
-	struct ast_bridge_channel *bridge_channel, ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size)
+	struct ast_bridge_channel *bridge_channel,
+	enum ast_bridge_channel_custom_callback_option flags,
+	ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size)
 {
 	struct bridge_custom_callback *cb_data;
 	size_t len_data = sizeof(*cb_data) + (payload ? payload_size : 0);
@@ -721,6 +814,7 @@ static int payload_helper_cb(ast_bridge_channel_post_action_data post_it,
 	cb_data = alloca(len_data);
 	cb_data->callback = callback;
 	cb_data->payload_size = payload_size;
+	cb_data->flags = flags;
 	cb_data->payload_exists = payload && payload_size;
 	if (cb_data->payload_exists) {
 		memcpy(cb_data->payload, payload, payload_size);/* Safe */
@@ -729,16 +823,20 @@ static int payload_helper_cb(ast_bridge_channel_post_action_data post_it,
 	return post_it(bridge_channel, BRIDGE_CHANNEL_ACTION_CALLBACK, cb_data, len_data);
 }
 
-int ast_bridge_channel_write_callback(struct ast_bridge_channel *bridge_channel, ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size)
+int ast_bridge_channel_write_callback(struct ast_bridge_channel *bridge_channel,
+	enum ast_bridge_channel_custom_callback_option flags,
+	ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size)
 {
 	return payload_helper_cb(bridge_channel_write_action_data,
-		bridge_channel, callback, payload, payload_size);
+		bridge_channel, flags, callback, payload, payload_size);
 }
 
-int ast_bridge_channel_queue_callback(struct ast_bridge_channel *bridge_channel, ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size)
+int ast_bridge_channel_queue_callback(struct ast_bridge_channel *bridge_channel,
+	enum ast_bridge_channel_custom_callback_option flags,
+	ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size)
 {
 	return payload_helper_cb(bridge_channel_queue_action_data,
-		bridge_channel, callback, payload, payload_size);
+		bridge_channel, flags, callback, payload, payload_size);
 }
 
 struct bridge_park {
@@ -802,87 +900,6 @@ int ast_bridge_channel_write_park(struct ast_bridge_channel *bridge_channel, con
 		bridge_channel, parkee_uuid, parker_uuid, app_data);
 }
 
-/*!
- * \internal
- * \brief Suspend a channel from a bridge.
- *
- * \param bridge_channel Channel to suspend.
- *
- * \note This function assumes bridge_channel->bridge is locked.
- *
- * \return Nothing
- */
-void bridge_channel_internal_suspend_nolock(struct ast_bridge_channel *bridge_channel)
-{
-	bridge_channel->suspended = 1;
-	if (bridge_channel->in_bridge) {
-		--bridge_channel->bridge->num_active;
-	}
-
-	/* Get technology bridge threads off of the channel. */
-	if (bridge_channel->bridge->technology->suspend) {
-		bridge_channel->bridge->technology->suspend(bridge_channel->bridge, bridge_channel);
-	}
-}
-
-/*!
- * \internal
- * \brief Suspend a channel from a bridge.
- *
- * \param bridge_channel Channel to suspend.
- *
- * \return Nothing
- */
-static void bridge_channel_suspend(struct ast_bridge_channel *bridge_channel)
-{
-	ast_bridge_channel_lock_bridge(bridge_channel);
-	bridge_channel_internal_suspend_nolock(bridge_channel);
-	ast_bridge_unlock(bridge_channel->bridge);
-}
-
-/*!
- * \internal
- * \brief Unsuspend a channel from a bridge.
- *
- * \param bridge_channel Channel to unsuspend.
- *
- * \note This function assumes bridge_channel->bridge is locked.
- *
- * \return Nothing
- */
-void bridge_channel_internal_unsuspend_nolock(struct ast_bridge_channel *bridge_channel)
-{
-	bridge_channel->suspended = 0;
-	if (bridge_channel->in_bridge) {
-		++bridge_channel->bridge->num_active;
-	}
-
-	/* Wake technology bridge threads to take care of channel again. */
-	if (bridge_channel->bridge->technology->unsuspend) {
-		bridge_channel->bridge->technology->unsuspend(bridge_channel->bridge, bridge_channel);
-	}
-
-	/* Wake suspended channel. */
-	ast_bridge_channel_lock(bridge_channel);
-	ast_cond_signal(&bridge_channel->cond);
-	ast_bridge_channel_unlock(bridge_channel);
-}
-
-/*!
- * \internal
- * \brief Unsuspend a channel from a bridge.
- *
- * \param bridge_channel Channel to unsuspend.
- *
- * \return Nothing
- */
-static void bridge_channel_unsuspend(struct ast_bridge_channel *bridge_channel)
-{
-	ast_bridge_channel_lock_bridge(bridge_channel);
-	bridge_channel_internal_unsuspend_nolock(bridge_channel);
-	ast_bridge_unlock(bridge_channel->bridge);
-}
-
 /*!
  * \internal
  * \brief Handle bridge channel interval expiration.
@@ -897,7 +914,7 @@ static void bridge_channel_handle_interval(struct ast_bridge_channel *bridge_cha
 	struct ast_heap *interval_hooks;
 	struct ast_bridge_hook_timer *hook;
 	struct timeval start;
-	int hook_run = 0;
+	int chan_suspended = 0;
 
 	interval_hooks = bridge_channel->features->interval_hooks;
 	ast_heap_wrlock(interval_hooks);
@@ -914,8 +931,9 @@ static void bridge_channel_handle_interval(struct ast_bridge_channel *bridge_cha
 		ao2_ref(hook, +1);
 		ast_heap_unlock(interval_hooks);
 
-		if (!hook_run) {
+		if (!chan_suspended
-			hook_run = 1;
+			&& ast_test_flag(&hook->timer, AST_BRIDGE_HOOK_TIMER_OPTION_MEDIA)) {
+			chan_suspended = 1;
 			bridge_channel_suspend(bridge_channel);
 			ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
 		}
@@ -970,7 +988,7 @@ static void bridge_channel_handle_interval(struct ast_bridge_channel *bridge_cha
 	}
 	ast_heap_unlock(interval_hooks);
 
-	if (hook_run) {
+	if (chan_suspended) {
 		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
 		bridge_channel_unsuspend(bridge_channel);
 	}
@@ -1276,11 +1294,7 @@ static void bridge_channel_handle_action(struct ast_bridge_channel *bridge_chann
 		bridge_channel_unsuspend(bridge_channel);
 		break;
 	case BRIDGE_CHANNEL_ACTION_CALLBACK:
-		bridge_channel_suspend(bridge_channel);
-		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
 		bridge_channel_do_callback(bridge_channel, action->data.ptr);
-		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
-		bridge_channel_unsuspend(bridge_channel);
 		break;
 	case BRIDGE_CHANNEL_ACTION_PARK:
 		bridge_channel_suspend(bridge_channel);

main/features.c

@@ -558,13 +558,6 @@ static void dynamic_dtmf_hook_callback(struct ast_bridge_channel *bridge_channel
 		run_data->moh_offset ? &run_data->app_name[run_data->moh_offset] : NULL);
 }
 
-static int dynamic_dtmf_hook_run_callback(struct ast_bridge_channel *bridge_channel,
-	ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size)
-{
-	callback(bridge_channel, payload, payload_size);
-	return 0;
-}
-
 struct dynamic_dtmf_hook_data {
 	/*! Which side of bridge to run app (AST_FEATURE_FLAG_ONSELF/AST_FEATURE_FLAG_ONPEER) */
 	unsigned int flags;
@@ -592,7 +585,6 @@ struct dynamic_dtmf_hook_data {
 static int dynamic_dtmf_hook_trip(struct ast_bridge_channel *bridge_channel, void *hook_pvt)
 {
 	struct dynamic_dtmf_hook_data *pvt = hook_pvt;
-	int (*run_it)(struct ast_bridge_channel *bridge_channel, ast_bridge_custom_callback_fn callback, const void *payload, size_t payload_size);
 	struct dynamic_dtmf_hook_run *run_data;
 	const char *activated_name;
 	size_t len_name;
@@ -633,11 +625,12 @@ static int dynamic_dtmf_hook_trip(struct ast_bridge_channel *bridge_channel, voi
 	strcpy(&run_data->app_name[run_data->activated_offset], activated_name);/* Safe */
 
 	if (ast_test_flag(pvt, AST_FEATURE_FLAG_ONPEER)) {
-		run_it = ast_bridge_channel_write_callback;
+		ast_bridge_channel_write_callback(bridge_channel,
+			AST_BRIDGE_CHANNEL_CB_OPTION_MEDIA,
+			dynamic_dtmf_hook_callback, run_data, len_data);
 	} else {
-		run_it = dynamic_dtmf_hook_run_callback;
+		dynamic_dtmf_hook_callback(bridge_channel, run_data, len_data);
 	}
-	run_it(bridge_channel, dynamic_dtmf_hook_callback, run_data, len_data);
 	return 0;
 }
 

res/parking/parking_bridge_features.c

@@ -602,9 +602,9 @@ void parking_set_duration(struct ast_bridge_features *features, struct parked_us
 	/* The interval hook is going to need a reference to the parked_user */
 	ao2_ref(user, +1);
 
-	if (ast_bridge_interval_hook(features, time_limit,
+	if (ast_bridge_interval_hook(features, 0, time_limit,
 		parking_duration_callback, user, __ao2_cleanup, AST_BRIDGE_HOOK_REMOVE_ON_PULL)) {
-		ast_log(LOG_ERROR, "Failed to apply duration limits to the parking call.\n");
+		ast_log(LOG_ERROR, "Failed to apply duration limit to the parked call.\n");
 		ao2_ref(user, -1);
 	}
 }