[asterisk-commits] mjordan: branch 10 r374652 - in /branches/10/apps: ./ confbridge/ confbridge/...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Mon Oct 8 08:46:34 CDT 2012


Author: mjordan
Date: Mon Oct  8 08:46:27 2012
New Revision: 374652

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=374652
Log:
Resolve issues in ConfBridge regarding marked, waitmarked, and unmarked users

Thank's to Neil Tallim (flan)'s tireless testing, issue reporting, and patches
it became clear that app_confbridge had some complex logic in how it handled
interactions between marked, waitmarked, and unmarked users.  In particular,
there were some areas in which the interactions between the users resulted
in inconsistent behavior, and app_confbridge was missing logic in how to handle
some corner cases.  Some areas included:
 * Poor handling of mixing unmarked and waitmarked users
 * Inconsistencies in how MOH and muting was applied to various users
 * Handling of various announcements for different user profile options
flan's patches seem to fix the various issues, but highlighted how hard the
code could be to maintain.  In an attempt to make things easier to maintain and
to more fully enumerate the various cases that exist, this patch breaks up the
logic into a state machine-like setup.

Please note that the various state transitioned are documented on the Asterisk
wiki:

https://wiki.asterisk.org/wiki/display/AST/Confbridge+state+changes

Review: //https://reviewboard.asterisk.org/r/2072/

Note that for the following issues, mjordan uploaded the patch, although it
was written by twilson.  Any contributor license discrepency is due to that.

(closes issue ASTERISK-19562)
Reported by: flan
Tested by: flan, mjordan, jrose
patches:
  bugASTERISK-19562_ASTERISK-19726_ASTERISK-20181.patch uploaded by twilson (license 6283)

(closes issue ASTERISK-19726)
Reported by: flan
Tested by: flan
patches:
  bugASTERISK-19562_ASTERISK-19726_ASTERISK-20181.patch uploaded by twilson (license 6283)

(closes issue ASTERISK-20181)
Reported by: Jonathan White
Tested by: Jonathan White
patches:
  bugASTERISK-19562_ASTERISK-19726_ASTERISK-20181.patch uploaded by twilson (license 6283)



Added:
    branches/10/apps/confbridge/conf_state.c   (with props)
    branches/10/apps/confbridge/conf_state_empty.c   (with props)
    branches/10/apps/confbridge/conf_state_inactive.c   (with props)
    branches/10/apps/confbridge/conf_state_multi.c   (with props)
    branches/10/apps/confbridge/conf_state_multi_marked.c   (with props)
    branches/10/apps/confbridge/conf_state_single.c   (with props)
    branches/10/apps/confbridge/conf_state_single_marked.c   (with props)
    branches/10/apps/confbridge/include/conf_state.h   (with props)
Modified:
    branches/10/apps/app_confbridge.c
    branches/10/apps/confbridge/include/confbridge.h

Modified: branches/10/apps/app_confbridge.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/apps/app_confbridge.c?view=diff&rev=374652&r1=374651&r2=374652
==============================================================================
--- branches/10/apps/app_confbridge.c (original)
+++ branches/10/apps/app_confbridge.c Mon Oct  8 08:46:27 2012
@@ -273,10 +273,16 @@
 /* Number of buckets our conference bridges container can have */
 #define CONFERENCE_BRIDGE_BUCKETS 53
 
+enum {
+	CONF_RECORD_EXIT = 0,
+	CONF_RECORD_START,
+	CONF_RECORD_STOP,
+};
+
 /*! \brief Container to hold all conference bridges in progress */
 static struct ao2_container *conference_bridges;
 
-static int play_sound_file(struct conference_bridge *conference_bridge, const char *filename);
+static void leave_conference_bridge(struct conference_bridge *conference_bridge, struct conference_bridge_user *conference_bridge_user);
 static int play_sound_number(struct conference_bridge *conference_bridge, int say_number);
 static int execute_menu_entry(struct conference_bridge *conference_bridge,
 	struct conference_bridge_user *conference_bridge_user,
@@ -390,136 +396,166 @@
 	struct ast_channel *chan;
 	struct ast_str *filename = ast_str_alloca(PATH_MAX);
 
+	ast_mutex_lock(&conference_bridge->record_lock);
 	if (!mixmonapp) {
+		ast_log(LOG_WARNING, "Can not record ConfBridge, MixMonitor app is not installed\n");
+		conference_bridge->record_thread = AST_PTHREADT_NULL;
+		ast_mutex_unlock(&conference_bridge->record_lock);
 		ao2_ref(conference_bridge, -1);
 		return NULL;
 	}
 
-	ao2_lock(conference_bridge);
-	if (!(conference_bridge->record_chan)) {
-		conference_bridge->record_thread = AST_PTHREADT_NULL;
-		ao2_unlock(conference_bridge);
-		ao2_ref(conference_bridge, -1);
-		return NULL;
-	}
-	chan = ast_channel_ref(conference_bridge->record_chan);
-
-	if (!(ast_strlen_zero(conference_bridge->b_profile.rec_file))) {
-		ast_str_append(&filename, 0, "%s", conference_bridge->b_profile.rec_file);
-	} else {
-		time_t now;
-		time(&now);
-		ast_str_append(&filename, 0, "confbridge-%s-%u.wav",
-			conference_bridge->name,
-			(unsigned int) now);
-	}
-	ao2_unlock(conference_bridge);
-
-	ast_answer(chan);
-	pbx_exec(chan, mixmonapp, ast_str_buffer(filename));
-	ast_bridge_join(conference_bridge->bridge, chan, NULL, NULL, NULL);
-
-	ao2_lock(conference_bridge);
-	conference_bridge->record_thread = AST_PTHREADT_NULL;
-	ao2_unlock(conference_bridge);
-
-	ast_hangup(chan); /* This will eat this threads reference to the channel as well */
+	/* XXX If we get an EXIT right here, START will essentially be a no-op */
+	while (conference_bridge->record_state != CONF_RECORD_EXIT) {
+		if (!(ast_strlen_zero(conference_bridge->b_profile.rec_file))) {
+			ast_str_append(&filename, 0, "%s", conference_bridge->b_profile.rec_file);
+		} else {
+			time_t now;
+			time(&now);
+			ast_str_append(&filename, 0, "confbridge-%s-%u.wav",
+				conference_bridge->name,
+				(unsigned int) now);
+		}
+
+		chan = ast_channel_ref(conference_bridge->record_chan);
+		ast_answer(chan);
+		pbx_exec(chan, mixmonapp, ast_str_buffer(filename));
+		ast_bridge_join(conference_bridge->bridge, chan, NULL, NULL, NULL);
+
+		ast_hangup(chan); /* This will eat this thread's reference to the channel as well */
+		/* STOP has been called. Wait for either a START or an EXIT */
+		ast_cond_wait(&conference_bridge->record_cond, &conference_bridge->record_lock);
+	}
+	ast_mutex_unlock(&conference_bridge->record_lock);
 	ao2_ref(conference_bridge, -1);
 	return NULL;
 }
 
