[asterisk-commits] rmudgett: branch group/bridge_construction r384161 - in /team/group/bridge_co...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Mar 27 14:50:14 CDT 2013


Author: rmudgett
Date: Wed Mar 27 14:50:11 2013
New Revision: 384161

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=384161
Log:
Bridge API Enhancements - add control frame support/processing

* Refactored softmix_bridge_write().  Handling of control frames in the
softmix technology is deferred until there is a need for it.

(closes issue ASTERISK-20874)
Reported by: Matt Jordan

Modified:
    team/group/bridge_construction/bridges/bridge_holding.c
    team/group/bridge_construction/bridges/bridge_simple.c
    team/group/bridge_construction/bridges/bridge_softmix.c
    team/group/bridge_construction/include/asterisk/bridging_technology.h
    team/group/bridge_construction/main/abstract_jb.c
    team/group/bridge_construction/main/bridging.c

Modified: team/group/bridge_construction/bridges/bridge_holding.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/bridges/bridge_holding.c?view=diff&rev=384161&r1=384160&r2=384161
==============================================================================
--- team/group/bridge_construction/bridges/bridge_holding.c (original)
+++ team/group/bridge_construction/bridges/bridge_holding.c Wed Mar 27 14:50:11 2013
@@ -278,11 +278,7 @@
 		}
 
 		if (other->state == AST_BRIDGE_CHANNEL_STATE_WAIT) {
-/* BUGBUG need to handle control frames in a bridge tech specific way here.  Mostly just queue action to bridge channel. */
-			if (!other->suspended
-				|| ast_is_deferrable_frame(frame)) {
-				ast_bridge_channel_queue_frame(other, frame);
-			}
+			ast_bridge_channel_queue_frame(other, frame);
 		}
 	}
 

Modified: team/group/bridge_construction/bridges/bridge_simple.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/bridges/bridge_simple.c?view=diff&rev=384161&r1=384160&r2=384161
==============================================================================
--- team/group/bridge_construction/bridges/bridge_simple.c (original)
+++ team/group/bridge_construction/bridges/bridge_simple.c Wed Mar 27 14:50:11 2013
@@ -83,11 +83,7 @@
 
 	/* The bridging core takes care of freeing the passed in frame. */
 	if (other->state == AST_BRIDGE_CHANNEL_STATE_WAIT) {
-/* BUGBUG need to handle control frames in a bridge tech specific way here.  Mostly just queue action to bridge channel. */
-		if (!other->suspended
-			|| ast_is_deferrable_frame(frame)) {
-			ast_bridge_channel_queue_frame(other, frame);
-		}
+		ast_bridge_channel_queue_frame(other, frame);
 	}
 
 	return 0;

Modified: team/group/bridge_construction/bridges/bridge_softmix.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/bridges/bridge_softmix.c?view=diff&rev=384161&r1=384160&r2=384161
==============================================================================
--- team/group/bridge_construction/bridges/bridge_softmix.c (original)
+++ team/group/bridge_construction/bridges/bridge_softmix.c Wed Mar 27 14:50:11 2013
@@ -468,8 +468,64 @@
 	}
 }
 
