[asterisk-commits] file: branch file/bridging r167020 - in /team/file/bridging: apps/ bridges/ c...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Dec 31 15:43:18 CST 2008


Author: file
Date: Wed Dec 31 15:43:17 2008
New Revision: 167020

URL: http://svn.digium.com/view/asterisk?view=rev&rev=167020
Log:
A round of changes from the code review along with a few bugs fixed.

Modified:
    team/file/bridging/apps/app_confbridge.c
    team/file/bridging/bridges/bridge_multiplexed.c
    team/file/bridging/bridges/bridge_simple.c
    team/file/bridging/bridges/bridge_softmix.c
    team/file/bridging/channels/chan_bridge.c

Modified: team/file/bridging/apps/app_confbridge.c
URL: http://svn.digium.com/view/asterisk/team/file/bridging/apps/app_confbridge.c?view=diff&rev=167020&r1=167019&r2=167020
==============================================================================
--- team/file/bridging/apps/app_confbridge.c (original)
+++ team/file/bridging/apps/app_confbridge.c Wed Dec 31 15:43:17 2008
@@ -151,12 +151,14 @@
 
 /*! \brief The structure that represents a conference bridge */
 struct conference_bridge {
-	char name[MAX_CONF_NAME];                                  /*!< Name of the conference bridge */
-	struct ast_bridge *bridge;                                 /*!< Bridge structure doing the mixing */
-	int users;                                                 /*!< Number of users present */
-	int markedusers;                                           /*!< Number of marked users present */
-	unsigned int locked:1;                                     /*!< Is this conference bridge locked? */
-	AST_LIST_HEAD_NOLOCK(, conference_bridge_user) users_list; /*!< List of users participating in the conference bridge */
+	char name[MAX_CONF_NAME];                                         /*!< Name of the conference bridge */
+	struct ast_bridge *bridge;                                        /*!< Bridge structure doing the mixing */
+	int users;                                                        /*!< Number of users present */
+	int markedusers;                                                  /*!< Number of marked users present */
+	unsigned int locked:1;                                            /*!< Is this conference bridge locked? */
+	AST_LIST_HEAD_NOLOCK(, conference_bridge_user) users_list;        /*!< List of users participating in the conference bridge */
+	struct ast_channel *playback_chan;                                /*!< Channel used for playback into the conference bridge */
+	ast_mutex_t playback_lock;                                        /*!< Lock used for playback channel */
 };
 
 /*! \brief The structure that represents a conference bridge user */
@@ -179,7 +181,7 @@
 static int conference_bridge_hash_cb(const void *obj, const int flags)
 {
 	const struct conference_bridge *conference_bridge = obj;
-	return ast_str_hash(conference_bridge->name);
+	return ast_str_case_hash(conference_bridge->name);
 }
 
 /*! \brief Comparison function used for conference bridges container */
