[svn-commits] rmudgett: branch 11 r377993 - in /branches/11: ./ apps/ apps/confbridge/ apps...
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Thu Dec 13 15:04:20 CST 2012
Author: rmudgett
Date: Thu Dec 13 15:04:16 2012
New Revision: 377993
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=377993
Log:
confbridge: Fix MOH on simultaneous user entry to a new conference.
When two users entered a new conference simultaneously, one of the callers
hears MOH. This happened if two unmarked users entered simultaneously and
also if a waitmarked and a marked user entered simultaneously.
* Created a confbridge internal MOH API to eliminate the inlined MOH
handling code. Note that the conference mixing bridge needs to be locked
when actually starting/stopping MOH because there is a small window
between the conference join unsuspend MOH and actually joining the mixing
bridge.
* Created the concept of suspended MOH so it can be interrupted while
conference join announcements to the user and DTMF features can operate.
* Suspend any MOH until the user is about to actually join the mixing
bridge of the conference. This way any pre-join file playback does not
need to worry about MOH.
* Made post-join actions only play deferred entry announcement files.
Changing the user/conference state during that time is not protected or
controlled by the state machine.
(closes issue ASTERISK-20606)
Reported by: Eugenia Belova
Tested by: rmudgett
Review: https://reviewboard.asterisk.org/r/2232/
........
Merged revisions 377992 from http://svn.asterisk.org/svn/asterisk/branches/10
Modified:
branches/11/ (props changed)
branches/11/apps/app_confbridge.c
branches/11/apps/confbridge/conf_state.c
branches/11/apps/confbridge/conf_state_multi_marked.c
branches/11/apps/confbridge/include/confbridge.h
branches/11/include/asterisk/bridging.h
Propchange: branches/11/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.
Modified: branches/11/apps/app_confbridge.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/apps/app_confbridge.c?view=diff&rev=377993&r1=377992&r2=377993
==============================================================================
--- branches/11/apps/app_confbridge.c (original)
+++ branches/11/apps/app_confbridge.c Thu Dec 13 15:04:16 2012
@@ -900,6 +900,94 @@
return 0;
}
+void conf_moh_stop(struct conference_bridge_user *user)
+{
+ user->playing_moh = 0;
+ if (!user->suspended_moh) {
+ int in_bridge;
+
+ /*
+ * Locking the ast_bridge here is the only way to hold off the
+ * call to ast_bridge_join() in confbridge_exec() from
+ * interfering with the bridge and MOH operations here.
+ */
+ ast_bridge_lock(user->conference_bridge->bridge);
+
+ /*
+ * Temporarily suspend the user from the bridge so we have
+ * control to stop MOH if needed.
+ */
+ in_bridge = !ast_bridge_suspend(user->conference_bridge->bridge, user->chan);
+ ast_moh_stop(user->chan);
+ if (in_bridge) {
+ ast_bridge_unsuspend(user->conference_bridge->bridge, user->chan);
+ }
+
+ ast_bridge_unlock(user->conference_bridge->bridge);
+ }
+}
+
+void conf_moh_start(struct conference_bridge_user *user)
+{
+ user->playing_moh = 1;
+ if (!user->suspended_moh) {
+ int in_bridge;
+
+ /*
+ * Locking the ast_bridge here is the only way to hold off the
+ * call to ast_bridge_join() in confbridge_exec() from
+ * interfering with the bridge and MOH operations here.
+ */
+ ast_bridge_lock(user->conference_bridge->bridge);
+
+ /*
+ * Temporarily suspend the user from the bridge so we have
+ * control to start MOH if needed.
+ */
+ in_bridge = !ast_bridge_suspend(user->conference_bridge->bridge, user->chan);
+ ast_moh_start(user->chan, user->u_profile.moh_class, NULL);
+ if (in_bridge) {
+ ast_bridge_unsuspend(user->conference_bridge->bridge, user->chan);
+ }
+
+ ast_bridge_unlock(user->conference_bridge->bridge);
+ }
+}
+
+/*!
+ * \internal
+ * \brief Unsuspend MOH for the conference user.
+ *
+ * \param user Conference user to unsuspend MOH on.
+ *
+ * \return Nothing
+ */
+static void conf_moh_unsuspend(struct conference_bridge_user *user)
+{
+ ao2_lock(user->conference_bridge);
+ if (--user->suspended_moh == 0 && user->playing_moh) {
+ ast_moh_start(user->chan, user->u_profile.moh_class, NULL);
+ }
+ ao2_unlock(user->conference_bridge);
+}
+
+/*!
+ * \internal
+ * \brief Suspend MOH for the conference user.
+ *
+ * \param user Conference user to suspend MOH on.
+ *
+ * \return Nothing
+ */
+static void conf_moh_suspend(struct conference_bridge_user *user)
+{
+ ao2_lock(user->conference_bridge);
+ if (user->suspended_moh++ == 0 && user->playing_moh) {
+ ast_moh_stop(user->chan);
+ }
+ ao2_unlock(user->conference_bridge);
+}
+
int conf_handle_first_marked_common(struct conference_bridge_user *cbu)
{
if (!ast_test_flag(&cbu->u_profile, USER_OPT_QUIET) && play_prompt_to_user(cbu, conf_get_sound(CONF_SOUND_PLACE_IN_CONF, cbu->b_profile.sounds))) {
@@ -910,18 +998,11 @@
int conf_handle_inactive_waitmarked(struct conference_bridge_user *cbu)
{
- /* Be sure we are muted so we can't talk to anybody else waiting */
- cbu->features.mute = 1;
/* If we have not been quieted play back that they are waiting for the leader */
if (!ast_test_flag(&cbu->u_profile, USER_OPT_QUIET) && play_prompt_to_user(cbu,
conf_get_sound(CONF_SOUND_WAIT_FOR_LEADER, cbu->b_profile.sounds))) {
/* user hungup while the sound was playing */
return -1;
- }
- /* Start music on hold if needed */
- if (ast_test_flag(&cbu->u_profile, USER_OPT_MUSICONHOLD)) {
- ast_moh_start(cbu->chan, cbu->u_profile.moh_class, NULL);
- cbu->playing_moh = 1;
}
return 0;
}
@@ -961,11 +1042,8 @@
/* If we are the second participant we may need to stop music on hold on the first */
struct conference_bridge_user *first_participant = AST_LIST_FIRST(&conference_bridge->active_list);
- /* Temporarily suspend the above participant from the bridge so we have control to stop MOH if needed */
- if (ast_test_flag(&first_participant->u_profile, USER_OPT_MUSICONHOLD) && !ast_bridge_suspend(conference_bridge->bridge, first_participant->chan)) {
- first_participant->playing_moh = 0;
- ast_moh_stop(first_participant->chan);
- ast_bridge_unsuspend(conference_bridge->bridge, first_participant->chan);
+ if (ast_test_flag(&first_participant->u_profile, USER_OPT_MUSICONHOLD)) {
+ conf_moh_stop(first_participant);
}
if (!ast_test_flag(&first_participant->u_profile, USER_OPT_STARTMUTED)) {
first_participant->features.mute = 0;
@@ -1089,6 +1167,13 @@
conference_bridge_user->conference_bridge = conference_bridge;
ao2_lock(conference_bridge);
+
+ /*
+ * Suspend any MOH until the user actually joins the bridge of
+ * the conference. This way any pre-join file playback does not
+ * need to worry about MOH.
+ */
+ conference_bridge_user->suspended_moh = 1;
if (handle_conf_user_join(conference_bridge_user)) {
/* Invalid event, nothing was done, so we don't want to process a leave. */
@@ -1521,20 +1606,17 @@
/* Play the Join sound to both the conference and the user entering. */
if (!quiet) {
const char *join_sound = conf_get_sound(CONF_SOUND_JOIN, conference_bridge_user.b_profile.sounds);
- if (conference_bridge_user.playing_moh) {
- ast_moh_stop(chan);
- }
+
ast_stream_and_wait(chan, join_sound, "");
ast_autoservice_start(chan);
play_sound_file(conference_bridge, join_sound);
ast_autoservice_stop(chan);
- if (conference_bridge_user.playing_moh) {
- ast_moh_start(chan, conference_bridge_user.u_profile.moh_class, NULL);
- }
}
/* See if we need to automatically set this user as a video source or not */
handle_video_on_join(conference_bridge, conference_bridge_user.chan, ast_test_flag(&conference_bridge_user.u_profile, USER_OPT_MARKEDUSER));
+
+ conf_moh_unsuspend(&conference_bridge_user);
/* Join our conference bridge for real */
send_join_event(conference_bridge_user.chan, conference_bridge->name);
@@ -1916,25 +1998,14 @@
struct conf_menu_entry *menu_entry,
struct conf_menu *menu)
{
- struct conference_bridge *conference_bridge = conference_bridge_user->conference_bridge;
-
/* See if music on hold is playing */
- ao2_lock(conference_bridge);
- if (conference_bridge_user->playing_moh) {
- /* MOH is going, let's stop it */
- ast_moh_stop(bridge_channel->chan);
- }
- ao2_unlock(conference_bridge);
+ conf_moh_suspend(conference_bridge_user);
/* execute the list of actions associated with this menu entry */
- execute_menu_entry(conference_bridge, conference_bridge_user, bridge_channel, menu_entry, menu);
+ execute_menu_entry(conference_bridge_user->conference_bridge, conference_bridge_user, bridge_channel, menu_entry, menu);
/* See if music on hold needs to be started back up again */
- ao2_lock(conference_bridge);
- if (conference_bridge_user->playing_moh) {
- ast_moh_start(bridge_channel->chan, conference_bridge_user->u_profile.moh_class, NULL);
- }
- ao2_unlock(conference_bridge);
+ conf_moh_unsuspend(conference_bridge_user);
return 0;
}
@@ -2826,13 +2897,7 @@
/* Turn on MOH/mute if the single participant is set up for it */
if (ast_test_flag(&only_participant->u_profile, USER_OPT_MUSICONHOLD)) {
only_participant->features.mute = 1;
- if (!ast_channel_internal_bridge(only_participant->chan) || !ast_bridge_suspend(conference_bridge->bridge, only_participant->chan)) {
- ast_moh_start(only_participant->chan, only_participant->u_profile.moh_class, NULL);
- only_participant->playing_moh = 1;
- if (ast_channel_internal_bridge(only_participant->chan)) {
- ast_bridge_unsuspend(conference_bridge->bridge, only_participant->chan);
- }
- }
+ conf_moh_start(only_participant);
}
}
Modified: branches/11/apps/confbridge/conf_state.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/apps/confbridge/conf_state.c?view=diff&rev=377993&r1=377992&r2=377993
==============================================================================
--- branches/11/apps/confbridge/conf_state.c (original)
+++ branches/11/apps/confbridge/conf_state.c Thu Dec 13 15:04:16 2012
@@ -47,9 +47,28 @@
ast_log(LOG_ERROR, "Invalid event for confbridge user '%s'\n", cbu->u_profile.name);
}
+/*!
+ * \internal
+ * \brief Mute the user and play MOH if the user requires it.
+ *
+ * \param user Conference user to mute and optionally start MOH on.
+ *
+ * \return Nothing
+ */
+static void conf_mute_moh_inactive_waitmarked(struct conference_bridge_user *user)
+{
+ /* Be sure we are muted so we can't talk to anybody else waiting */
+ user->features.mute = 1;
+ /* Start music on hold if needed */
+ if (ast_test_flag(&user->u_profile, USER_OPT_MUSICONHOLD)) {
+ conf_moh_start(user);
+ }
+}
+
void conf_default_join_waitmarked(struct conference_bridge_user *cbu)
{
conf_add_user_waiting(cbu->conference_bridge, cbu);
+ conf_mute_moh_inactive_waitmarked(cbu);
conf_add_post_join_action(cbu, conf_handle_inactive_waitmarked);
}
Modified: branches/11/apps/confbridge/conf_state_multi_marked.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/apps/confbridge/conf_state_multi_marked.c?view=diff&rev=377993&r1=377992&r2=377993
==============================================================================
--- branches/11/apps/confbridge/conf_state_multi_marked.c (original)
+++ branches/11/apps/confbridge/conf_state_multi_marked.c Thu Dec 13 15:04:16 2012
@@ -107,12 +107,8 @@
cbu_iter->conference_bridge->waitingusers++;
/* Handle muting/moh of cbu_iter if necessary */
if (ast_test_flag(&cbu_iter->u_profile, USER_OPT_MUSICONHOLD)) {
- cbu_iter->features.mute = 1;
- if (!ast_bridge_suspend(cbu_iter->conference_bridge->bridge, cbu_iter->chan)) {
- ast_moh_start(cbu_iter->chan, cbu_iter->u_profile.moh_class, NULL);
- cbu_iter->playing_moh = 1;
- ast_bridge_unsuspend(cbu_iter->conference_bridge->bridge, cbu_iter->chan);
- }
+ cbu_iter->features.mute = 1;
+ conf_moh_start(cbu_iter);
}
}
}
@@ -173,10 +169,8 @@
cbu->conference_bridge->waitingusers--;
AST_LIST_INSERT_TAIL(&cbu->conference_bridge->active_list, cbu_iter, list);
cbu->conference_bridge->activeusers++;
- if (cbu_iter->playing_moh && !ast_bridge_suspend(cbu->conference_bridge->bridge, cbu_iter->chan)) {
- cbu_iter->playing_moh = 0;
- ast_moh_stop(cbu_iter->chan);
- ast_bridge_unsuspend(cbu->conference_bridge->bridge, cbu_iter->chan);
+ if (cbu_iter->playing_moh) {
+ conf_moh_stop(cbu_iter);
}
/* only unmute them if they are not supposed to start muted */
if (!ast_test_flag(&cbu_iter->u_profile, USER_OPT_STARTMUTED)) {
Modified: branches/11/apps/confbridge/include/confbridge.h
URL: http://svnview.digium.com/svn/asterisk/branches/11/apps/confbridge/include/confbridge.h?view=diff&rev=377993&r1=377992&r2=377993
==============================================================================
--- branches/11/apps/confbridge/include/confbridge.h (original)
+++ branches/11/apps/confbridge/include/confbridge.h Thu Dec 13 15:04:16 2012
@@ -236,6 +236,7 @@
struct ast_channel *chan; /*!< Asterisk channel participating */
struct ast_bridge_features features; /*!< Bridge features structure */
struct ast_bridge_tech_optimizations tech_args; /*!< Bridge technology optimizations for talk detection */
+ unsigned int suspended_moh; /*!< Count of active suspended MOH actions. */
unsigned int kicked:1; /*!< User has been kicked from the conference */
unsigned int playing_moh:1; /*!< MOH is currently being played to the user */
AST_LIST_HEAD_NOLOCK(, post_join_action) post_join_list; /*!< List of sounds to play after joining */;
@@ -358,6 +359,24 @@
*/
void conf_ended(struct conference_bridge *conference_bridge);
+/*!
+ * \brief Stop MOH for the conference user.
+ *
+ * \param user Conference user to stop MOH on.
+ *
+ * \return Nothing
+ */
+void conf_moh_stop(struct conference_bridge_user *user);
+
+/*!
+ * \brief Start MOH for the conference user.
+ *
+ * \param user Conference user to start MOH on.
+ *
+ * \return Nothing
+ */
+void conf_moh_start(struct conference_bridge_user *user);
+
/*! \brief Attempt to mute/play MOH to the only user in the conference if they require it
* \param conference_bridge A conference bridge containing a single user
*/
Modified: branches/11/include/asterisk/bridging.h
URL: http://svnview.digium.com/svn/asterisk/branches/11/include/asterisk/bridging.h?view=diff&rev=377993&r1=377992&r2=377993
==============================================================================
--- branches/11/include/asterisk/bridging.h (original)
+++ branches/11/include/asterisk/bridging.h Thu Dec 13 15:04:16 2012
@@ -271,6 +271,32 @@
*/
struct ast_bridge *ast_bridge_new(uint32_t capabilities, int flags);
+/*!
+ * \brief Lock the bridge.
+ *
+ * \param bridge Bridge to lock
+ *
+ * \return Nothing
+ */
+#define ast_bridge_lock(bridge) _ast_bridge_lock(bridge, __FILE__, __PRETTY_FUNCTION__, __LINE__, #bridge)
+static inline void _ast_bridge_lock(struct ast_bridge *bridge, const char *file, const char *function, int line, const char *var)
+{
+ __ao2_lock(bridge, AO2_LOCK_REQ_MUTEX, file, function, line, var);
+}
+
+/*!
+ * \brief Unlock the bridge.
+ *
+ * \param bridge Bridge to unlock
+ *
+ * \return Nothing
+ */
+#define ast_bridge_unlock(bridge) _ast_bridge_unlock(bridge, __FILE__, __PRETTY_FUNCTION__, __LINE__, #bridge)
+static inline void _ast_bridge_unlock(struct ast_bridge *bridge, const char *file, const char *function, int line, const char *var)
+{
+ __ao2_unlock(bridge, file, function, line, var);
+}
+
/*! \brief See if it is possible to create a bridge
*
* \param capabilities The capabilities that the bridge will use
More information about the svn-commits
mailing list