-/*!
- * \internal
- * \brief Returns whether or not conference is being recorded.
+/*! \brief Returns whether or not conference is being recorded.
+ * \param conference_bridge The bridge to check for recording
  * \retval 1, conference is recording.
  * \retval 0, conference is NOT recording.
  */
 static int conf_is_recording(struct conference_bridge *conference_bridge)
 {
-	int res = 0;
-	ao2_lock(conference_bridge);
-	if (conference_bridge->record_chan || conference_bridge->record_thread != AST_PTHREADT_NULL) {
-		res = 1;
-	}
-	ao2_unlock(conference_bridge);
-	return res;
+	return conference_bridge->record_state == CONF_RECORD_START;
+}
+
+/*! \brief Stop recording a conference bridge
+ * \internal
+ * \param conference_bridge The conference bridge on which to stop the recording
+ * \retval -1 Failure
+ * \retval 0 Success
+ */
+static int conf_stop_record(struct conference_bridge *conference_bridge)
+{
+	struct ast_channel *chan;
+	if (conference_bridge->record_thread == AST_PTHREADT_NULL || !conf_is_recording(conference_bridge)) {
+		return -1;
+	}
+	conference_bridge->record_state = CONF_RECORD_STOP;
+	chan = ast_channel_ref(conference_bridge->record_chan);
+	ast_bridge_remove(conference_bridge->bridge, chan);
+	ast_queue_frame(chan, &ast_null_frame);
+	chan = ast_channel_unref(chan);
+	ast_test_suite_event_notify("CONF_STOP_RECORD", "Message: stopped conference recording channel\r\nConference: %s", conference_bridge->b_profile.name);
+
+	return 0;
 }
 
 /*!
  * \internal
  * \brief Stops the confbridge recording thread.
  *
- * \note do not call this function with any locks
+ * \note Must be called with the conference_bridge locked
  */