@@ -224,12 +226,31 @@
 }
 
 /*!
+ * \brief Play back an audio file to a channel
+ *
+ * \param conference_bridge Conference bridge they are in
+ * \param chan Channel to play audio prompt to
+ * \param file Prompt to play
+ *
+ * \return Returns nothing
+ *
+ * \note This function assumes that conference_bridge is locked
+ */
+static void play_prompt_to_channel(struct conference_bridge *conference_bridge, struct ast_channel *chan, const char *file)
+{
+	ao2_unlock(conference_bridge);
+	ast_stream_and_wait(chan, file, "");
+	ao2_lock(conference_bridge);
+	return;
+}
+
+/*!
  * \brief Perform post-joining marked specific actions
  *
  * \param conference_bridge Conference bridge being joined
  * \param conference_bridge_user Conference bridge user joining
  *
- * \return Nothing
+ * \return Returns nothing
  */
 static void post_join_marked(struct conference_bridge *conference_bridge, struct conference_bridge_user *conference_bridge_user)
 {
@@ -276,9 +297,7 @@
 		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->flags, OPTION_QUIET)) {
-			ao2_unlock(conference_bridge);
-			ast_stream_and_wait(conference_bridge_user->chan, "conf-waitforleader", "");
-			ao2_lock(conference_bridge);
+			play_prompt_to_channel(conference_bridge, conference_bridge_user->chan, "conf-waitforleader");
 		}
 		/* Start music on hold if needed */
 		if (ast_test_flag(&conference_bridge_user->flags, OPTION_MUSICONHOLD)) {
@@ -295,17 +314,15 @@
  * \param conference_bridge Conference bridge being joined
  * \param conference_bridge_user Conference bridge user joining
  *
- * \return Nothing
+ * \return Returns nothing
  */
 static void 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->flags, OPTION_QUIET) && !ast_test_flag(&conference_bridge_user->flags, OPTION_NOONLYPERSON)) {
-			ao2_unlock(conference_bridge);
-			ast_stream_and_wait(conference_bridge_user->chan, "conf-onlyperson", "");
-			ao2_lock(conference_bridge);
+		if (!ast_test_flag(&conference_bridge_user->flags, OPTION_QUIET | OPTION_NOONLYPERSON)) {
+			play_prompt_to_channel(conference_bridge, conference_bridge_user->chan, "conf-onlyperson");
 		}
 		/* If we need to start music on hold on the channel do so now */
 		if (ast_test_flag(&conference_bridge_user->flags, OPTION_MUSICONHOLD)) {
@@ -340,7 +357,7 @@
  *
  * \param obj The conference bridge object
  *
- * \return Nothing
+ * \return Returns nothing
  */
 static void destroy_conference_bridge(void *obj)
 {
@@ -348,8 +365,18 @@
 
 	ast_debug(1, "Destroying conference bridge '%s'\n", conference_bridge->name);
 
+	ast_mutex_destroy(&conference_bridge->playback_lock);
+	
+	if (conference_bridge->playback_chan) {
+		struct ast_channel *underlying_channel = conference_bridge->playback_chan->tech->bridged_channel(conference_bridge->playback_chan, NULL);
+		ast_hangup(underlying_channel);
+		ast_hangup(conference_bridge->playback_chan);
+		conference_bridge->playback_chan = NULL;
+	}
+	
 	/* Destroying a conference bridge is simple, all we have to do is destroy the bridging object */
 	ast_bridge_destroy(conference_bridge->bridge);
+	conference_bridge->bridge = NULL;
 
 	return;
 }
@@ -369,6 +396,7 @@
 
 	ast_copy_string(tmp.name, name, sizeof(tmp.name));
 
+	/* We explictly lock the conference bridges container ourselves so that other callers can not create duplicate conferences at the same */
 	ao2_lock(conference_bridges);
 
 	ast_debug(1, "Trying to find conference bridge '%s'\n", name);
@@ -400,11 +428,15 @@
 		/* Create an actual bridge that will do the audio mixing */
 		if (!(conference_bridge->bridge = ast_bridge_new(AST_BRIDGE_CAPABILITY_1TO1MIX, AST_BRIDGE_FLAG_SMART))) {
 			ao2_ref(conference_bridge, -1);
+			conference_bridge = NULL;
 			ao2_unlock(conference_bridges);
 			ast_log(LOG_ERROR, "Conference bridge '%s' could not be created.\n", name);
 			return NULL;
 		}
 
+		/* Setup lock for playback channel */
+		ast_mutex_init(&conference_bridge->playback_lock);
+		
 		/* Link it into the conference bridges container */
 		ao2_link(conference_bridges, conference_bridge);
 		ao2_ref(conference_bridge, -1);
@@ -431,7 +463,7 @@
 	}
 
 	/* 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->flags, OPTION_MARKEDUSER) || ast_test_flag(&conference_bridge_user->flags, OPTION_WAITMARKED)) {
+	if (ast_test_flag(&conference_bridge_user->flags, OPTION_MARKEDUSER | OPTION_WAITMARKED)) {
 		post_join_marked(conference_bridge, conference_bridge_user);
 	} else {
 		post_join_unmarked(conference_bridge, conference_bridge_user);
@@ -448,7 +480,8 @@
  * \param conference_bridge The conference bridge to leave
  * \param conference_bridge_user The conference bridge user structure
  *
- * \return Returns 0 on success, -1 on failure
+ * \retval 0 success
+ * \retval -1 failure
  */
 static int leave_conference_bridge(struct conference_bridge *conference_bridge, struct conference_bridge_user *conference_bridge_user)
 {
@@ -514,33 +547,49 @@
  * \param conference_bridge The conference bridge to play sound file into
  * \param filename Sound file to play
  *
- * \return Returns 0 on success, -1 on failure
+ * \retval 0 success
+ * \retval -1 failure
  */
 static int play_sound_file(struct conference_bridge *conference_bridge, const char *filename)
 {
-	int cause;
-	struct ast_channel *chan = NULL;
-
-	/* Request a channel that we will use to interface with the bridge */
-	if (!(chan = ast_request("Bridge", AST_FORMAT_SLINEAR, "", &cause))) {
-		return -1;
-	}
-
-	/* Before we actually dial into the bridge we need to say what bridge */
-	chan->bridge = conference_bridge->bridge;
-
-	/* Now actually dial. Once this is done we have a clear conduit into the bridge. */
-	if (ast_call(chan, "", 0)) {
-		ast_hangup(chan);
-		return -1;
-	}
-
-	/* Stream the file into the conference while waiting for it to finish */
-	ast_stream_and_wait(chan, filename, "");
-
-	/* Now that it is done drop the channel */
-	ast_hangup(chan);
-
+	struct ast_channel *underlying_channel;
+
+	ast_mutex_lock(&conference_bridge->playback_lock);
+	
+	if (!(conference_bridge->playback_chan)) {
+		int cause;
+		
+		if (!(conference_bridge->playback_chan = ast_request("Bridge", AST_FORMAT_SLINEAR, "", &cause))) {
+			ast_mutex_unlock(&conference_bridge->playback_lock);
+			return -1;
+		}
+
+		conference_bridge->playback_chan->bridge = conference_bridge->bridge;
+		
+		if (ast_call(conference_bridge->playback_chan, "", 0)) {
+			ast_hangup(conference_bridge->playback_chan);
+			conference_bridge->playback_chan = NULL;
+			ast_mutex_unlock(&conference_bridge->playback_lock);
+			return -1;
+		}
+
+		ast_debug(1, "Created a playback channel to conference bridge '%s'\n", conference_bridge->name);
+
+		underlying_channel = conference_bridge->playback_chan->tech->bridged_channel(conference_bridge->playback_chan, NULL);
+	} else {
+		/* Channel was already available so we just need to add it back into the bridge */
+		underlying_channel = conference_bridge->playback_chan->tech->bridged_channel(conference_bridge->playback_chan, NULL);
+		ast_bridge_impart(conference_bridge->bridge, underlying_channel, NULL, NULL);
+	}
+
+	/* The channel is all under our control, in goes the prompt */
+	ast_stream_and_wait(conference_bridge->playback_chan, filename, "");
+
+	ast_debug(1, "Departing underlying channel '%s' from bridge '%p'\n", underlying_channel->name, conference_bridge->bridge);
+	ast_bridge_depart(conference_bridge->bridge, underlying_channel);
+
+	ast_mutex_unlock(&conference_bridge->playback_lock);
+	
 	return 0;
 }
 
@@ -551,7 +600,8 @@
  * \param bridge_channel Bridged channel this is involving
  * \param hook_pvt User's conference bridge structure
  *
- * \return Returns 0 on success, -1 on failure
+ * \retval 0 success
+ * \retval -1 failure
  */
 static int menu_callback(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, void *hook_pvt)
 {

Modified: team/file/bridging/bridges/bridge_multiplexed.c
URL: http://svn.digium.com/view/asterisk/team/file/bridging/bridges/bridge_multiplexed.c?view=diff&rev=167020&r1=167019&r2=167020
==============================================================================
--- team/file/bridging/bridges/bridge_multiplexed.c (original)
+++ team/file/bridging/bridges/bridge_multiplexed.c Wed Dec 31 15:43:17 2008
@@ -55,9 +55,9 @@
 	/*! Channels in this thread */
 	struct ast_channel *chans[MULTIPLEXED_MAX_CHANNELS];
 	/*! Number of channels in this thread */
-	int count;
+	unsigned int count;
 	/*! Number of channels actually being serviced by this thread */
-	int service_count;
+	unsigned int service_count;
 	/*! Bit used to indicate that the thread is waiting on channels */
 	unsigned int waiting:1;
 };
@@ -145,7 +145,7 @@
 }
 
 /*! \brief Thread function that executes for multiplexed threads */
-static attribute_unused void *multiplexed_thread_function(void *data)
+static void *multiplexed_thread_function(void *data)
 {
 	struct multiplexed_thread *multiplexed_thread = data;
 
@@ -223,13 +223,23 @@
 /*! \brief Join function which actually adds the channel into the array to be monitored */
 static int multiplexed_bridge_join(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
 {
+	struct ast_channel *c0 = AST_LIST_FIRST(&bridge->channels)->chan, *c1 = AST_LIST_LAST(&bridge->channels)->chan;
 	struct multiplexed_thread *multiplexed_thread = bridge->bridge_pvt;
 
 	ast_debug(1, "Adding channel '%s' to multiplexed thread '%p' for monitoring\n", bridge_channel->chan->name, multiplexed_thread);
 
 	multiplexed_add_or_remove(multiplexed_thread, bridge_channel->chan, 1);
 
-	return 0;
+	/* If the second channel has not yet joined do not make things compatible */
+	if (c0 == c1) {
+		return 0;
+	}
+
+	if (((c0->writeformat == c1->readformat) && (c0->readformat == c1->writeformat) && (c0->nativeformats == c1->nativeformats))) {
+		return 0;
+	}
+	
+	return ast_channel_make_compatible(c0, c1);
 }
 
 /*! \brief Leave function which actually removes the channel from the array */

Modified: team/file/bridging/bridges/bridge_simple.c
URL: http://svn.digium.com/view/asterisk/team/file/bridging/bridges/bridge_simple.c?view=diff&rev=167020&r1=167019&r2=167020
==============================================================================
--- team/file/bridging/bridges/bridge_simple.c (original)
+++ team/file/bridging/bridges/bridge_simple.c Wed Dec 31 15:43:17 2008
@@ -78,7 +78,7 @@
 static struct ast_bridge_technology simple_bridge = {
 	.name = "simple_bridge",
 	.capabilities = AST_BRIDGE_CAPABILITY_1TO1MIX | AST_BRIDGE_CAPABILITY_NATIVE | AST_BRIDGE_CAPABILITY_THREAD,
-	.preference = AST_BRIDGE_PREFERENCE_HIGH,
+	.preference = AST_BRIDGE_PREFERENCE_MEDIUM,
 	.formats = AST_FORMAT_AUDIO_MASK | AST_FORMAT_VIDEO_MASK | AST_FORMAT_TEXT_MASK,
 	.join = simple_bridge_join,
 	.write = simple_bridge_write,

Modified: team/file/bridging/bridges/bridge_softmix.c
URL: http://svn.digium.com/view/asterisk/team/file/bridging/bridges/bridge_softmix.c?view=diff&rev=167020&r1=167019&r2=167020
==============================================================================
--- team/file/bridging/bridges/bridge_softmix.c (original)
+++ team/file/bridging/bridges/bridge_softmix.c Wed Dec 31 15:43:17 2008
@@ -55,7 +55,10 @@
 #define SOFTMIX_INTERVAL 20
 
 /*! \brief Size of the buffer used for sample manipulation */
-#define SOFTMIX_SAMPLES (160 * (SOFTMIX_INTERVAL / 10))
+#define SOFTMIX_DATALEN (160 * (SOFTMIX_INTERVAL / 10))
+
+/*! \brief Number of samples we are dealing with */
+#define SOFTMIX_SAMPLES (SOFTMIX_DATALEN / 2)
 
 /*! \brief Define used to turn on 16 kHz audio support */
 /* #define SOFTMIX_16_SUPPORT */
@@ -73,9 +76,9 @@
 	/*! Bit used to indicate that a frame is available to be written out to the channel */
 	int have_frame:1;
 	/*! Buffer containing final mixed audio from all sources */
-	short final_buf[SOFTMIX_SAMPLES];
+	short final_buf[SOFTMIX_DATALEN];
 	/*! Buffer containing only the audio from the channel */
-	short our_buf[SOFTMIX_SAMPLES];
+	short our_buf[SOFTMIX_DATALEN];
 };
 
 /*! \brief Function called when a channel is joined into the bridge */
@@ -102,8 +105,8 @@
 	sc->frame.subclass = AST_FORMAT_SLINEAR;
 #endif
 	sc->frame.data.ptr = sc->final_buf;
-	sc->frame.datalen = SOFTMIX_SAMPLES;
-	sc->frame.samples = SOFTMIX_SAMPLES / 2;
+	sc->frame.datalen = SOFTMIX_DATALEN;
+	sc->frame.samples = SOFTMIX_SAMPLES;
 
 	/* Can't forget to record our pvt structure within the bridged channel structure */
 	bridge_channel->bridge_pvt = sc;
@@ -191,7 +194,7 @@
 
 	while (!bridge->stop && !bridge->refresh && bridge->array_num) {
 		struct ast_bridge_channel *bridge_channel = NULL;
-		short buf[SOFTMIX_SAMPLES] = {0, };
+		short buf[SOFTMIX_DATALEN] = {0, };
 		int timeout = -1;
 
 		/* Go through pulling audio from each factory that has it available */
@@ -206,7 +209,7 @@
 				int i;
 
 				/* Put into the local final buffer */
-				for (i = 0, data1 = buf, data2 = sc->our_buf; i < SOFTMIX_SAMPLES; i++, data1++, data2++)
+				for (i = 0, data1 = buf, data2 = sc->our_buf; i < SOFTMIX_DATALEN; i++, data1++, data2++)
 					ast_slinear_saturated_add(data1, data2);
 				/* Yay we have our own audio */
 				sc->have_audio = 1;
@@ -223,7 +226,7 @@
 			int i = 0;
 			
 			/* Copy from local final buffer to our final buffer while subtracting our audio if present */
-			for (i = 0; i < SOFTMIX_SAMPLES; i++) {
+			for (i = 0; i < SOFTMIX_DATALEN; i++) {
 				sc->final_buf[i] = buf[i];
 				/* Subtract our own audio from the end frame if present */
 				if (sc->have_audio)

Modified: team/file/bridging/channels/chan_bridge.c
URL: http://svn.digium.com/view/asterisk/team/file/bridging/channels/chan_bridge.c?view=diff&rev=167020&r1=167019&r2=167020
==============================================================================
--- team/file/bridging/channels/chan_bridge.c (original)
+++ team/file/bridging/channels/chan_bridge.c Wed Dec 31 15:43:17 2008
@@ -52,6 +52,7 @@
 static int bridge_hangup(struct ast_channel *ast);
 static struct ast_frame *bridge_read(struct ast_channel *ast);
 static int bridge_write(struct ast_channel *ast, struct ast_frame *f);
+static struct ast_channel *bridge_bridgedchannel(struct ast_channel *chan, struct ast_channel *bridge);
 
 static const struct ast_channel_tech bridge_tech = {
 	.type = "Bridge",
@@ -64,6 +65,7 @@
 	.write = bridge_write,
 	.write_video = bridge_write,
 	.exception = bridge_read,
+	.bridged_channel = bridge_bridgedchannel,
 };
 
 struct bridge_pvt {
@@ -72,6 +74,13 @@
 	struct ast_channel *output; /*!< Output channel - talking to bridge */
 };
 
+/*! \brief Called when the user of this channel wants to get the actual channel in the bridge */
+static struct ast_channel *bridge_bridgedchannel(struct ast_channel *chan, struct ast_channel *bridge)
+{
+	struct bridge_pvt *p = chan->tech_pvt;
+	return (chan == p->input) ? p->output : bridge;
+}
+
 /*! \brief Called when a frame should be read from the channel */
 static struct ast_frame  *bridge_read(struct ast_channel *ast)
 {
@@ -82,12 +91,28 @@
 static int bridge_write(struct ast_channel *ast, struct ast_frame *f)
 {
 	struct bridge_pvt *p = ast->tech_pvt;
-
+	struct ast_channel *other;
+	
+	ast_mutex_lock(&p->lock);
+
+	other = (p->input == ast ? p->output : p->input);
+
+	while (other && ast_channel_trylock(other)) {
+		ast_mutex_unlock(&p->lock);
+		do {
+			CHANNEL_DEADLOCK_AVOIDANCE(ast);
+		} while (ast_mutex_trylock(&p->lock));
+		other = (p->input == ast ? p->output : p->input);
+	}
+	
 	/* We basically queue the frame up on the other channel if present */
-	if (p->input && p->output) {
-		ast_queue_frame((p->input == ast ? p->output : p->input), f);
-	}
-
+	if (other) {
+		ast_queue_frame(other, f);
+		ast_channel_unlock(other);
+	}
+
+	ast_mutex_unlock(&p->lock);
+	
 	return 0;
 }
 
@@ -107,6 +132,28 @@
 	return 0;
 }
 
+/*! \brief Helper function to not deadlock when queueing the hangup frame */
+static void bridge_queue_hangup(struct bridge_pvt *p, struct ast_channel *us)
+{
+	struct ast_channel *other = (p->input == us ? p->output : p->input);
+
+	while (other && ast_channel_trylock(other)) {
+		ast_mutex_unlock(&p->lock);
+		do {
+			CHANNEL_DEADLOCK_AVOIDANCE(us);
+		} while (ast_mutex_trylock(&p->lock));
+		other = (p->input == us ? p->output : p->input);
+	}
+
+	/* We basically queue the frame up on the other channel if present */
+	if (other) {
+		ast_queue_hangup(other);
+		ast_channel_unlock(other);
+	}
+	
+	return;
+}
+
 /*! \brief Called when a channel should be hung up */
 static int bridge_hangup(struct ast_channel *ast)
 {
@@ -116,15 +163,15 @@
 
 	/* Figure out which channel this is... and set it to NULL as it has gone, but also queue up a hangup frame. */
 	if (p->input == ast) {
+		if (p->output) {
+			bridge_queue_hangup(p, ast);
+		}
 		p->input = NULL;
-		if (p->output) {
-			ast_queue_hangup(p->output);
+	} else if (p->output == ast) {
+		if (p->input) {
+			bridge_queue_hangup(p, ast);
 		}
-	} else if (p->output == ast) {
 		p->output = NULL;
-		if (p->input) {
-			ast_queue_hangup(p->input);
-		}
 	}
 
 	/* Deal with the Asterisk portion of it */
@@ -160,6 +207,7 @@
 	if (!(p->output = ast_channel_alloc(1, AST_STATE_UP, 0, 0, "", "", "", 0, "Bridge/%p-output", p))) {
 		ast_channel_free(p->input);
 		ast_free(p);
+		return NULL;
 	}
 
 	/* Setup the lock on the pvt structure, we will need that */
@@ -188,7 +236,7 @@
 	return AST_MODULE_LOAD_SUCCESS;
 }
 
-/*! \brief Unload the local proxy channel from Asterisk */
+/*! \brief Unload the bridge interaction channel from Asterisk */
 static int unload_module(void)
 {
 	ast_channel_unregister(&bridge_tech);




More information about the asterisk-commits mailing list