[Asterisk-code-review] ConfBridge: Make some announcements asynchronous. (asterisk[certified/13.8])

Mark Michelson asteriskteam at digium.com
Thu Aug 11 13:25:14 CDT 2016


Mark Michelson has uploaded a new change for review.

  https://gerrit.asterisk.org/3506

Change subject: ConfBridge: Make some announcements asynchronous.
......................................................................

ConfBridge: Make some announcements asynchronous.

Confbridge announcements tend to block a channel while they are being
played. In some circumstances, this is warranted since you want that
particular channel not to hear the announcement (Example: "John Doe has
entered the conference"). For others it makes less sense.

This change first introduces methods for playing sounds asynchronously
into the conference. This is very similar to how synchronous sounds are
played, except the channel initiating the playback does not wait for the
sound to complete before moving on.

Asynchronous announcements are used for two circumstances:
* Sounds played for a user after they have left the bridge
* Sounds that play first to a single user and then the rest of the
  conference.

ASTERISK-26289 #close
Reported by Mark Michelson

Change-Id: Ie486bb3de1646d50894489030326a423e594ab0a
---
M CHANGES
M apps/app_confbridge.c
M apps/confbridge/conf_state_multi_marked.c
M apps/confbridge/include/confbridge.h
4 files changed, 171 insertions(+), 34 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/06/3506/1

diff --git a/CHANGES b/CHANGES
index 82cd6da..37a8f11 100644
--- a/CHANGES
+++ b/CHANGES
@@ -12,6 +12,42 @@
 --- Functionality changes from Asterisk 13.8-cert1 to Asterisk 13.8-cert2 ----
 ------------------------------------------------------------------------------
 
+app_voicemail
+------------------
+ * Added "tps_queue_high" and "tps_queue_low" options.
+   The options can modify the taskprocessor alert levels for this module.
+   Additional information can be found in the sample configuration file at
+   config/samples/voicemail.conf.sample.
+
+res_pjsip_mwi
+------------------
+ * Added "mwi_tps_queue_high" and "mwi_tps_queue_low" global configuration
+   options to tune taskprocessor alert levels.
+
+ * Added "mwi_disable_initial_unsolicited" global configuration option
+   to disable sending unsolicited MWI to all endpoints on startup.
+   Additional information can be found in the sample configuration file at
+   config/samples/pjsip.conf.sample.
+
+chan_pjsip
+------------------
+ * A new dialplan function, PJSIP_SEND_SESSION_REFRESH, has been added. When
+   invoked, a re-INVITE or UPDATE request will be sent immediately to the
+   endpoint underlying the channel. When used in combination with the existing
+   dialplan function PJSIP_MEDIA_OFFER, this allows the formats on a PJSIP
+   channel to be re-negotiated and updated after session set up.
+
+app_confbridge
+------------------
+  * Some sounds played into the bridge are played asynchronously. This, for
+    instance, allows a channel to immediately exit the ConfBridge without having
+	to wait for a leave announcement to play.
+
+------------------------------------------------------------------------------
+--- Functionality changes from Asterisk 13.10.0 to Asterisk 13.11.0 ----------
+>>>>>>> 45b3546... ConfBridge: Make some announcements asynchronous.
+------------------------------------------------------------------------------
+
 chan_dahdi
 ------------------
  * Added "faxdetect_timeout" option.
diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index f603264..012e95c 100644
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -1609,6 +1609,26 @@
 	user->conference = NULL;
 }
 