-static int conf_stop_record(struct conference_bridge *conference_bridge)
-{
-	ao2_lock(conference_bridge);
-
-	if (conference_bridge->record_thread != AST_PTHREADT_NULL) {
-		struct ast_channel *chan = ast_channel_ref(conference_bridge->record_chan);
-		pthread_t thread = conference_bridge->record_thread;
-		ao2_unlock(conference_bridge);
-
-		ast_bridge_remove(conference_bridge->bridge, chan);
-		ast_queue_frame(chan, &ast_null_frame);
-
-		chan = ast_channel_unref(chan);
-		pthread_join(thread, NULL);
-		ast_test_suite_event_notify("CONF_STOP_RECORD", "Message: stopped conference recording channel\r\nConference: %s", conference_bridge->b_profile.name);
-
-		ao2_lock(conference_bridge);
-	}
+static int conf_stop_record_thread(struct conference_bridge *conference_bridge)
+{
+	if (conference_bridge->record_thread == AST_PTHREADT_NULL) {
+		return -1;
+	}
+	conf_stop_record(conference_bridge);
+
+	ast_mutex_lock(&conference_bridge->record_lock);
+	conference_bridge->record_state = CONF_RECORD_EXIT;
+	ast_cond_signal(&conference_bridge->record_cond);
+	ast_mutex_unlock(&conference_bridge->record_lock);
+
+	pthread_join(conference_bridge->record_thread, NULL);
+	conference_bridge->record_thread = AST_PTHREADT_NULL;
 
 	/* this is the reference given to the channel during the channel alloc */
 	if (conference_bridge->record_chan) {
 		conference_bridge->record_chan = ast_channel_unref(conference_bridge->record_chan);
 	}
 
-	ao2_unlock(conference_bridge);
-	return 0;
-}
-
+	return 0;
+}
+
+/*! \brief Start recording the conference
+ * \internal
+ * \note conference_bridge must be locked when calling this function
+ * \param conference_bridge The conference bridge to start recording
+ * \retval 0 success
+ * \rteval non-zero failure
+ */
 static int conf_start_record(struct conference_bridge *conference_bridge)
 {
-	struct ast_format_cap *cap = ast_format_cap_alloc_nolock();
+	struct ast_format_cap *cap;
 	struct ast_format tmpfmt;
 	int cause;
 
-	ao2_lock(conference_bridge);
-	if (conference_bridge->record_chan || conference_bridge->record_thread != AST_PTHREADT_NULL) {
-		ao2_unlock(conference_bridge);
-		return -1; /* already recording */
-	}
-	if (!cap) {
-		ao2_unlock(conference_bridge);
+	if (conference_bridge->record_state != CONF_RECORD_STOP) {
 		return -1;
 	}
+
 	if (!pbx_findapp("MixMonitor")) {
 		ast_log(LOG_WARNING, "Can not record ConfBridge, MixMonitor app is not installed\n");
-		cap = ast_format_cap_destroy(cap);
-		ao2_unlock(conference_bridge);
 		return -1;
 	}
+
+	if (!(cap = ast_format_cap_alloc_nolock())) {
+		return -1;
+	}
+
 	ast_format_cap_add(cap, ast_format_set(&tmpfmt, AST_FORMAT_SLINEAR, 0));
+
 	if (!(conference_bridge->record_chan = ast_request("ConfBridgeRec", cap, NULL, conference_bridge->name, &cause))) {
 		cap = ast_format_cap_destroy(cap);
-		ao2_unlock(conference_bridge);
 		return -1;
 	}
 
 	cap = ast_format_cap_destroy(cap);
+
+	conference_bridge->record_state = CONF_RECORD_START;
+	ast_mutex_lock(&conference_bridge->record_lock);
+	ast_cond_signal(&conference_bridge->record_cond);
+	ast_mutex_unlock(&conference_bridge->record_lock);
+	ast_test_suite_event_notify("CONF_START_RECORD", "Message: started conference recording channel\r\nConference: %s", conference_bridge->b_profile.name);
+
+	return 0;
+}
+
+/*! \brief Start the recording thread on a conference bridge
+ * \internal
+ * \param conference_bridge The conference bridge on which to start the recording thread
+ * \retval 0 success
+ * \retval -1 failure
+ */
+static int start_conf_record_thread(struct conference_bridge *conference_bridge)
+{
 	ao2_ref(conference_bridge, +1); /* give the record thread a ref */
+
+	ao2_lock(conference_bridge);
+	conf_start_record(conference_bridge);
+	ao2_unlock(conference_bridge);
 
 	if (ast_pthread_create_background(&conference_bridge->record_thread, NULL, record_thread, conference_bridge)) {
 		ast_log(LOG_WARNING, "Failed to create recording channel for conference %s\n", conference_bridge->name);
-
-		ao2_unlock(conference_bridge);
 		ao2_ref(conference_bridge, -1); /* error so remove ref */
 		return -1;
 	}
 
-	ast_test_suite_event_notify("CONF_START_RECORD", "Message: started conference recording channel\r\nConference: %s", conference_bridge->b_profile.name);
-	ao2_unlock(conference_bridge);
 	return 0;
 }
 
@@ -580,10 +616,10 @@
 	const char *only_one = conf_get_sound(CONF_SOUND_ONLY_ONE, conference_bridge->b_profile.sounds);
 	const char *there_are = conf_get_sound(CONF_SOUND_THERE_ARE, conference_bridge->b_profile.sounds);
 
-	if (conference_bridge->users == 1) {
-		/* Awww we are the only person in the conference bridge */
-		return 0;
-	} else if (conference_bridge->users == 2) {
+	if (conference_bridge->activeusers <= 1) {
+		/* Awww we are the only person in the conference bridge OR we only have waitmarked users */
+		return 0;
+	} else if (conference_bridge->activeusers == 2) {
 		if (conference_bridge_user) {
 			/* Eep, there is one other person */
 			if (ast_stream_and_wait(conference_bridge_user->chan,
@@ -602,7 +638,7 @@
 				"")) {
 				return -1;
 			}
-			if (ast_say_number(conference_bridge_user->chan, conference_bridge->users - 1, "", conference_bridge_user->chan->language, NULL)) {
+			if (ast_say_number(conference_bridge_user->chan, conference_bridge->activeusers - 1, "", conference_bridge_user->chan->language, NULL)) {
 				return -1;
 			}
 			if (ast_stream_and_wait(conference_bridge_user->chan,
@@ -612,7 +648,7 @@
 			}
 		} else if (ast_fileexists(there_are, NULL, NULL) && ast_fileexists(other_in_party, NULL, NULL)) {
 			play_sound_file(conference_bridge, there_are);
-			play_sound_number(conference_bridge, conference_bridge->users - 1);
+			play_sound_number(conference_bridge, conference_bridge->activeusers - 1);
 			play_sound_file(conference_bridge, other_in_party);
 		}
 	}
@@ -627,16 +663,13 @@
  * \param file Prompt to play
  *
  * \return Returns 0 on success, -1 if the user hung up
- *
- * \note This function assumes that conference_bridge is locked
+ * \note Generally this should be called when the conference is unlocked to avoid blocking
+ * the entire conference while the sound is played. But don't unlock the conference bridge
+ * in the middle of a state transition.
  */
-static int play_prompt_to_channel(struct conference_bridge *conference_bridge, struct ast_channel *chan, const char *file)
-{
-	int res;
-	ao2_unlock(conference_bridge);
-	res = ast_stream_and_wait(chan, file, "");
-	ao2_lock(conference_bridge);
-	return res;
+static int play_prompt_to_user(struct conference_bridge_user *cbu, const char *filename)
+{
+	return ast_stream_and_wait(cbu->chan, filename, "");
 }
 
 static void handle_video_on_join(struct conference_bridge *conference_bridge, struct ast_channel *chan, int marked)
@@ -651,7 +684,7 @@
 		struct conference_bridge_user *tmp_user = NULL;
 		ao2_lock(conference_bridge);
 		/* see if anyone is already the video src */
-		AST_LIST_TRAVERSE(&conference_bridge->users_list, tmp_user, list) {
+		AST_LIST_TRAVERSE(&conference_bridge->active_list, tmp_user, list) {
 			if (tmp_user->chan == chan) {
 				continue;
 			}
@@ -696,7 +729,7 @@
 
 	/* Make the next available marked user the video src.  */
 	ao2_lock(conference_bridge);
-	AST_LIST_TRAVERSE(&conference_bridge->users_list, tmp_user, list) {
+	AST_LIST_TRAVERSE(&conference_bridge->active_list, tmp_user, list) {
 		if (tmp_user->chan == chan) {
 			continue;
 		}
@@ -706,151 +739,6 @@
 		}
 	}
 	ao2_unlock(conference_bridge);
-}
-
-/*!
- * \brief Perform post-joining marked specific actions
- *
- * \param conference_bridge Conference bridge being joined
- * \param conference_bridge_user Conference bridge user joining
- *
- * \return Returns 0 on success, -1 if the user hung up
- */
-static int post_join_marked(struct conference_bridge *conference_bridge, struct conference_bridge_user *conference_bridge_user)
-{
-	if (ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_MARKEDUSER)) {
-		struct conference_bridge_user *other_conference_bridge_user = NULL;
-
-		/* If we are not the first user to join, then the users are already
-		 * in the conference so we do not need to update them. */
-		if (conference_bridge->markedusers >= 2) {
-			return 0;
-		}
-
-		/* Iterate through every participant stopping MOH on them if need be */
-		AST_LIST_TRAVERSE(&conference_bridge->users_list, other_conference_bridge_user, list) {
-			if (other_conference_bridge_user == conference_bridge_user) {
-				continue;
-			}
-			if (other_conference_bridge_user->playing_moh && !ast_bridge_suspend(conference_bridge->bridge, other_conference_bridge_user->chan)) {
-				other_conference_bridge_user->playing_moh = 0;
-				ast_moh_stop(other_conference_bridge_user->chan);
-				ast_bridge_unsuspend(conference_bridge->bridge, other_conference_bridge_user->chan);
-			}
-		}
-
-		/* Next play the audio file stating they are going to be placed into the conference */
-		if (!ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_QUIET)) {
-			if (play_prompt_to_channel(conference_bridge,
-				conference_bridge_user->chan,
-				conf_get_sound(CONF_SOUND_PLACE_IN_CONF, conference_bridge_user->b_profile.sounds))) {
-				/* user hungup while the sound was playing */
-				return -1;
-			}
-		}
-
-		/* Finally iterate through and unmute them all */
-		AST_LIST_TRAVERSE(&conference_bridge->users_list, other_conference_bridge_user, list) {
-			if (other_conference_bridge_user == conference_bridge_user) {
-				continue;
-			}
-			/* only unmute them if they are not supposed to start muted */
-			if (!ast_test_flag(&other_conference_bridge_user->u_profile, USER_OPT_STARTMUTED)) {
-				other_conference_bridge_user->features.mute = 0;
-			}
-		}
-	} else {
-		/* If a marked user already exists in the conference bridge we can just bail out now */
-		if (conference_bridge->markedusers) {
-			return 0;
-		}
-		/* Be sure we are muted so we can't talk to anybody else waiting */
-		conference_bridge_user->features.mute = 1;
-		/* If we have not been quieted play back that they are waiting for the leader */
-		if (!ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_QUIET)) {
-			if (play_prompt_to_channel(conference_bridge,
-				conference_bridge_user->chan,
-				conf_get_sound(CONF_SOUND_WAIT_FOR_LEADER, conference_bridge_user->b_profile.sounds))) {
-				/* user hungup while the sound was playing */
-				return -1;
-			}
-		}
-		/* Start music on hold if needed */
-		/* We need to recheck the markedusers value here. play_prompt_to_channel unlocks the conference bridge, potentially
-		 * allowing a marked user to enter while the prompt was playing
-		 */
-		if (!conference_bridge->markedusers && ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_MUSICONHOLD)) {
-			ast_moh_start(conference_bridge_user->chan, conference_bridge_user->u_profile.moh_class, NULL);
-			conference_bridge_user->playing_moh = 1;
-		}
-	}
-	return 0;
-}
-
-/*!
- * \brief Perform post-joining non-marked specific actions
- *
- * \param conference_bridge Conference bridge being joined
- * \param conference_bridge_user Conference bridge user joining
- *
- * \return Returns 0 on success, -1 if the user hung up
- */
-static int post_join_unmarked(struct conference_bridge *conference_bridge, struct conference_bridge_user *conference_bridge_user)
-{
-	/* Play back audio prompt and start MOH if need be if we are the first participant */
-	if (conference_bridge->users == 1) {
-		/* If audio prompts have not been quieted or this prompt quieted play it on out */
-		if (!ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_QUIET | USER_OPT_NOONLYPERSON)) {
-			if (play_prompt_to_channel(conference_bridge,
-				conference_bridge_user->chan,
-				conf_get_sound(CONF_SOUND_ONLY_PERSON, conference_bridge_user->b_profile.sounds))) {
-				/* user hungup while the sound was playing */
-				return -1;
-			}
-		}
-		/* If we need to start music on hold on the channel do so now */
-		/* We need to re-check the number of users in the conference bridge here because another conference bridge
-		 * participant could have joined while the above prompt was playing for the first user.
-		 */
-		if (conference_bridge->users == 1 && ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_MUSICONHOLD)) {
-			ast_moh_start(conference_bridge_user->chan, conference_bridge_user->u_profile.moh_class, NULL);
-			conference_bridge_user->playing_moh = 1;
-		}
-		return 0;
-	}
-
-	/* Announce number of users if need be */
-	if (ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_ANNOUNCEUSERCOUNT)) {
-		ao2_unlock(conference_bridge);
-		if (announce_user_count(conference_bridge, conference_bridge_user)) {
-			ao2_lock(conference_bridge);
-			return -1;
-		}
-		ao2_lock(conference_bridge);
-	}
-
-	/* If we are the second participant we may need to stop music on hold on the first */
-	if (conference_bridge->users == 2) {
-		struct conference_bridge_user *first_participant = AST_LIST_FIRST(&conference_bridge->users_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(&conference_bridge_user->u_profile, USER_OPT_ANNOUNCEUSERCOUNTALL) &&
-		(conference_bridge->users > conference_bridge_user->u_profile.announce_user_count_all_after)) {
-		ao2_unlock(conference_bridge);
-		if (announce_user_count(conference_bridge, NULL)) {
-			ao2_lock(conference_bridge);
-			return -1;
-		}
-		ao2_lock(conference_bridge);
-	}
-	return 0;
 }
 
 /*!
@@ -885,7 +773,146 @@
 	conf_bridge_profile_destroy(&conference_bridge->b_profile);
 }
 
-static void leave_conference_bridge(struct conference_bridge *conference_bridge, struct conference_bridge_user *conference_bridge_user);
+/*! \brief Call the proper join event handler for the user for the conference bridge's current state
+ * \internal
+ * \param cbu The conference bridge user that is joining
+ * \retval 0 success
+ * \retval -1 failure
+ */
+static int handle_conf_user_join(struct conference_bridge_user *cbu)
+{
+	conference_event_fn handler;
+	if (ast_test_flag(&cbu->u_profile, USER_OPT_MARKEDUSER)) {
+		handler = cbu->conference_bridge->state->join_marked;
+	} else if (ast_test_flag(&cbu->u_profile, USER_OPT_WAITMARKED)) {
+		handler = cbu->conference_bridge->state->join_waitmarked;
+	} else {
+		handler = cbu->conference_bridge->state->join_unmarked;
+	}
+
+	ast_assert(handler != NULL);
+
+	if (!handler) {
+		conf_invalid_event_fn(cbu);
+		return -1;
+	}
+
+	handler(cbu);
+
+	return 0;
+}
+
+/*! \brief Call the proper leave event handler for the user for the conference bridge's current state
+ * \internal
+ * \param cbu The conference bridge user that is leaving
+ * \retval 0 success
+ * \retval -1 failure
+ */
+static int handle_conf_user_leave(struct conference_bridge_user *cbu)
+{
+	conference_event_fn handler;
+	if (ast_test_flag(&cbu->u_profile, USER_OPT_MARKEDUSER)) {
+		handler = cbu->conference_bridge->state->leave_marked;
+	} else if (ast_test_flag(&cbu->u_profile, USER_OPT_WAITMARKED)) {
+		handler = cbu->conference_bridge->state->leave_waitmarked;
+	} else {
+		handler = cbu->conference_bridge->state->leave_unmarked;
+	}
+
+	ast_assert(handler != NULL);
+
+	if (!handler) {
+		/* This should never happen. If it does, though, it is bad. The user will not have been removed
+		 * from the appropriate list, so counts will be off and stuff. The conference won't be torn down, etc.
+		 * Shouldn't happen, though. */
+		conf_invalid_event_fn(cbu);
+		return -1;
+	}
+
+	handler(cbu);
+
+	return 0;
+}
+
+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))) {
+		return -1;
+	}
+	return 0;
+}
+
+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;
+}
+
+int conf_handle_only_unmarked(struct conference_bridge_user *cbu)
+{
+	/* If audio prompts have not been quieted or this prompt quieted play it on out */
+	if (!ast_test_flag(&cbu->u_profile, USER_OPT_QUIET | USER_OPT_NOONLYPERSON)) {
+		if (play_prompt_to_user(cbu,
+			conf_get_sound(CONF_SOUND_ONLY_PERSON, cbu->b_profile.sounds))) {
+			/* user hungup while the sound was playing */
+			return -1;
+		}
+	}
+	return 0;
+}
+
+int conf_add_post_join_action(struct conference_bridge_user *cbu, int (*func)(struct conference_bridge_user *cbu))
+{
+	struct post_join_action *action;
+	if (!(action = ast_calloc(1, sizeof(*action)))) {
+		return -1;
+	}
+	action->func = func;
+	AST_LIST_INSERT_TAIL(&cbu->post_join_list, action, list);
+	return 0;
+}
+
+
+void conf_handle_first_join(struct conference_bridge *conference_bridge)
+{
+	ast_devstate_changed(AST_DEVICE_INUSE, "confbridge:%s", conference_bridge->name);
+}
+
+void conf_handle_second_active(struct conference_bridge *conference_bridge)
+{
+	/* 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_STARTMUTED)) {
+		first_participant->features.mute = 0;
+	}
+}
+
+void conf_ended(struct conference_bridge *conference_bridge)
+{
+	/* Called with a reference to conference_bridge */
+	ao2_unlink(conference_bridges, conference_bridge);
+	send_conf_end_event(conference_bridge->name);
+	conf_stop_record_thread(conference_bridge);
+}
 
 /*!
  * \brief Join a conference bridge
@@ -898,8 +925,8 @@
 static struct conference_bridge *join_conference_bridge(const char *name, struct conference_bridge_user *conference_bridge_user)
 {
 	struct conference_bridge *conference_bridge = NULL;
+	struct post_join_action *action;
 	struct conference_bridge tmp;
-	int start_record = 0;
 	int max_members_reached = 0;
 
 	ast_copy_string(tmp.name, name, sizeof(tmp.name));
@@ -913,7 +940,7 @@
 	conference_bridge = ao2_find(conference_bridges, &tmp, OBJ_POINTER);
 
 	if (conference_bridge && conference_bridge->b_profile.max_members) {
-		max_members_reached = conference_bridge->b_profile.max_members > conference_bridge->users ? 0 : 1;
+		max_members_reached = conference_bridge->b_profile.max_members > conference_bridge->activeusers ? 0 : 1;
 	}
 
 	/* When finding a conference bridge that already exists make sure that it is not locked, and if so that we are not an admin */
