[Asterisk-code-review] ConfBridge: Make some announcements asynchronous. (asterisk[master])
Anonymous Coward
asteriskteam at digium.com
Wed Sep 7 20:37:09 CDT 2016
Anonymous Coward #1000019 has submitted this change and it was merged.
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 (if the channel and conference use the same language)
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, 375 insertions(+), 41 deletions(-)
Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, approved
diff --git a/CHANGES b/CHANGES
index 39e70ce..2851b26 100644
--- a/CHANGES
+++ b/CHANGES
@@ -72,6 +72,12 @@
configure these options then you already had to do a reload after making
changes.
+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 to Asterisk 14 --------------------
------------------------------------------------------------------------------
diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index d5cbc41..a7fd92a 100644
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -1630,6 +1630,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;
@@ -1656,23 +1676,8 @@
{
struct playback_task_data *ptd = data;
- /* Don't try to play if the playback channel has been hung up */
- if (!ptd->conference->playback_chan) {
- goto end;
- }
+ playback_common(ptd->conference, ptd->filename, ptd->say_number);
- 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);
-
-end:
ast_mutex_lock(&ptd->lock);
ptd->playback_finished = 1;
ast_cond_signal(&ptd->cond);
@@ -1704,7 +1709,11 @@
struct playback_task_data ptd;
/* Do not waste resources trying to play files that do not exist */
- if (!ast_strlen_zero(filename) && !sound_file_exists(filename)) {
+ if (ast_strlen_zero(filename)) {
+ if (say_number < 0) {
+ return 0;
+ }
+ } else if (!sound_file_exists(filename)) {
return 0;
}
@@ -1736,6 +1745,274 @@
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;
+ struct ast_channel *initiator;
+ char filename[0];
+};
+
+struct async_datastore_data {
+ ast_mutex_t lock;
+ ast_cond_t cond;
+ int wait;
+};
+
+static void async_datastore_data_destroy(void *data)
+{
+ struct async_datastore_data *add = data;
+
+ ast_mutex_destroy(&add->lock);
+ ast_cond_destroy(&add->cond);
+
+ ast_free(add);
+}
+
+/*!
+ * \brief Datastore used for timing of async announcement playback
+ *
+ * Announcements that are played to the entire conference can be played
+ * asynchronously (i.e. The channel that queues the playback does not wait
+ * for the playback to complete before continuing)
+ *
+ * The thing about async announcements is that the channel that queues the
+ * announcement is either not in the bridge or is in some other way "occupied"
+ * at the time the announcement is queued. Because of that, the initiator of
+ * the announcement may enter after the announcement has already started,
+ * resulting in the sound being "clipped".
+ *
+ * This datastore makes it so that the channel that queues the async announcement
+ * can say "I'm ready now". This way the announcement does not start until the
+ * initiator of the announcement is ready to hear the sound.
+ */
+static struct ast_datastore_info async_datastore_info = {
+ .type = "Confbridge async playback",
+ .destroy = async_datastore_data_destroy,
+};
+
+static struct async_datastore_data *async_datastore_data_alloc(void)
+{
+ struct async_datastore_data *add;
+
+ add = ast_malloc(sizeof(*add));
+ if (!add) {
+ return NULL;
+ }
+
+ ast_mutex_init(&add->lock);
+ ast_cond_init(&add->cond, NULL);
+ add->wait = 1;
+
+ return add;
+}
+
+/*!
+ * \brief Prepare the async playback datastore
+ *
+ * This is done prior to queuing an async announcement. If the
+ * datastore has not yet been created, it is allocated and initialized.
+ * If it already exists, we set it to be in "waiting" mode.
+ *
+ * \param initiator The channel that is queuing the async playback
+ * \retval 0 Success
+ * \retval -1 Failure :(
+ */
+static int setup_async_playback_datastore(struct ast_channel *initiator)
+{
+ struct ast_datastore *async_datastore;
+
+ async_datastore = ast_channel_datastore_find(initiator, &async_datastore_info, NULL);
+ if (async_datastore) {
+ struct async_datastore_data *add;
+
+ add = async_datastore->data;
+ add->wait = 1;
+
+ return 0;
+ }
+
+ async_datastore = ast_datastore_alloc(&async_datastore_info, NULL);
+ if (!async_datastore) {
+ return -1;
+ }
+
+ async_datastore->data = async_datastore_data_alloc();
+ if (!async_datastore->data) {
+ ast_datastore_free(async_datastore);
+ return -1;
+ }
+
+ ast_channel_datastore_add(initiator, async_datastore);
+ return 0;
+}
+
+static struct async_playback_task_data *async_playback_task_data_alloc(
+ struct confbridge_conference *conference, const char *filename, int say_number,
+ struct ast_channel *initiator)
+{
+ 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;
+
+ aptd->initiator = initiator;
+ if (initiator) {
+ ast_channel_ref(initiator);
+ ast_channel_lock(aptd->initiator);
+ /* We don't really care if this fails. If the datastore fails to get set up
+ * we'll still play the announcement. It's possible that the sound will be
+ * clipped for the initiator, but that's not the end of the world.
+ */
+ setup_async_playback_datastore(aptd->initiator);
+ ast_channel_unlock(aptd->initiator);
+ }
+
+ return aptd;
+}
+
+static void async_playback_task_data_destroy(struct async_playback_task_data *aptd)
+{
+ ast_channel_cleanup(aptd->initiator);
+ ast_free(aptd);
+}
+
+/*!
+ * \brief Wait for the initiator of an async playback to be ready
+ *
+ * See the description on the async_datastore_info structure for more
+ * information about what this is about.
+ *
+ * \param initiator The channel that queued the async announcement
+ */
+static void wait_for_initiator(struct ast_channel *initiator)
+{
+ struct ast_datastore *async_datastore;
+ struct async_datastore_data *add;
+
+ ast_channel_lock(initiator);
+ async_datastore = ast_channel_datastore_find(initiator, &async_datastore_info, NULL);
+ ast_channel_unlock(initiator);
+
+ if (!async_datastore) {
+ return;
+ }
+
+ add = async_datastore->data;
+
+ ast_mutex_lock(&add->lock);
+ while (add->wait) {
+ ast_cond_wait(&add->cond, &add->lock);
+ }
+ ast_mutex_unlock(&add->lock);
+}
+
+/*!
+ * \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;
+
+ /* Wait for the initiator to get back in the bridge or be hung up */
+ if (aptd->initiator) {
+ wait_for_initiator(aptd->initiator);
+ }
+
+ 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 ast_channel *initiator)
+{
+ struct async_playback_task_data *aptd;
+
+ /* Do not waste resources trying to play files that do not exist */
+ if (ast_strlen_zero(filename)) {
+ if (say_number < 0) {
+ return 0;
+ }
+ } else if (!sound_file_exists(filename)) {
+ return 0;
+ }
+
+ aptd = async_playback_task_data_alloc(conference, filename, say_number, initiator);
+ 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 '%s'\n",
+ filename, conference->name);
+ } else {
+ ast_log(LOG_WARNING, "Unable to say number '%d' to conference '%s'\n",
+ say_number, conference->name);
+ }
+ async_playback_task_data_destroy(aptd);
+ return -1;
+ }
+
+ return 0;
+}
+
+int async_play_sound_file(struct confbridge_conference *conference,
+ const char *filename, struct ast_channel *initiator)
+{
+ return async_play_sound_helper(conference, filename, -1, initiator);
+}
+
+void async_play_sound_ready(struct ast_channel *chan)
+{
+ struct ast_datastore *async_datastore;
+ struct async_datastore_data *add;
+
+ ast_channel_lock(chan);
+ async_datastore = ast_channel_datastore_find(chan, &async_datastore_info, NULL);
+ ast_channel_unlock(chan);
+ if (!async_datastore) {
+ return;
+ }
+
+ add = async_datastore->data;
+
+ ast_mutex_lock(&add->lock);
+ add->wait = 0;
+ ast_cond_signal(&add->cond);
+ ast_mutex_unlock(&add->lock);
}
/*!
@@ -1866,6 +2143,12 @@
user->name_rec_location[0] = '\0';
return -1;
}
+ return 0;
+}
+
+static int join_callback(struct ast_bridge_channel *bridge_channel, void *ignore)
+{
+ async_play_sound_ready(bridge_channel->chan);
return 0;
}
@@ -2047,10 +2330,14 @@
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);
+ if (strcmp(conference->b_profile.language, ast_channel_language(chan))) {
+ ast_stream_and_wait(chan, join_sound, "");
+ ast_autoservice_start(chan);
+ play_sound_file(conference, join_sound);
+ ast_autoservice_stop(chan);
+ } else {
+ async_play_sound_file(conference, join_sound, chan);
+ }
}
if (user.u_profile.timeout) {
@@ -2070,12 +2357,22 @@
/* Join our conference bridge for real */
send_join_event(&user, conference);
+
+ if (ast_bridge_join_hook(&user.features, join_callback, NULL, NULL, 0)) {
+ async_play_sound_ready(user.chan);
+ }
+
ast_bridge_join(conference->bridge,
chan,
NULL,
&user.features,
&user.tech_args,
0);
+
+ /* This is a catch-all in case joining the bridge failed or for some reason
+ * an async announcement got queued up and hasn't been told to play yet
+ */
+ async_play_sound_ready(chan);
if (!user.kicked && ast_check_hangup(chan)) {
pbx_builtin_setvar_helper(chan, "CONFBRIDGE_RESULT", "HANGUP");
@@ -2101,19 +2398,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,
- conf_get_sound(CONF_SOUND_HAS_LEFT, conference->b_profile.sounds));
- ast_autoservice_stop(chan);
+ async_play_sound_file(conference, user.name_rec_location, NULL);
+ async_play_sound_file(conference,
+ conf_get_sound(CONF_SOUND_HAS_LEFT, conference->b_profile.sounds), NULL);
}
/* 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, NULL);
}
/* If the user was kicked from the conference play back the audio prompt for it */
@@ -2186,13 +2479,18 @@
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, "");
+ if (strcmp(conference->b_profile.language, ast_channel_language(user->chan))) {
+ /* 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);
+ /* Announce to the group that all participants are muted */
+ ast_autoservice_start(user->chan);
+ play_sound_file(conference, sound_to_play);
+ ast_autoservice_stop(user->chan);
+ } else {
+ /* Playing the sound asynchronously lets the sound be heard by everyone at once */
+ async_play_sound_file(conference, sound_to_play, user->chan);
+ }
return 0;
}
@@ -2477,6 +2775,8 @@
/* See if music on hold needs to be started back up again */
conf_moh_unsuspend(user);
+ async_play_sound_ready(bridge_channel->chan);
+
return 0;
}
diff --git a/apps/confbridge/conf_state_multi_marked.c b/apps/confbridge/conf_state_multi_marked.c
index fabe99b..17ca65c 100644
--- a/apps/confbridge/conf_state_multi_marked.c
+++ b/apps/confbridge/conf_state_multi_marked.c
@@ -160,12 +160,9 @@
if (need_prompt) {
/* Play back the audio prompt saying the leader has left the conference */
if (!ast_test_flag(&user->u_profile, USER_OPT_QUIET)) {
- ao2_unlock(user->conference);
- ast_autoservice_start(user->chan);
- 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);
+ async_play_sound_file(user->conference,
+ conf_get_sound(CONF_SOUND_LEADER_HAS_LEFT, user->conference->b_profile.sounds),
+ NULL);
}
AST_LIST_TRAVERSE(&user->conference->waiting_list, user_iter, list) {
diff --git a/apps/confbridge/include/confbridge.h b/apps/confbridge/include/confbridge.h
index 5d71a63..93cac3a 100644
--- a/apps/confbridge/include/confbridge.h
+++ b/apps/confbridge/include/confbridge.h
@@ -389,6 +389,37 @@
*/
int play_sound_file(struct confbridge_conference *conference, const char *filename);
+/*!
+ * \brief Play sound file into conference bridge asynchronously
+ *
+ * If the initiator parameter is non-NULL, then the playback will wait for
+ * that initiator channel to get back in the bridge before playing the sound
+ * file. This way, the initiator has no danger of hearing a "clipped" file.
+ *
+ * \param conference The conference bridge to play sound file into
+ * \param filename Sound file to play
+ * \param initiator Channel that initiated playback.
+ *
+ * \retval 0 success
+ * \retval -1 failure
+ */
+int async_play_sound_file(struct confbridge_conference *conference, const char *filename,
+ struct ast_channel *initiator);
+
+/*!
+ * \brief Indicate the initiator of an async sound file is ready for it to play.
+ *
+ * When playing an async sound file, the initiator is typically either out of the bridge
+ * or not in a position to hear the queued announcement. This function lets the announcement
+ * thread know that the initiator is now ready for the sound to play.
+ *
+ * If an async announcement was queued and no initiator channel was provided, then this is
+ * a no-op
+ *
+ * \param chan The channel that initiated the async announcement
+ */
+void async_play_sound_ready(struct ast_channel *chan);
+
/*! \brief Callback to be called when the conference has become empty
* \param conference The conference bridge
*/
--
To view, visit https://gerrit.asterisk.org/3508
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie486bb3de1646d50894489030326a423e594ab0a
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list