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

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Jan 31 13:59:37 CST 2013


Author: rmudgett
Date: Thu Jan 31 13:59:33 2013
New Revision: 380697

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=380697
Log:
Move BUGBUG comments here for easier merging later.

Modified:
    team/group/bridge_construction/apps/app_dial.c
    team/group/bridge_construction/apps/app_queue.c
    team/group/bridge_construction/bridges/bridge_builtin_features.c
    team/group/bridge_construction/bridges/bridge_multiplexed.c
    team/group/bridge_construction/bridges/bridge_simple.c
    team/group/bridge_construction/include/asterisk/bridging_features.h
    team/group/bridge_construction/main/bridging.c
    team/group/bridge_construction/main/channel.c
    team/group/bridge_construction/main/channel_internal_api.c
    team/group/bridge_construction/main/features.c

Modified: team/group/bridge_construction/apps/app_dial.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/apps/app_dial.c?view=diff&rev=380697&r1=380696&r2=380697
==============================================================================
--- team/group/bridge_construction/apps/app_dial.c (original)
+++ team/group/bridge_construction/apps/app_dial.c Thu Jan 31 13:59:33 2013
@@ -3044,6 +3044,7 @@
 
 				ast_channel_setoption(chan, AST_OPTION_OPRMODE, &oprmode, sizeof(oprmode), 0);
 			}
+/* BUGBUG need to determine where peer is going to execute on bridge completion. */
 			res = ast_bridge_call(chan, peer, &config);
 		}
 

Modified: team/group/bridge_construction/apps/app_queue.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/apps/app_queue.c?view=diff&rev=380697&r1=380696&r2=380697
==============================================================================
--- team/group/bridge_construction/apps/app_queue.c (original)
+++ team/group/bridge_construction/apps/app_queue.c Thu Jan 31 13:59:33 2013
@@ -5890,6 +5890,7 @@
 
 		time(&callstart);
 		transfer_ds = setup_transfer_datastore(qe, member, callstart, callcompletedinsl);
+/* BUGBUG need to determine where peer is going to execute on bridge completion. */
 		bridge = ast_bridge_call(qe->chan, peer, &bridge_config);
 
 		/* If the queue member did an attended transfer, then the TRANSFER already was logged in the queue_log

Modified: team/group/bridge_construction/bridges/bridge_builtin_features.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/bridges/bridge_builtin_features.c?view=diff&rev=380697&r1=380696&r2=380697
==============================================================================
--- team/group/bridge_construction/bridges/bridge_builtin_features.c (original)
+++ team/group/bridge_construction/bridges/bridge_builtin_features.c Thu Jan 31 13:59:33 2013
@@ -72,6 +72,7 @@
 	}
 
 	/* Drop to dialtone so they can enter the extension they want to transfer to */
+/* BUGBUG the timeout needs to be configurable from features.conf. */
 	res = ast_app_dtget(chan, context, exten, exten_len, exten_len - 1, 3000);
 	if (res < 0) {
 		/* Hangup or error */
@@ -101,6 +102,11 @@
 	int cause;
 
 	/* Fill the variable with the extension and context we want to call */
+/* BUGBUG if local channel optimization is using masquerades then this needs /n so the destination keeps its DTMF features.
+ * Or use /n to keep the peer channel stable until after the atxfer completes and remove the /n from the channel.
+ *
+ * Local channel optimization currently is disabled because I don't set the chan->bridge pointers.
+ */
 	snprintf(destination, sizeof(destination), "%s@%s", exten, context);
 
 	/* Now we request that chan_local prepare to call the destination */
@@ -165,6 +171,7 @@
 	struct ast_bridge_features_blind_transfer *blind_transfer = hook_pvt;
 	const char *context;
 
+/* BUGBUG the peer needs to be put on hold for the transfer. */
 	ast_channel_lock(bridge_channel->chan);
 	context = ast_strdupa(get_transfer_context(bridge_channel->chan,
 		blind_transfer ? blind_transfer->context : NULL));
@@ -174,6 +181,10 @@
 	if (grab_transfer(bridge_channel->chan, exten, sizeof(exten), context)) {
 		return 0;
 	}
+
+/* BUGBUG just need to ast_async_goto the peer so this bridge will go away and not accumulate local channels and bridges if the destination is to an application. */
+/* ast_async_goto actually is a blind transfer. */
+/* BUGBUG Use the bridge count to determine if can do DTMF transfer features.  If count is not 2 then don't allow it. */
 
 	/* Get a channel that is the destination we wish to call */
 	chan = dial_transfer(bridge_channel->chan, exten, context);
@@ -214,6 +225,7 @@
 	struct ast_bridge_features_attended_transfer *attended_transfer = hook_pvt;
 	const char *context;
 
+/* BUGBUG the peer needs to be put on hold for the transfer. */
 	ast_channel_lock(bridge_channel->chan);
 	context = ast_strdupa(get_transfer_context(bridge_channel->chan,
 		attended_transfer ? attended_transfer->context : NULL));
@@ -230,17 +242,21 @@
 		ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_NONE);
 		return 0;
 	}