@@ -962,9 +989,22 @@
 		/* Setup lock for playback channel */
 		ast_mutex_init(&conference_bridge->playback_lock);
 
+		/* Setup lock for the record channel */
+		ast_mutex_init(&conference_bridge->record_lock);
+		ast_cond_init(&conference_bridge->record_cond, NULL);
+
 		/* Link it into the conference bridges container */
 		ao2_link(conference_bridges, conference_bridge);
 
+		/* Set the initial state to EMPTY */
+		conference_bridge->state = CONF_STATE_EMPTY;
+
+		conference_bridge->record_state = CONF_RECORD_STOP;
+		if (ast_test_flag(&conference_bridge->b_profile, BRIDGE_OPT_RECORD_CONFERENCE)) {
+			ao2_lock(conference_bridge);
+			start_conf_record_thread(conference_bridge);
+			ao2_unlock(conference_bridge);
+		}
 
 		send_conf_start_event(conference_bridge->name);
 		ast_debug(1, "Created conference bridge '%s' and linked to container '%p'\n", name, conference_bridges);
@@ -977,46 +1017,41 @@
 
 	ao2_lock(conference_bridge);
 
-	/* All good to go, add them in */
-	AST_LIST_INSERT_TAIL(&conference_bridge->users_list, conference_bridge_user, list);
-
-	/* Increment the users count on the bridge, but record it as it is going to need to be known right after this */
-	conference_bridge->users++;
-
-	/* If the caller is a marked user bump up the count */
-	if (ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_MARKEDUSER)) {
-		conference_bridge->markedusers++;
-	}
-
-	/* Set the device state for this conference */
-	if (conference_bridge->users == 1) {
-		ast_devstate_changed(AST_DEVICE_INUSE, "confbridge:%s", conference_bridge->name);
-	}
-
-	/* If the caller is a marked user or is waiting for a marked user to enter pass 'em off, otherwise pass them off to do regular joining stuff */
-	if (ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_MARKEDUSER | USER_OPT_WAITMARKED)) {
-		if (post_join_marked(conference_bridge, conference_bridge_user)) {
-			ao2_unlock(conference_bridge);
+	if (handle_conf_user_join(conference_bridge_user)) {
+		/* Invalid event, nothing was done, so we don't want to process a leave. */
+		ao2_unlock(conference_bridge);
+		ao2_ref(conference_bridge, -1);
+		return NULL;
+	}
+
+	if (ast_check_hangup(conference_bridge_user->chan)) {
+		ao2_unlock(conference_bridge);
+		leave_conference_bridge(conference_bridge, conference_bridge_user);
+		return NULL;
+	}
+
+	ao2_unlock(conference_bridge);
+
+	/* Announce number of users if need be */
+	if (ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_ANNOUNCEUSERCOUNT)) {
+		if (announce_user_count(conference_bridge, conference_bridge_user)) {
 			leave_conference_bridge(conference_bridge, conference_bridge_user);
 			return NULL;
 		}
-	} else {
-		if (post_join_unmarked(conference_bridge, conference_bridge_user)) {
-			ao2_unlock(conference_bridge);
+	}
+
+	if (ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_ANNOUNCEUSERCOUNTALL) &&
+		(conference_bridge->activeusers > conference_bridge_user->u_profile.announce_user_count_all_after)) {
+		if (announce_user_count(conference_bridge, NULL)) {
 			leave_conference_bridge(conference_bridge, conference_bridge_user);
 			return NULL;
 		}
 	}
 
-	/* check to see if recording needs to be started or not */
-	if (ast_test_flag(&conference_bridge->b_profile, BRIDGE_OPT_RECORD_CONFERENCE) && !conf_is_recording(conference_bridge)) {
-		start_record = 1;
-	}
-
-	ao2_unlock(conference_bridge);
-
-	if (start_record) {
-		conf_start_record(conference_bridge);
+	/* Handle post-join actions */
+	while ((action = AST_LIST_REMOVE_HEAD(&conference_bridge_user->post_join_list, list))) {
+		action->func(conference_bridge_user);
+		ast_free(action);
 	}
 
 	return conference_bridge;
@@ -1033,73 +1068,10 @@
 {
 	ao2_lock(conference_bridge);
 
-	/* If this caller is a marked user bump down the count */
-	if (ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_MARKEDUSER)) {
-		conference_bridge->markedusers--;
-	}
-
-	/* Decrement the users count while keeping the previous participant count */
-	conference_bridge->users--;
-
-	/* Drop conference bridge user from the list, they be going bye bye */
-	AST_LIST_REMOVE(&conference_bridge->users_list, conference_bridge_user, list);
-
-	/* If there are still users in the conference bridge we may need to do things (such as start MOH on them) */
-	if (conference_bridge->users) {
-		if (ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_MARKEDUSER) && !conference_bridge->markedusers) {
-			struct conference_bridge_user *other_participant = NULL;
-
-			/* Start out with muting everyone */
-			AST_LIST_TRAVERSE(&conference_bridge->users_list, other_participant, list) {
-				other_participant->features.mute = 1;
-			}
-
-			/* Play back the audio prompt saying the leader has left the conference */
-			if (!ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_QUIET)) {
-				ao2_unlock(conference_bridge);
-				ast_autoservice_start(conference_bridge_user->chan);
-				play_sound_file(conference_bridge,
-					conf_get_sound(CONF_SOUND_LEADER_HAS_LEFT, conference_bridge_user->b_profile.sounds));
-				ast_autoservice_stop(conference_bridge_user->chan);
-				ao2_lock(conference_bridge);
-			}
-
-			/* Now on to starting MOH or kick if needed */
-			AST_LIST_TRAVERSE(&conference_bridge->users_list, other_participant, list) {
-				if (ast_test_flag(&other_participant->u_profile, USER_OPT_ENDMARKED)) {
-					other_participant->kicked = 1;
-					ast_bridge_remove(conference_bridge->bridge, other_participant->chan);
-				} else if (ast_test_flag(&other_participant->u_profile, USER_OPT_MUSICONHOLD) && !ast_bridge_suspend(conference_bridge->bridge, other_participant->chan)) {
-					ast_moh_start(other_participant->chan, other_participant->u_profile.moh_class, NULL);
-					other_participant->playing_moh = 1;
-					ast_bridge_unsuspend(conference_bridge->bridge, other_participant->chan);
-				}
-			}
-		} else if (conference_bridge->users == 1) {
-			/* Of course if there is one other person in here we may need to start up MOH on them */
-			struct conference_bridge_user *first_participant = AST_LIST_FIRST(&conference_bridge->users_list);
-
-			if (ast_test_flag(&first_participant->u_profile, USER_OPT_MUSICONHOLD) && !ast_bridge_suspend(conference_bridge->bridge, first_participant->chan)) {
-				ast_moh_start(first_participant->chan, first_participant->u_profile.moh_class, NULL);
-				first_participant->playing_moh = 1;
-				ast_bridge_unsuspend(conference_bridge->bridge, first_participant->chan);
-			}
-		}
-	} else {
-		/* Set device state to "not in use" */
-		ast_devstate_changed(AST_DEVICE_NOT_INUSE, "confbridge:%s", conference_bridge->name);
-
-		ao2_unlink(conference_bridges, conference_bridge);
-		send_conf_end_event(conference_bridge->name);
-	}
+	handle_conf_user_leave(conference_bridge_user);
 
 	/* Done mucking with the conference bridge, huzzah */
 	ao2_unlock(conference_bridge);
