[Asterisk-code-review] ConfBridge: Make some announcements asynchronous. (asterisk[13])
Mark Michelson
asteriskteam at digium.com
Thu Aug 18 11:03:49 CDT 2016
Mark Michelson has uploaded a new change for review.
https://gerrit.asterisk.org/3504
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, 185 insertions(+), 40 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/04/3504/4
diff --git a/CHANGES b/CHANGES
index 9535479..6e3573a 100644
--- a/CHANGES
+++ b/CHANGES
@@ -37,6 +37,12 @@
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 ----------
------------------------------------------------------------------------------
diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index ec41365..5dce591 100644
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -1627,6 +1627,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;
@@ -1653,23 +1673,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);
@@ -1733,6 +1738,122 @@
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];
+};
+
+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 ? ast_channel_ref(initiator) : NULL;
+
+ return aptd;
+}
+
+static void async_playback_task_data_destroy(struct async_playback_task_data *aptd)
+{
+ ast_channel_cleanup(aptd->initiator);
+ 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;
+
+ /* Wait for the initiator to get back in the bridge or be hung up*/
+ if (aptd->initiator) {
+ int go_on = 0;
+
+ while (!go_on) {
+ ast_channel_lock(aptd->initiator);
+ go_on = ast_check_hangup(aptd->initiator) || ast_channel_is_bridged(aptd->initiator);
+ ast_channel_unlock(aptd->initiator);
+ usleep(1);
+ }
+ }
+
+ 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) && !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);
}
/*!
@@ -2044,10 +2165,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) {
@@ -2098,19 +2223,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 */
@@ -2183,13 +2304,17 @@
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))) {
+ 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_helper(conference, sound_to_play, 0);
+ ast_autoservice_stop(user->chan);
+ } else {
+ /* Playing the sound asynchronously lets the sound be heard by everyone at once */
+ async_play_sound_helper(conference, sound_to_play, 0, user->chan);
+ }
return 0;
}
diff --git a/apps/confbridge/conf_state_multi_marked.c b/apps/confbridge/conf_state_multi_marked.c
index fabe99b..200e70b 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),
+ user->chan);
}
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 451d810..e782b7b 100644
--- a/apps/confbridge/include/confbridge.h
+++ b/apps/confbridge/include/confbridge.h
@@ -386,6 +386,23 @@
*/
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 Callback to be called when the conference has become empty
* \param conference The conference bridge
*/
--
To view, visit https://gerrit.asterisk.org/3504
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie486bb3de1646d50894489030326a423e594ab0a
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list