-/*! \brief Function called when a channel writes a frame into the bridge */
-static int softmix_bridge_write(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
+/*!
+ * \internal
+ * \brief Determine what to do with a video frame.
+ * \since 12.0.0
+ *
+ * \param bridge Which bridge is getting the frame
+ * \param bridge_channel Which channel is writing the frame.
+ * \param frame What is being written.
+ *
+ * \return Nothing
+ */
+static void softmix_bridge_write_video(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
+{
+	struct softmix_channel *sc = bridge_channel->tech_pvt;
+	int num_src;
+	int video_src_priority;
+
+	num_src = ast_bridge_number_video_src(bridge);
+	video_src_priority = ast_bridge_is_video_src(bridge, bridge_channel->chan);
+
+	/* Determine if the video frame should be distributed or not */
+	switch (bridge->video_mode.mode) {
+	case AST_BRIDGE_VIDEO_MODE_NONE:
+		break;
+	case AST_BRIDGE_VIDEO_MODE_SINGLE_SRC:
+		if (video_src_priority == 1) {
+			softmix_pass_video_all(bridge, bridge_channel, frame, 1);
+		}
+		break;
+	case AST_BRIDGE_VIDEO_MODE_TALKER_SRC:
+		ast_mutex_lock(&sc->lock);
+		ast_bridge_update_talker_src_video_mode(bridge, bridge_channel->chan,
+			sc->video_talker.energy_average,
+			ast_format_get_video_mark(&frame->subclass.format));
+		ast_mutex_unlock(&sc->lock);
+		if (video_src_priority == 1) {
+			int echo = num_src > 1 ? 0 : 1;
+
+			softmix_pass_video_all(bridge, bridge_channel, frame, echo);
+		} else if (video_src_priority == 2) {
+			softmix_pass_video_top_priority(bridge, frame);
+		}
+		break;
+	}
+}
+
+/*!
+ * \internal
+ * \brief Determine what to do with a voice frame.
+ * \since 12.0.0
+ *
+ * \param bridge Which bridge is getting the frame
+ * \param bridge_channel Which channel is writing the frame.
+ * \param frame What is being written.
+ *
+ * \return Nothing
+ */
+static void softmix_bridge_write_voice(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
 {
 	struct softmix_channel *sc = bridge_channel->tech_pvt;
 	struct softmix_bridge_data *softmix_data = bridge->tech_pvt;
@@ -479,49 +535,8 @@
 		bridge_channel->tech_args.silence_threshold :
 		DEFAULT_SOFTMIX_SILENCE_THRESHOLD;
 	char update_talking = -1;  /* if this is set to 0 or 1, tell the bridge that the channel has started or stopped talking. */
-	int res = 0;
-
-	/* Only accept audio frames, all others are unsupported */
-	if (frame->frametype == AST_FRAME_DTMF_END || frame->frametype == AST_FRAME_DTMF_BEGIN) {
-		softmix_pass_dtmf(bridge, bridge_channel, frame);
-		return res;
-	} else if (frame->frametype != AST_FRAME_VOICE && frame->frametype != AST_FRAME_VIDEO) {
-		/* Frame type unsupported. */
-		res = -1;
-		return res;
-	} else if (frame->datalen == 0) {
-		return res;
-	}
-
-	/* Determine if this video frame should be distributed or not */
-	if (frame->frametype == AST_FRAME_VIDEO) {
-		int num_src = ast_bridge_number_video_src(bridge);
-		int video_src_priority = ast_bridge_is_video_src(bridge, bridge_channel->chan);
-
-		switch (bridge->video_mode.mode) {
-		case AST_BRIDGE_VIDEO_MODE_NONE:
-			break;
-		case AST_BRIDGE_VIDEO_MODE_SINGLE_SRC:
-			if (video_src_priority == 1) {
-				softmix_pass_video_all(bridge, bridge_channel, frame, 1);
-			}
-			break;
-		case AST_BRIDGE_VIDEO_MODE_TALKER_SRC:
-			ast_mutex_lock(&sc->lock);
-			ast_bridge_update_talker_src_video_mode(bridge, bridge_channel->chan, sc->video_talker.energy_average, ast_format_get_video_mark(&frame->subclass.format));
-			ast_mutex_unlock(&sc->lock);
-			if (video_src_priority == 1) {
-				int echo = num_src > 1 ? 0 : 1;
-				softmix_pass_video_all(bridge, bridge_channel, frame, echo);
-			} else if (video_src_priority == 2) {
-				softmix_pass_video_top_priority(bridge, frame);
-			}
-			break;
-		}
-		return res;
-	}
-
-	/* If we made it here, we are going to write the frame into the conference */
+
+	/* Write the frame into the conference */
 	ast_mutex_lock(&sc->lock);
 	ast_dsp_silence_with_energy(sc->dsp, frame, &totalsilence, &cur_energy);
 
@@ -569,6 +584,62 @@
 
 	if (update_talking != -1) {
 		ast_bridge_notify_talking(bridge_channel, update_talking);
+	}
+}
+
+/*!
+ * \internal
+ * \brief Determine what to do with a control frame.
+ * \since 12.0.0
+ *
+ * \param bridge Which bridge is getting the frame
+ * \param bridge_channel Which channel is writing the frame.
+ * \param frame What is being written.
+ *
+ * \return Nothing
+ */
+static void softmix_bridge_write_control(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
+{
+/* BUGBUG need to look at channel roles to determine what to do with control frame. */
+	/*! \todo BUGBUG softmix_bridge_write_control() not written */
+}
+
+/*!
+ * \internal
+ * \brief Determine what to do with a frame written into the bridge.
+ * \since 12.0.0
+ *
+ * \param bridge Which bridge is getting the frame
+ * \param bridge_channel Which channel is writing the frame.
+ * \param frame What is being written.
+ *
+ * \retval 0 on success
+ * \retval -1 on failure
+ *
+ * \note On entry, bridge is already locked.
+ */
+static int softmix_bridge_write(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
+{
+	int res = 0;
+
+	switch (frame->frametype) {
+	case AST_FRAME_DTMF_BEGIN:
+	case AST_FRAME_DTMF_END:
+		softmix_pass_dtmf(bridge, bridge_channel, frame);
+		break;
+	case AST_FRAME_VOICE:
+		softmix_bridge_write_voice(bridge, bridge_channel, frame);
+		break;
+	case AST_FRAME_VIDEO:
+		softmix_bridge_write_video(bridge, bridge_channel, frame);
+		break;
+	case AST_FRAME_CONTROL:
+		softmix_bridge_write_control(bridge, bridge_channel, frame);
+		break;
+	default:
+		ast_debug(3, "Frame type %d unsupported\n", frame->frametype);
+		res = -1;
+		break;
 	}
 
 	return res;

Modified: team/group/bridge_construction/include/asterisk/bridging_technology.h
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/include/asterisk/bridging_technology.h?view=diff&rev=384161&r1=384160&r2=384161
==============================================================================
--- team/group/bridge_construction/include/asterisk/bridging_technology.h (original)
+++ team/group/bridge_construction/include/asterisk/bridging_technology.h Wed Mar 27 14:50:11 2013
@@ -83,6 +83,8 @@
 	 *
 	 * \retval 0 on success
 	 * \retval -1 on failure
+	 *
+	 * \note On entry, bridge is already locked.
 	 */
 	int (*write)(struct ast_bridge *bridge, struct ast_bridge_channel *bridged_channel, struct ast_frame *frame);
 	/*! Formats that the bridge technology supports */

Modified: team/group/bridge_construction/main/abstract_jb.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/main/abstract_jb.c?view=diff&rev=384161&r1=384160&r2=384161
==============================================================================
--- team/group/bridge_construction/main/abstract_jb.c (original)
+++ team/group/bridge_construction/main/abstract_jb.c Wed Mar 27 14:50:11 2013
@@ -570,6 +570,7 @@
 
 void ast_jb_configure(struct ast_channel *chan, const struct ast_jb_conf *conf)
 {
+/* BUGBUG convert this to use func_jitterbuffer then the channel jitterbuffer may be able to go away. */
 	memcpy(&ast_channel_jb(chan)->conf, conf, sizeof(*conf));
 }
 

Modified: team/group/bridge_construction/main/bridging.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/main/bridging.c?view=diff&rev=384161&r1=384160&r2=384161
==============================================================================
--- team/group/bridge_construction/main/bridging.c (original)
+++ team/group/bridge_construction/main/bridging.c Wed Mar 27 14:50:11 2013
@@ -296,7 +296,14 @@
 	struct ast_frame *dup;
 	char nudge = 0;
 
-/* BUGBUG need to do something with media frames when channel is suspended. Likely just drop non-deferable frames. */
+	if (bridge_channel->suspended
+		/* Also defer DTMF frames. */
+		&& fr->frametype != AST_FRAME_DTMF_BEGIN
+		&& fr->frametype != AST_FRAME_DTMF_END
+		&& !ast_is_deferrable_frame(fr)) {
+		/* Drop non-deferable frames when suspended. */
+		return 0;
+	}
 
 	dup = ast_frdup(fr);
 	if (!dup) {
@@ -386,6 +393,8 @@
 	ast_debug(1, "Pulling bridge channel %p(%s) from bridge %p\n",
 		bridge_channel, ast_channel_name(bridge_channel->chan), bridge);
 
+/* BUGBUG This is where incoming HOLD/UNHOLD memory should write UNHOLD into bridge. (if not local optimizing) */
+/* BUGBUG This is where incoming DTMF begin/end memory should write DTMF end into bridge. (if not local optimizing) */
 	if (!bridge_channel->just_joined) {
 		/* Tell the bridge technology we are leaving so they tear us down */
 		ast_debug(1, "Giving bridge technology %s notification that %p(%s) is leaving bridge %p\n",
@@ -614,32 +623,6 @@
 	return ready;
 }
 
-/*! \brief Internal function used to determine whether a control frame should be dropped or not */
-static int bridge_drop_control_frame(int subclass)
-{
-/* BUGBUG I think this code should be removed. Let the bridging tech determine what to do with control frames. */
-#if 1
-	/* Block all control frames. */
-	return 1;
-#else
-	switch (subclass) {
-	case AST_CONTROL_READ_ACTION:
-	case AST_CONTROL_CC:
-	case AST_CONTROL_MCID:
-	case AST_CONTROL_AOC:
-	case AST_CONTROL_CONNECTED_LINE:
-	case AST_CONTROL_REDIRECTING:
-		return 1;
-
-	case AST_CONTROL_ANSWER:
-	case -1:
-		return 1;
-	default:
-		return 0;
-	}
-#endif
-}
-
 void ast_bridge_notify_talking(struct ast_bridge_channel *bridge_channel, int started_talking)
 {
 	struct ast_frame action = {
@@ -677,17 +660,14 @@
 		ast_frfree(frame);
 		return;
 	case AST_FRAME_CONTROL:
-		if (frame->subclass.integer == AST_CONTROL_HANGUP) {
+		switch (frame->subclass.integer) {
+		case AST_CONTROL_HANGUP:
 			bridge_handle_hangup(bridge_channel);
 			ast_frfree(frame);
 			return;
-		}
-		if (bridge_drop_control_frame(frame->subclass.integer)) {
-			ast_debug(1, "Dropping control frame %d from bridge channel %p(%s)\n",
-				frame->subclass.integer, bridge_channel,
-				ast_channel_name(bridge_channel->chan));
-			ast_frfree(frame);
-			return;
+/* BUGBUG This is where incoming HOLD/UNHOLD memory should register.  Write UNHOLD into bridge when this channel is pulled. */
+		default:
+			break;
 		}
 		break;
 	case AST_FRAME_DTMF_BEGIN:
@@ -701,19 +681,18 @@
 			ast_frfree(frame);
 			return;
 		}
+/* BUGBUG This is where incoming DTMF begin/end memory should register.  Write DTMF end into bridge when this channel is pulled. */
 		break;
 	default:
 		break;
 	}
 
-/* BUGBUG looks like the place to handle the control frame exchange between 1-1 bridge participants is the bridge tech write callback. */
-/* BUGBUG make a 1-1 bridge write handler for control frames. */
-/* BUGBUG make bridge_channel thread run the CONNECTED_LINE and REDIRECTING interception macros. */
-/* BUGBUG should make AST_CONTROL_ANSWER do an ast_indicate(-1) to the bridge peer if it is not UP as well as a connected line update. */
 /* BUGBUG bridge join or impart needs to do CONNECTED_LINE updates if the channels are being swapped and it is a 1-1 bridge. */
 
 	/* Simply write the frame out to the bridge technology. */
 	ast_bridge_channel_lock_bridge(bridge_channel);
+/* BUGBUG The tech is where AST_CONTROL_ANSWER hook should go. (early bridge) */
+/* BUGBUG The tech is where incoming BUSY/CONGESTION hangup should happen? (early bridge) */
 	bridge_channel->bridge->technology->write(bridge_channel->bridge, bridge_channel, frame);
 	ast_bridge_unlock(bridge_channel->bridge);
 	ast_frfree(frame);
@@ -1714,13 +1693,84 @@
  */
 static void bridge_channel_handle_control(struct ast_bridge_channel *bridge_channel, struct ast_frame *fr)
 {
+	struct ast_channel *chan;
+	struct ast_option_header *aoh;
+	int is_caller;
+	int intercept_failed;
+
+	chan = bridge_channel->chan;
 	switch (fr->subclass.integer) {
+	case AST_CONTROL_REDIRECTING:
+		is_caller = !ast_test_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING);
+		bridge_channel_suspend(bridge_channel);
+		intercept_failed = ast_channel_redirecting_sub(NULL, chan, fr, 1)
+			&& ast_channel_redirecting_macro(NULL, chan, fr, is_caller, 1);
+		bridge_channel_unsuspend(bridge_channel);
+		if (intercept_failed) {
+			ast_indicate_data(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
+		}
+		break;
 	case AST_CONTROL_CONNECTED_LINE:
+		is_caller = !ast_test_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING);
+		bridge_channel_suspend(bridge_channel);
+		intercept_failed = ast_channel_connected_line_sub(NULL, chan, fr, 1)
+			&& ast_channel_connected_line_macro(NULL, chan, fr, is_caller, 1);
+		bridge_channel_unsuspend(bridge_channel);
+		if (intercept_failed) {
+			ast_indicate_data(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
+		}
+		break;
+	case AST_CONTROL_HOLD:
+	case AST_CONTROL_UNHOLD:
+/*
+ * BUGBUG bridge_channels should remember sending/receiving an outstanding HOLD to/from the bridge
+ *
+ * When the sending channel is pulled from the bridge it needs to write into the bridge an UNHOLD before being pulled.
+ * When the receiving channel is pulled from the bridge it needs to generate its own UNHOLD.
+ * Something similar needs to be done for DTMF begin/end.
+ */
+	case AST_CONTROL_VIDUPDATE:
+	case AST_CONTROL_SRCUPDATE:
+	case AST_CONTROL_SRCCHANGE:
+	case AST_CONTROL_T38_PARAMETERS:
+/* BUGBUG may have to do something with a jitter buffer for these. */
+		ast_indicate_data(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
+		break;
+	case AST_CONTROL_OPTION:
+		/*
+		 * Forward option Requests, but only ones we know are safe These
+		 * are ONLY sent by chan_iax2 and I'm not convinced that they
+		 * are useful. I haven't deleted them entirely because I just am
+		 * not sure of the ramifications of removing them.
+		 */
+		aoh = fr->data.ptr;
+		if (aoh && aoh->flag == AST_OPTION_FLAG_REQUEST) {
+			switch (ntohs(aoh->option)) {
+			case AST_OPTION_TONE_VERIFY:
+			case AST_OPTION_TDD:
+			case AST_OPTION_RELAXDTMF:
+			case AST_OPTION_AUDIO_MODE:
+			case AST_OPTION_DIGIT_DETECT:
+			case AST_OPTION_FAX_DETECT:
+				ast_channel_setoption(chan, ntohs(aoh->option), aoh->data,
+					fr->datalen - sizeof(*aoh), 0);
+				break;
+			default:
+				break;
+			}
+		}
+		break;
+	case AST_CONTROL_ANSWER:
+		if (ast_channel_state(chan) != AST_STATE_UP) {
+			ast_answer(chan);
+		} else {
+			ast_indicate(chan, -1);
+		}
 		break;
 	default:
+		ast_indicate_data(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
 		break;
 	}
-	/*! \todo BUGBUG bridge_channel_handle_control() not written */
 }
 
 /*!
@@ -1901,6 +1951,7 @@
 	ast_bridge_channel_unlock(bridge_channel);
 
 /* BUGBUG Revisit in regards to moving channels between bridges and local channel optimization. */
+/* BUGBUG This is where outgoing HOLD/UNHOLD memory should write UNHOLD to channel. */
 	/* Complete any partial DTMF digit before exiting the bridge. */
 	if (ast_channel_sending_dtmf_digit(bridge_channel->chan)) {
 		ast_bridge_end_dtmf(bridge_channel->chan,




More information about the asterisk-commits mailing list