-
-	if (!conference_bridge->users) {
-		conf_stop_record(conference_bridge);
-	}
-
 	ao2_ref(conference_bridge, -1);
 }
 
@@ -1177,16 +1149,7 @@
 	return 0;
 }
 
-/*!
- * \brief Play sound file into conference bridge
- *
- * \param conference_bridge The conference bridge to play sound file into
- * \param filename Sound file to play
- *
- * \retval 0 success
- * \retval -1 failure
- */
-static int play_sound_file(struct conference_bridge *conference_bridge, const char *filename)
+int play_sound_file(struct conference_bridge *conference_bridge, const char *filename)
 {
 	return play_sound_helper(conference_bridge, filename, -1);
 }
@@ -1364,6 +1327,7 @@
 		res = -1;
 		goto confbridge_cleanup;
 	}
+
 	quiet = ast_test_flag(&conference_bridge_user.u_profile, USER_OPT_QUIET);
 
 	/* ask for a PIN immediately after finding user profile.  This has to be
@@ -1663,7 +1627,7 @@
 	}
 
 	ao2_lock(conference_bridge);
-	if (((last_participant = AST_LIST_LAST(&conference_bridge->users_list)) == conference_bridge_user)
+	if (((last_participant = AST_LIST_LAST(&conference_bridge->active_list)) == conference_bridge_user)
 		|| (ast_test_flag(&last_participant->u_profile, USER_OPT_ADMIN))) {
 		ao2_unlock(conference_bridge);
 		ast_stream_and_wait(bridge_channel->chan,
@@ -1901,7 +1865,7 @@
 		return CLI_SUCCESS;
 	}
 	ao2_lock(bridge);
-	AST_LIST_TRAVERSE(&bridge->users_list, participant, list) {
+	AST_LIST_TRAVERSE(&bridge->active_list, participant, list) {
 		if (!strncmp(a->argv[3], participant->chan->name, strlen(participant->chan->name))) {
 			break;
 		}
@@ -1942,7 +1906,7 @@
 		ast_cli(a->fd, "================================ ====== ====== ========\n");
 		i = ao2_iterator_init(conference_bridges, 0);
 		while ((bridge = ao2_iterator_next(&i))) {
-			ast_cli(a->fd, "%-32s %6i %6i %s\n", bridge->name, bridge->users, bridge->markedusers, (bridge->locked ? "locked" : "unlocked"));
+			ast_cli(a->fd, "%-32s %6i %6i %s\n", bridge->name, bridge->activeusers, bridge->markedusers, (bridge->locked ? "locked" : "unlocked"));
 			ao2_ref(bridge, -1);
 		}
 		ao2_iterator_destroy(&i);
@@ -1959,7 +1923,7 @@
 		ast_cli(a->fd, "Channel                       User Profile     Bridge Profile   Menu\n");
 		ast_cli(a->fd, "============================= ================ ================ ================\n");
 		ao2_lock(bridge);
-		AST_LIST_TRAVERSE(&bridge->users_list, participant, list) {
+		AST_LIST_TRAVERSE(&bridge->active_list, participant, list) {
 			ast_cli(a->fd, "%-29s ", participant->chan->name);
 			ast_cli(a->fd, "%-17s", participant->u_profile.name);
 			ast_cli(a->fd, "%-17s", participant->b_profile.name);
@@ -2019,7 +1983,7 @@
 		return -1;
 	}
 	ao2_lock(bridge);
-	AST_LIST_TRAVERSE(&bridge->users_list, participant, list) {
+	AST_LIST_TRAVERSE(&bridge->active_list, participant, list) {
 		if (!strncmp(user, participant->chan->name, strlen(user))) {
 			break;
 		}
@@ -2182,21 +2146,25 @@
 		ast_cli(a->fd, "Conference not found.\n");
 		return CLI_FAILURE;
 	}
+	ao2_lock(bridge);
 	if (conf_is_recording(bridge)) {
 		ast_cli(a->fd, "Conference is already being recorded.\n");
+		ao2_unlock(bridge);
 		ao2_ref(bridge, -1);
 		return CLI_SUCCESS;
 	}
 	if (!ast_strlen_zero(rec_file)) {
-		ao2_lock(bridge);
 		ast_copy_string(bridge->b_profile.rec_file, rec_file, sizeof(bridge->b_profile.rec_file));
-		ao2_unlock(bridge);
-	}
+	}
+
 	if (conf_start_record(bridge)) {
 		ast_cli(a->fd, "Could not start recording due to internal error.\n");
+		ao2_unlock(bridge);
 		ao2_ref(bridge, -1);
 		return CLI_FAILURE;
 	}
+	ao2_unlock(bridge);
+
 	ast_cli(a->fd, "Recording started\n");
 	ao2_ref(bridge, -1);
 	return CLI_SUCCESS;
@@ -2206,6 +2174,7 @@
 {
 	struct conference_bridge *bridge = NULL;
 	struct conference_bridge tmp;
+	int ret;
 
 	switch (cmd) {
 	case CLI_INIT:
@@ -2229,8 +2198,10 @@
 		ast_cli(a->fd, "Conference not found.\n");
 		return CLI_SUCCESS;
 	}
-	conf_stop_record(bridge);
-	ast_cli(a->fd, "Recording stopped.\n");
+	ao2_lock(bridge);
+	ret = conf_stop_record(bridge);
+	ao2_unlock(bridge);
+	ast_cli(a->fd, "Recording %sstopped.\n", ret ? "could not be " : "");
 	ao2_ref(bridge, -1);
 	return CLI_SUCCESS;
 }
@@ -2287,7 +2258,7 @@
 	astman_send_listack(s, m, "Confbridge user list will follow", "start");
 
 	ao2_lock(bridge);
-	AST_LIST_TRAVERSE(&bridge->users_list, participant, list) {
+	AST_LIST_TRAVERSE(&bridge->active_list, participant, list) {
 		total++;
 		astman_append(s,
 			"Event: ConfbridgeList\r\n"
@@ -2355,7 +2326,7 @@
 		"\r\n",
 		id_text,
 		bridge->name,
-		bridge->users,
+		bridge->activeusers,
 		bridge->markedusers,
 		bridge->locked ? "Yes" : "No"); 
 		ao2_unlock(bridge);
@@ -2470,7 +2441,7 @@
 	}
 
 	ao2_lock(bridge);
-	AST_LIST_TRAVERSE(&bridge->users_list, participant, list) {
+	AST_LIST_TRAVERSE(&bridge->active_list, participant, list) {
 		if (!strcasecmp(participant->chan->name, channel)) {
 			participant->kicked = 1;
 			ast_bridge_remove(bridge->bridge, participant->chan);
@@ -2512,23 +2483,25 @@
 		return 0;
 	}
 
+	ao2_lock(bridge);
 	if (conf_is_recording(bridge)) {
 		astman_send_error(s, m, "Conference is already being recorded.");
+		ao2_unlock(bridge);
 		ao2_ref(bridge, -1);
 		return 0;
 	}
 
 	if (!ast_strlen_zero(recordfile)) {
-		ao2_lock(bridge);
 		ast_copy_string(bridge->b_profile.rec_file, recordfile, sizeof(bridge->b_profile.rec_file));
-		ao2_unlock(bridge);
 	}
 

[... 1184 lines stripped ...]



More information about the asterisk-commits mailing list