[asterisk-commits] rmudgett: trunk r396732 - in /trunk: include/asterisk/ main/
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Thu Aug 15 09:21:02 CDT 2013
Author: rmudgett
Date: Thu Aug 15 09:20:59 2013
New Revision: 396732
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=396732
Log:
Fix Bridge API DTMF hook matching for begin and end DTMF events.
The Bridge API DTMF hook matching would not deal with DTMF end events
only. It required a DTMF begin event to start matching the DTMF hooks.
There are many places in Asterisk where code only generates DTMF end
events without the corresponding begin event. One such place is the AMI
action Atxfer.
* Fixed DTMF hook matching if there is a string of DTMF frames in the read
queue. We could potentially miss some of them before.
* Fixed AMI Atxfer action documentation.
(closes issue ASTERISK-22037)
Reported by: Matt Jordan
Review: https://reviewboard.asterisk.org/r/2752/
Modified:
trunk/include/asterisk/bridge_channel_internal.h
trunk/main/bridge_channel.c
trunk/main/manager.c
Modified: trunk/include/asterisk/bridge_channel_internal.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/bridge_channel_internal.h?view=diff&rev=396732&r1=396731&r2=396732
==============================================================================
--- trunk/include/asterisk/bridge_channel_internal.h (original)
+++ trunk/include/asterisk/bridge_channel_internal.h Thu Aug 15 09:20:59 2013
@@ -39,8 +39,6 @@
* \brief Actions that can be taken on a channel in a bridge
*/
enum bridge_channel_action_type {
- /*! Bridged channel is to detect a feature hook */
- BRIDGE_CHANNEL_ACTION_FEATURE,
/*! Bridged channel is to send a DTMF stream out */
BRIDGE_CHANNEL_ACTION_DTMF_STREAM,
/*! Bridged channel is to indicate talking start */
Modified: trunk/main/bridge_channel.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/bridge_channel.c?view=diff&rev=396732&r1=396731&r2=396732
==============================================================================
--- trunk/main/bridge_channel.c (original)
+++ trunk/main/bridge_channel.c Thu Aug 15 09:20:59 2013
@@ -991,16 +991,17 @@
}
/*!
- * \internal \brief Internal function that executes a feature on a bridge channel
+ * \internal
+ * \brief Internal function that executes a feature on a bridge channel
* \note Neither the bridge nor the bridge_channel locks should be held when entering
* this function.
*/
-static void bridge_channel_feature(struct ast_bridge_channel *bridge_channel)
+static void bridge_channel_feature(struct ast_bridge_channel *bridge_channel, const char *starting_dtmf)
{
struct ast_bridge_features *features = bridge_channel->features;
struct ast_bridge_hook_dtmf *hook = NULL;
- char dtmf[MAXIMUM_DTMF_FEATURE_STRING] = "";
- size_t dtmf_len = 0;
+ char dtmf[MAXIMUM_DTMF_FEATURE_STRING];
+ size_t dtmf_len;
unsigned int digit_timeout;
RAII_VAR(struct ast_features_general_config *, gen_cfg, NULL, ao2_cleanup);
@@ -1014,14 +1015,46 @@
digit_timeout = gen_cfg->featuredigittimeout;
ast_channel_unlock(bridge_channel->chan);
- /* The channel is now under our control and we don't really want any begin frames to do our DTMF matching so disable 'em at the core level */
- ast_set_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_END_DTMF_ONLY);
-
- /* Wait for DTMF on the channel and put it into a buffer. If the buffer matches any feature hook execute the hook. */
- do {
+ if (ast_strlen_zero(starting_dtmf)) {
+ dtmf[0] = '\0';
+ dtmf_len = 0;
+ } else {
+ ast_copy_string(dtmf, starting_dtmf, sizeof(dtmf));
+ dtmf_len = strlen(dtmf);
+ }
+
+ /*
+ * Check if any feature DTMF hooks match or could match and
+ * try to collect more DTMF digits.
+ */
+ for (;;) {
int res;
- /* If the above timed out simply exit */
+ if (dtmf_len) {
+ ast_debug(1, "DTMF feature string on %p(%s) is now '%s'\n",
+ bridge_channel, ast_channel_name(bridge_channel->chan), dtmf);
+
+ /* See if a DTMF feature hook matches or can match */
+ hook = ao2_find(features->dtmf_hooks, dtmf, OBJ_PARTIAL_KEY);
+ if (!hook) {
+ ast_debug(1, "No DTMF feature hooks on %p(%s) match '%s'\n",
+ bridge_channel, ast_channel_name(bridge_channel->chan), dtmf);
+ break;
+ }
+ if (strlen(hook->dtmf.code) == dtmf_len) {
+ ast_debug(1, "DTMF feature hook %p matched DTMF string '%s' on %p(%s)\n",
+ hook, dtmf, bridge_channel, ast_channel_name(bridge_channel->chan));
+ break;
+ }
+ ao2_ref(hook, -1);
+ hook = NULL;
+
+ if (ARRAY_LEN(dtmf) - 1 <= dtmf_len) {
+ /* We have reached the maximum length of a DTMF feature string. */
+ break;
+ }
+ }
+
res = ast_waitfordigit(bridge_channel->chan, digit_timeout);
if (!res) {
ast_debug(1, "DTMF feature string collection on %p(%s) timed out\n",
@@ -1034,37 +1067,15 @@
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;
- ast_debug(1, "DTMF feature string on %p(%s) is now '%s'\n",
- bridge_channel, ast_channel_name(bridge_channel->chan), dtmf);
-
- /* See if a DTMF feature hook matches or can match */
- hook = ao2_find(features->dtmf_hooks, dtmf, OBJ_PARTIAL_KEY);
- if (!hook) {
- ast_debug(1, "No DTMF feature hooks on %p(%s) match '%s'\n",
- bridge_channel, ast_channel_name(bridge_channel->chan), dtmf);
- break;
- }
- if (strlen(hook->dtmf.code) == dtmf_len) {
- ast_debug(1, "DTMF feature hook %p matched DTMF string '%s' on %p(%s)\n",
- hook, dtmf, bridge_channel, ast_channel_name(bridge_channel->chan));
- break;
- }
- ao2_ref(hook, -1);
- hook = NULL;
-
- /* Stop if we have reached the maximum length of a DTMF feature string. */
- } while (dtmf_len < ARRAY_LEN(dtmf) - 1);
-
- /* Since we are done bringing DTMF in return to using both begin and end frames */
- ast_clear_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_END_DTMF_ONLY);
-
- /* If a hook was actually matched execute it on this channel, otherwise stream up the DTMF to the other channels */
+ /* Add the new DTMF into the DTMF string so we can do our matching */
+ dtmf[dtmf_len] = res;
+ dtmf[++dtmf_len] = '\0';
+ }
+
if (hook) {
int remove_me;
+ /* Execute the matched hook on this channel. */
remove_me = hook->generic.callback(bridge_channel, hook->generic.hook_pvt);
if (remove_me) {
ast_debug(1, "DTMF hook %p is being removed from %p(%s)\n",
@@ -1083,6 +1094,7 @@
bridge_channel_handle_hangup(bridge_channel);
}
} else if (features->dtmf_passthrough) {
+ /* Stream any collected DTMF to the other channels. */
bridge_channel_write_dtmf_stream(bridge_channel, dtmf);
}
}
@@ -1226,13 +1238,6 @@
static void bridge_channel_handle_action(struct ast_bridge_channel *bridge_channel, struct ast_frame *action)
{
switch (action->subclass.integer) {
- case BRIDGE_CHANNEL_ACTION_FEATURE:
- bridge_channel_suspend(bridge_channel);
- ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
- bridge_channel_feature(bridge_channel);
- ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
- bridge_channel_unsuspend(bridge_channel);
- break;
case BRIDGE_CHANNEL_ACTION_DTMF_STREAM:
bridge_channel_suspend(bridge_channel);
ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
@@ -1600,22 +1605,35 @@
struct ast_bridge_hook_dtmf *hook;
char dtmf[2];
-/* 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. */
-/* BUGBUG the AMI atxfer action just sends DTMF end events to initiate DTMF atxfer and dial the extension. Another reason the DTMF hook matching needs rework. */
- /* 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 */
+ /* See if this DTMF matches the beginning of any feature hooks. */
dtmf[0] = frame->subclass.integer;
dtmf[1] = '\0';
hook = ao2_find(features->dtmf_hooks, dtmf, OBJ_PARTIAL_KEY);
if (hook) {
- struct ast_frame action = {
- .frametype = AST_FRAME_BRIDGE_ACTION,
- .subclass.integer = BRIDGE_CHANNEL_ACTION_FEATURE,
- };
+ enum ast_frame_type frametype = frame->frametype;
ast_frfree(frame);
frame = NULL;
- ast_bridge_channel_queue_frame(bridge_channel, &action);
+
ao2_ref(hook, -1);
+
+ /* Collect any more needed DTMF to execute a hook. */
+ bridge_channel_suspend(bridge_channel);
+ ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
+ switch (frametype) {
+ case AST_FRAME_DTMF_BEGIN:
+ bridge_channel_feature(bridge_channel, NULL);
+ break;
+ case AST_FRAME_DTMF_END:
+ bridge_channel_feature(bridge_channel, dtmf);
+ break;
+ default:
+ /* Unexpected frame type. */
+ ast_assert(0);
+ break;
+ }
+ ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
+ bridge_channel_unsuspend(bridge_channel);
}
return frame;
@@ -1655,12 +1673,11 @@
}
break;
case AST_FRAME_DTMF_BEGIN:
+ case AST_FRAME_DTMF_END:
frame = bridge_handle_dtmf(bridge_channel, frame);
if (!frame) {
return;
}
- /* Fall through */
- case AST_FRAME_DTMF_END:
if (!bridge_channel->features->dtmf_passthrough) {
ast_frfree(frame);
return;
Modified: trunk/main/manager.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/manager.c?view=diff&rev=396732&r1=396731&r2=396732
==============================================================================
--- trunk/main/manager.c (original)
+++ trunk/main/manager.c Thu Aug 15 09:20:59 2013
@@ -487,11 +487,8 @@
<parameter name="Exten" required="true">
<para>Extension to transfer to.</para>
</parameter>
- <parameter name="Context" required="true">
+ <parameter name="Context">
<para>Context to transfer to.</para>
- </parameter>
- <parameter name="Priority" required="true">
- <para>Priority to transfer to.</para>
</parameter>
</syntax>
<description>
@@ -4216,7 +4213,6 @@
pbx_builtin_setvar_helper(chan, "TRANSFER_CONTEXT", context);
}
-/* BUGBUG action_atxfer() is broken because the bridge DTMF hooks need both begin and end events to match correctly. */
for (digit = feature_code; *digit; ++digit) {
struct ast_frame f = { AST_FRAME_DTMF, .subclass.integer = *digit };
ast_queue_frame(chan, &f);
More information about the asterisk-commits
mailing list