[Asterisk-code-review] ConfBridge: Make some announcements asynchronous. (asterisk[master])
Mark Michelson
asteriskteam at digium.com
Thu Aug 11 13:32:16 CDT 2016
Mark Michelson has uploaded a new change for review.
https://gerrit.asterisk.org/3508
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, 140 insertions(+), 34 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/08/3508/1
diff --git a/CHANGES b/CHANGES
index ee74625..c27da72 100644
--- a/CHANGES
+++ b/CHANGES
@@ -42,6 +42,11 @@
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 to Asterisk 14 --------------------
diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index 3c780e8..fd8e59a 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,21 +1673,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;
@@ -1730,6 +1736,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);
}
/*!
@@ -2041,10 +2143,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) {
@@ -2095,19 +2194,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 */
@@ -2180,13 +2275,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 5079ae1..630ac73 100644
--- a/apps/confbridge/include/confbridge.h
+++ b/apps/confbridge/include/confbridge.h
@@ -389,6 +389,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/3508
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie486bb3de1646d50894489030326a423e594ab0a
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-code-review
mailing list