+static void playback_common(struct confbridge_conference *conference, const char *filename, int say_number)
+{
+	/* Don't try to play if the playback channel has been hung up */
+	if (!conference->playback_chan) {
+		return;
+	}
+
+	ast_autoservice_stop(conference->playback_chan);
+
+	/* The channel is all under our control, in goes the prompt */
+	if (!ast_strlen_zero(filename)) {
+		ast_stream_and_wait(conference->playback_chan, filename, "");
+	} else if (say_number >= 0) {
+		ast_say_number(conference->playback_chan, say_number, "",
+			ast_channel_language(conference->playback_chan), NULL);
+	}
+
+	ast_autoservice_start(conference->playback_chan);
+}
+
 struct playback_task_data {
 	struct confbridge_conference *conference;
 	const char *filename;
@@ -1635,21 +1655,7 @@
 {
 	struct playback_task_data *ptd = data;
 
-	/* Don't try to play if the playback channel has been hung up */
-	if (!ptd->conference->playback_chan) {
-		return 0;
-	}
-
-	ast_autoservice_stop(ptd->conference->playback_chan);
-
-	/* The channel is all under our control, in goes the prompt */
-	if (!ast_strlen_zero(ptd->filename)) {
-		ast_stream_and_wait(ptd->conference->playback_chan, ptd->filename, "");
-	} else if (ptd->say_number >= 0) {
-		ast_say_number(ptd->conference->playback_chan, ptd->say_number, "",
-			ast_channel_language(ptd->conference->playback_chan), NULL);
-	}
-	ast_autoservice_start(ptd->conference->playback_chan);
+	playback_common(ptd->conference, ptd->filename, ptd->say_number);
 
 	ast_mutex_lock(&ptd->lock);
 	ptd->playback_finished = 1;
@@ -1712,6 +1718,102 @@
 int play_sound_file(struct confbridge_conference *conference, const char *filename)
 {
 	return play_sound_helper(conference, filename, -1);
+}
+
+struct async_playback_task_data {
+	struct confbridge_conference *conference;
+	int say_number;
+	char filename[0];
+};
+
+static struct async_playback_task_data *async_playback_task_data_alloc(
+	struct confbridge_conference *conference, const char *filename, int say_number)
+{
+	struct async_playback_task_data *aptd;
+
+	aptd = ast_malloc(sizeof(*aptd) + strlen(filename) + 1);
+	if (!aptd) {
+		return NULL;
+	}
+
+	/* Safe */
+	strcpy(aptd->filename, filename);
+	aptd->say_number = say_number;
+
+	/* You may think that we need to bump the conference refcount since we are pushing
+	 * this task to the taskprocessor.
+	 *
+	 * In this case, that actually causes a problem. The destructor for the conference
+	 * pushes a hangup task into the taskprocessor and waits for it to complete before
+	 * continuing. If the destructor gets called from a taskprocessor task, we're
+	 * deadlocked.
+	 *
+	 * So is there a risk of the conference being freed out from under us? No. Since
+	 * the destructor pushes a task into the taskprocessor and waits for it to complete,
+	 * the destructor cannot free the conference out from under us. No further tasks
+	 * can be queued onto the taskprocessor after the hangup since no channels are referencing
+	 * the conference at that point any more.
+	 */
+	aptd->conference = conference;
+
+	return aptd;
+}
+
+static void async_playback_task_data_destroy(struct async_playback_task_data *aptd)
+{
+	ast_free(aptd);
+}
+
+/*!
+ * \brief Play an announcement into a confbridge asynchronously
+ *
+ * This runs in the playback queue taskprocessor. This ensures that
+ * all playbacks are handled in sequence and do not play over top one
+ * another.
+ *
+ * \param data An async_playback_task_data
+ * \return 0
+ */
+static int async_playback_task(void *data)
+{
+	struct async_playback_task_data *aptd = data;
+
+	playback_common(aptd->conference, aptd->filename, aptd->say_number);
+
+	async_playback_task_data_destroy(aptd);
+	return 0;
+}
+
+static int async_play_sound_helper(struct confbridge_conference *conference, const char *filename, int say_number)
+{
+	struct async_playback_task_data *aptd;
+
+	/* Do not waste resources trying to play files that do not exist */
+	if (!ast_strlen_zero(filename) && !sound_file_exists(filename)) {
+		return 0;
+	}
+
+	aptd = async_playback_task_data_alloc(conference, filename, say_number);
+	if (!aptd) {
+		return -1;
+	}
+
+	if (ast_taskprocessor_push(conference->playback_queue, async_playback_task, aptd)) {
+		if (!ast_strlen_zero(filename)) {
+			ast_log(LOG_WARNING, "Unable to play file '%s' to conference\n", filename);
+		} else {
+			ast_log(LOG_WARNING, "Unable to say number '%d' to conference\n", say_number);
+		}
+		async_playback_task_data_destroy(aptd);
+		return -1;
+	}
+
+	return 0;
+}
+
+int async_play_sound_file(struct confbridge_conference *conference, const char *filename)
+{
+	return async_play_sound_helper(conference, filename, -1);
 }
 
 /*!
@@ -2023,10 +2125,7 @@
 	if (!quiet) {
 		const char *join_sound = conf_get_sound(CONF_SOUND_JOIN, conference->b_profile.sounds);
 
-		ast_stream_and_wait(chan, join_sound, "");
-		ast_autoservice_start(chan);
-		play_sound_file(conference, join_sound);
-		ast_autoservice_stop(chan);
+		async_play_sound_file(conference, join_sound);
 	}
 
 	if (user.u_profile.timeout) {
@@ -2077,19 +2176,15 @@
 
 	/* if this user has a intro, play it when leaving */
 	if (!quiet && !ast_strlen_zero(user.name_rec_location)) {
-		ast_autoservice_start(chan);
-		play_sound_file(conference, user.name_rec_location);
-		play_sound_file(conference,
+		async_play_sound_file(conference, user.name_rec_location);
+		async_play_sound_file(conference,
 			conf_get_sound(CONF_SOUND_HAS_LEFT, conference->b_profile.sounds));
-		ast_autoservice_stop(chan);
 	}
 
 	/* play the leave sound */
 	if (!quiet) {
 		const char *leave_sound = conf_get_sound(CONF_SOUND_LEAVE, conference->b_profile.sounds);
-		ast_autoservice_start(chan);
-		play_sound_file(conference, leave_sound);
-		ast_autoservice_stop(chan);
+		async_play_sound_file(conference, leave_sound);
 	}
 
 	/* If the user was kicked from the conference play back the audio prompt for it */
@@ -2162,13 +2257,8 @@
 		mute ? CONF_SOUND_PARTICIPANTS_MUTED : CONF_SOUND_PARTICIPANTS_UNMUTED,
 		conference->b_profile.sounds);
 
-	/* The host needs to hear it seperately, as they don't get the audio from play_sound_helper */
-	ast_stream_and_wait(user->chan, sound_to_play, "");
-
-	/* Announce to the group that all participants are muted */
-	ast_autoservice_start(user->chan);
-	play_sound_helper(conference, sound_to_play, 0);
-	ast_autoservice_stop(user->chan);
+	/* Playing the sound asynchronously lets the sound be heard by everyone at once */
+	async_play_sound_helper(conference, sound_to_play, 0);
 
 	return 0;
 }
diff --git a/apps/confbridge/conf_state_multi_marked.c b/apps/confbridge/conf_state_multi_marked.c
index fabe99b..80a3dbd 100644
--- a/apps/confbridge/conf_state_multi_marked.c
+++ b/apps/confbridge/conf_state_multi_marked.c
@@ -162,7 +162,7 @@
 		if (!ast_test_flag(&user->u_profile, USER_OPT_QUIET)) {
 			ao2_unlock(user->conference);
 			ast_autoservice_start(user->chan);
-			play_sound_file(user->conference,
+			async_play_sound_file(user->conference,
 				conf_get_sound(CONF_SOUND_LEADER_HAS_LEFT, user->conference->b_profile.sounds));
 			ast_autoservice_stop(user->chan);
 			ao2_lock(user->conference);
diff --git a/apps/confbridge/include/confbridge.h b/apps/confbridge/include/confbridge.h
index 79e496b..1af1c85 100644
--- a/apps/confbridge/include/confbridge.h
+++ b/apps/confbridge/include/confbridge.h
@@ -385,6 +385,17 @@
  */
 int play_sound_file(struct confbridge_conference *conference, const char *filename);
 
+/*!
+ * \brief Play sound file into conference bridge asynchronously
+ *
+ * \param conference The conference bridge to play sound file into
+ * \param filename Sound file to play
+ *
+ * \retval 0 success
+ * \retval -1 failure
+ */
+int async_play_sound_file(struct confbridge_conference *conference, const char *filename);
+
 /*! \brief Callback to be called when the conference has become empty
  * \param conference The conference bridge
  */

-- 
To view, visit https://gerrit.asterisk.org/3506
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie486bb3de1646d50894489030326a423e594ab0a
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.8
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-code-review mailing list