+
+/* BUGBUG we need to wait for Party C (peer) to answer before dumping into the transient B-C bridge. */
 
 	/* Create a bridge to use to talk to the person we are calling */
 	attended_bridge = ast_bridge_new(AST_BRIDGE_CAPABILITY_1TO1MIX | AST_BRIDGE_CAPABILITY_NATIVE,
 		AST_BRIDGE_FLAG_DISSOLVE_HANGUP);
 	if (!attended_bridge) {
 		ast_hangup(peer);
+/* BUGBUG beeperr needs to be configurable from features.conf */
 		ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_NONE);
 		return 0;
 	}
 
 	/* This is how this is going down, we are imparting the channel we called above into this bridge first */
+/* BUGBUG we should impart the peer as an independent and move it to the original bridge. */
 	if (ast_bridge_impart(attended_bridge, peer, NULL, NULL, 0)) {
 		ast_bridge_destroy(attended_bridge);
 		ast_hangup(peer);
@@ -250,6 +266,9 @@
 
 	/* Before we join setup a features structure with the hangup option, just in case they want to use DTMF */
 	ast_bridge_features_init(&caller_features);
+/* BUGBUG bridging API features does not support features.conf featuremap */
+/* BUGBUG bridging API features does not support the features.conf atxfer bounce between C & B channels */
+/* BUGBUG The atxfer feature hooks need to be passed a pointer to where to mark which hook happened.  Rather than relying on the bridge join return value. */
 	ast_bridge_features_enable(&caller_features, AST_BRIDGE_BUILTIN_HANGUP,
 		attended_transfer && !ast_strlen_zero(attended_transfer->complete)
 			? attended_transfer->complete : "*1",

Modified: team/group/bridge_construction/bridges/bridge_multiplexed.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/bridges/bridge_multiplexed.c?view=diff&rev=380697&r1=380696&r2=380697
==============================================================================
--- team/group/bridge_construction/bridges/bridge_multiplexed.c (original)
+++ team/group/bridge_construction/bridges/bridge_multiplexed.c Thu Jan 31 13:59:33 2013
@@ -57,6 +57,7 @@
 struct multiplexed_thread {
 	/*! Thread itself */
 	pthread_t thread;
+/* BUGBUG this is only large enough for the supported number of bridge channel pairs.  It is not large enough to handle transient channels being swapped. */
 	/*! Channels serviced by this thread */
 	struct ast_channel *chans[2 * MULTIPLEXED_MAX_BRIDGES];
 	/*! Pipe used to wake up the multiplexed thread */
@@ -467,6 +468,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_write(other->chan, 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=380697&r1=380696&r2=380697
==============================================================================
--- team/group/bridge_construction/bridges/bridge_simple.c (original)
+++ team/group/bridge_construction/bridges/bridge_simple.c Thu Jan 31 13:59:33 2013
@@ -82,6 +82,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_write(other->chan, frame);
 		}

Modified: team/group/bridge_construction/include/asterisk/bridging_features.h
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/include/asterisk/bridging_features.h?view=diff&rev=380697&r1=380696&r2=380697
==============================================================================
--- team/group/bridge_construction/include/asterisk/bridging_features.h (original)
+++ team/group/bridge_construction/include/asterisk/bridging_features.h Thu Jan 31 13:59:33 2013
@@ -61,6 +61,7 @@
 	 * parking slot to which it was parked.
 	 */
 	AST_BRIDGE_BUILTIN_PARKCALL,
+/* BUGBUG does Monitor and/or MixMonitor require a two party bridge?  MixMonitor is used by ConfBridge so maybe it doesn't. */
 	/*!
 	 * DTMF one-touch-record toggle using Monitor app.
 	 *
@@ -157,6 +158,7 @@
 	unsigned int usable:1;
 	/*! Bit to indicate whether the channel/bridge is muted or not */
 	unsigned int mute:1;
+/* BUGBUG why is dtmf_passthrough not a feature_flags bit? */
 	/*! Bit to indicate whether DTMF should be passed into the bridge tech or not.  */
 	unsigned int dtmf_passthrough:1;
 
@@ -166,6 +168,7 @@
  * \brief Structure that contains configuration information for the blind transfer built in feature
  */
 struct ast_bridge_features_blind_transfer {
+/* BUGBUG the context should be figured out based upon TRANSFER_CONTEXT channel variable of A/B or current context of A/B. More appropriate for when channel moved to other bridges. */
 	/*! Context to use for transfers */
 	char context[AST_MAX_CONTEXT];
 };
@@ -174,6 +177,7 @@
  * \brief Structure that contains configuration information for the attended transfer built in feature
  */
 struct ast_bridge_features_attended_transfer {
+/* BUGBUG the context should be figured out based upon TRANSFER_CONTEXT channel variable of A/B or current context of A/B. More appropriate for when channel moved to other bridges. */
 	/*! Context to use for transfers */
 	char context[AST_MAX_CONTEXT];
 	/*! DTMF string used to turn the transfer into a three way conference */

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=380697&r1=380696&r2=380697
==============================================================================
--- team/group/bridge_construction/main/bridging.c (original)
+++ team/group/bridge_construction/main/bridging.c Thu Jan 31 13:59:33 2013
@@ -346,6 +346,7 @@
 		return frame;
 	}
 
+/* BUGBUG the feature hook matching needs to be done here.  Any matching feature hook needs to be queued onto the bridge_channel.  Also the feature hook digit timeout needs to be handled. */
 	/* See if this DTMF matches the beginnings of any feature hooks, if so we switch to the feature state to either execute the feature or collect more DTMF */
 	AST_LIST_TRAVERSE(&features->hooks, hook, entry) {
 		if (hook->dtmf[0] == frame->subclass.integer) {
@@ -367,6 +368,7 @@
 /*! \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. */
 	switch (subclass) {
 	case AST_CONTROL_READ_ACTION:
 	case AST_CONTROL_CC:
@@ -436,6 +438,13 @@
 				bridge->technology->write(bridge, bridge_channel, frame);
 			}
 		} else {
+/* 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 we assume that all parties need to be already answered when bridged? */
+/* 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. */
+/* BUGBUG could make a queue of things the bridge_channel thread needs to handle in case it gets behind on processing because of the interception macros. */
 			/* Simply write the frame out to the bridge technology if it still exists */
 			bridge->technology->write(bridge, bridge_channel, frame);
 		}
@@ -691,10 +700,12 @@
 	ao2_lock(bridge);
 
 	if (bridge->callid) {
+/* BUGBUG the bridge callid needs to be verified. */
 		bridge->callid = ast_callid_unref(bridge->callid);
 	}
 
 	if (bridge->thread != AST_PTHREADT_NULL) {
+/* BUGBUG this needs to be moved to the last bridge_channel removal code if the bridge flag AST_BRIDGE_FLAG_DISSOLVE_EMPTY. */
 		bridge_stop(bridge);
 	}
 
@@ -1021,6 +1032,7 @@
 			break;
 		}
 
+/* BUGBUG need to record the duration of DTMF digits so when the string is played back, they are reproduced. */
 		/* Add the above DTMF into the DTMF string so we can do our matching */
 		dtmf[dtmf_len++] = res;
 
@@ -1445,6 +1457,9 @@
 
 	ao2_ref(bridge_channel, -1);
 
+/* BUGBUG need to run a PBX on this channel or hangup. */
+/* BUGBUG need to start the PBX at the appropriate location. */
+/* BUGBUG need to determine where to execute in the dialplan. */
 	switch (state) {
 	case AST_BRIDGE_CHANNEL_STATE_DEPART:
 		ast_log(LOG_ERROR, "Independently imparted channel was departed: %s\n",
@@ -1667,6 +1682,8 @@
 int ast_bridge_suspend(struct ast_bridge *bridge, struct ast_channel *chan)
 {
 	struct ast_bridge_channel *bridge_channel;
+/* BUGBUG the case of a disolved bridge while channel is suspended is not handled. */
+/* BUGBUG suspend/unsuspend needs to be rethought. The caller must block until it has successfully suspended the channel for temporary control. */
 
 	ao2_lock(bridge);
 
@@ -1685,6 +1702,7 @@
 int ast_bridge_unsuspend(struct ast_bridge *bridge, struct ast_channel *chan)
 {
 	struct ast_bridge_channel *bridge_channel;
+/* BUGBUG the case of a disolved bridge while channel is suspended is not handled. */
 
 	ao2_lock(bridge);
 
@@ -1776,6 +1794,7 @@
 
 int ast_bridge_features_enable(struct ast_bridge_features *features, enum ast_bridge_builtin_feature feature, const char *dtmf, void *config)
 {
+/* BUGBUG a destructor for config is needed if it is going to be non-NULL */
 	if (ARRAY_LEN(builtin_features_handlers) <= feature
 		|| !builtin_features_handlers[feature]) {
 		return -1;

Modified: team/group/bridge_construction/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/main/channel.c?view=diff&rev=380697&r1=380696&r2=380697
==============================================================================
--- team/group/bridge_construction/main/channel.c (original)
+++ team/group/bridge_construction/main/channel.c Thu Jan 31 13:59:33 2013
@@ -7856,6 +7856,7 @@
 	}
 }
 
+/* BUGBUG ast_channel_bridge() and anything that only it calls will be removed. */
 /*! \brief Bridge two channels together */
 enum ast_bridge_result ast_channel_bridge(struct ast_channel *c0, struct ast_channel *c1,
 					  struct ast_bridge_config *config, struct ast_frame **fo, struct ast_channel **rc)

Modified: team/group/bridge_construction/main/channel_internal_api.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/main/channel_internal_api.c?view=diff&rev=380697&r1=380696&r2=380697
==============================================================================
--- team/group/bridge_construction/main/channel_internal_api.c (original)
+++ team/group/bridge_construction/main/channel_internal_api.c Thu Jan 31 13:59:33 2013
@@ -62,6 +62,7 @@
 	void *music_state;				/*!< Music State*/
 	void *generatordata;				/*!< Current generator data if there is any */
 	struct ast_generator *generator;		/*!< Current active data generator */
+/* BUGBUG bridged_channel must be eliminated from ast_channel */
 	struct ast_channel * bridged_channel;			/*!< Who are we bridged to, if we're bridged.
 							 *   Who is proxying for us, if we are proxied (i.e. chan_agent).
 							 *   Do not access directly, use ast_bridged_channel(chan) */
@@ -185,6 +186,7 @@
 
 	unsigned short transfercapability;		/*!< ISDN Transfer Capability - AST_FLAG_DIGITAL is not enough */
 
+/* BUGBUG the bridge pointer must change to an ast_channel_bridge pointer because it will never change while the channel is in the bridging system whereas the bridge could change. */
 	struct ast_bridge *bridge;                      /*!< Bridge this channel is participating in */
 	struct ast_timer *timer;			/*!< timer object that provided timingfd */
 

Modified: team/group/bridge_construction/main/features.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/main/features.c?view=diff&rev=380697&r1=380696&r2=380697
==============================================================================
--- team/group/bridge_construction/main/features.c (original)
+++ team/group/bridge_construction/main/features.c Thu Jan 31 13:59:33 2013
@@ -8264,6 +8264,7 @@
 	 * continue in the dialplan.
 	 */
 	ast_set_flag(ast_channel_flags(chan), AST_FLAG_BRIDGE_HANGUP_DONT);
+/* BUGBUG need to determine where peer is going to execute on bridge completion. */
 	ast_bridge_call(chan, final_dest_chan, &bconfig);
 
 	/* The bridge has ended, set BRIDGERESULT to SUCCESS. */




More information about the asterisk-commits mailing list