diff options
author | Richard Mudgett <rmudgett@digium.com> | 2014-11-06 19:03:46 +0000 |
---|---|---|
committer | Richard Mudgett <rmudgett@digium.com> | 2014-11-06 19:03:46 +0000 |
commit | 2878554bccbd279a86b7510d3561f2a82419e469 (patch) | |
tree | d89c1c5190219219a6d0bd33d93a70b7b187f9a1 /main/bridge_channel.c | |
parent | 248c592292fdf380cc996ef9fb5c410ce3f48671 (diff) |
Bridge DTMF hooks: Made audio pass from the bridge while waiting for more matching digits.
* Made collecting DTMF digits for the DTMF feature hooks pass frames from
the bridge.
* Made collecting DTMF digits possible by other bridge hooks if there is a
need.
ASTERISK-24447 #close
Reported by: Richard Mudgett
Review: https://reviewboard.asterisk.org/r/4123/
........
Merged revisions 427493 from http://svn.asterisk.org/svn/asterisk/branches/12
git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/13@427494 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Diffstat (limited to 'main/bridge_channel.c')
-rw-r--r-- | main/bridge_channel.c | 354 |
1 files changed, 244 insertions, 110 deletions
diff --git a/main/bridge_channel.c b/main/bridge_channel.c index d94b7387f..60fa03082 100644 --- a/main/bridge_channel.c +++ b/main/bridge_channel.c @@ -1510,117 +1510,146 @@ static void testsuite_notify_feature_success(struct ast_channel *chan, const cha #endif /* TEST_FRAMEWORK */ } -/*! - * \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, const char *starting_dtmf) +void ast_bridge_channel_feature_digit(struct ast_bridge_channel *bridge_channel, int digit) { 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; - unsigned int digit_timeout; - RAII_VAR(struct ast_features_general_config *, gen_cfg, NULL, ao2_cleanup); - ast_channel_lock(bridge_channel->chan); - gen_cfg = ast_get_chan_features_general_config(bridge_channel->chan); - if (!gen_cfg) { - ast_log(LOG_ERROR, "Unable to retrieve features configuration.\n"); - ast_channel_unlock(bridge_channel->chan); + struct sanity_check_of_dtmf_size { + char check[1 / (ARRAY_LEN(bridge_channel->dtmf_hook_state.collected) == ARRAY_LEN(hook->dtmf.code))]; + }; + + dtmf_len = strlen(bridge_channel->dtmf_hook_state.collected); + if (!dtmf_len && !digit) { + /* Nothing to do */ return; } - digit_timeout = gen_cfg->featuredigittimeout; - ast_channel_unlock(bridge_channel->chan); - 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); - } + if (digit) { + /* There should always be room for the new digit. */ + ast_assert(dtmf_len < ARRAY_LEN(bridge_channel->dtmf_hook_state.collected) - 1); - /* - * Check if any feature DTMF hooks match or could match and - * try to collect more DTMF digits. - */ - for (;;) { - int res; + /* Add the new digit to the DTMF string so we can do our matching */ + bridge_channel->dtmf_hook_state.collected[dtmf_len++] = digit; + bridge_channel->dtmf_hook_state.collected[dtmf_len] = '\0'; - 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); + ast_debug(1, "DTMF feature string on %p(%s) is now '%s'\n", + bridge_channel, ast_channel_name(bridge_channel->chan), + bridge_channel->dtmf_hook_state.collected); - /* 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; + /* See if a DTMF feature hook matches or can match */ + hook = ao2_find(features->dtmf_hooks, bridge_channel->dtmf_hook_state.collected, + OBJ_SEARCH_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), + bridge_channel->dtmf_hook_state.collected); + } else if (dtmf_len != strlen(hook->dtmf.code)) { + unsigned int digit_timeout; + struct ast_features_general_config *gen_cfg; + + /* Need more digits to match */ + ao2_ref(hook, -1); + + /* Determine interdigit timeout */ + ast_channel_lock(bridge_channel->chan); + gen_cfg = ast_get_chan_features_general_config(bridge_channel->chan); + ast_channel_unlock(bridge_channel->chan); + if (!gen_cfg) { + ast_log(LOG_ERROR, "Unable to retrieve features configuration.\n"); + digit_timeout = 3000; /* Pick a reasonable failsafe timeout in ms */ + } else { + digit_timeout = gen_cfg->featuredigittimeout; + ao2_ref(gen_cfg, -1); } - 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; + + bridge_channel->dtmf_hook_state.interdigit_timeout = + ast_tvadd(ast_tvnow(), ast_samp2tv(digit_timeout, 1000)); + return; + } else { + int remove_me; + int already_suspended; + + ast_debug(1, "DTMF feature hook %p matched DTMF string '%s' on %p(%s)\n", + hook, bridge_channel->dtmf_hook_state.collected, bridge_channel, + ast_channel_name(bridge_channel->chan)); + + /* + * Clear the collected digits before executing the hook + * in case the hook starts another sequence. + */ + bridge_channel->dtmf_hook_state.collected[0] = '\0'; + + ast_bridge_channel_lock_bridge(bridge_channel); + already_suspended = bridge_channel->suspended; + if (!already_suspended) { + bridge_channel_internal_suspend_nolock(bridge_channel); } + ast_bridge_unlock(bridge_channel->bridge); + ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE); + + /* 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", + hook, bridge_channel, ast_channel_name(bridge_channel->chan)); + ao2_unlink(features->dtmf_hooks, hook); + } + testsuite_notify_feature_success(bridge_channel->chan, hook->dtmf.code); 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; + ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE); + if (!already_suspended) { + bridge_channel_unsuspend(bridge_channel); } - } - res = ast_waitfordigit(bridge_channel->chan, digit_timeout); - if (!res) { - ast_debug(1, "DTMF feature string collection on %p(%s) timed out\n", - bridge_channel, ast_channel_name(bridge_channel->chan)); - break; - } - if (res < 0) { - ast_debug(1, "DTMF feature string collection failed on %p(%s) for some reason\n", - bridge_channel, ast_channel_name(bridge_channel->chan)); - break; + /* + * If we are handing the channel off to an external hook for + * ownership, we are not guaranteed what kind of state it will + * come back in. If the channel hungup, we need to detect that + * here if the hook did not already change the state. + */ + if (bridge_channel->chan && ast_check_hangup_locked(bridge_channel->chan)) { + ast_bridge_channel_kick(bridge_channel, 0); + } + return; } - - /* Add the new DTMF into the DTMF string so we can do our matching */ - dtmf[dtmf_len] = res; - dtmf[++dtmf_len] = '\0'; + } else { + ast_debug(1, "DTMF feature string collection on %p(%s) timed out\n", + bridge_channel, ast_channel_name(bridge_channel->chan)); } - if (hook) { - int remove_me; + /* Timeout or DTMF digit didn't allow a match with any hooks. */ + if (features->dtmf_passthrough) { + /* Stream the collected DTMF to the other channels. */ + bridge_channel_write_dtmf_stream(bridge_channel, + bridge_channel->dtmf_hook_state.collected); + } + bridge_channel->dtmf_hook_state.collected[0] = '\0'; - /* 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", - hook, bridge_channel, ast_channel_name(bridge_channel->chan)); - ao2_unlink(features->dtmf_hooks, hook); - } - testsuite_notify_feature_success(bridge_channel->chan, hook->dtmf.code); - ao2_ref(hook, -1); + ast_test_suite_event_notify("FEATURE_DETECTION", "Result: fail"); +} - /* - * If we are handing the channel off to an external hook for - * ownership, we are not guaranteed what kind of state it will - * come back in. If the channel hungup, we need to detect that - * here if the hook did not already change the state. - */ - if (bridge_channel->chan && ast_check_hangup_locked(bridge_channel->chan)) { - ast_bridge_channel_kick(bridge_channel, 0); - } - } else { - if (features->dtmf_passthrough) { - /* Stream any collected DTMF to the other channels. */ - bridge_channel_write_dtmf_stream(bridge_channel, dtmf); - } - ast_test_suite_event_notify("FEATURE_DETECTION", "Result: fail"); +/*! + * \internal + * \brief Handle bridge channel DTMF feature timeout expiration. + * \since 12.8.0 + * + * \param bridge_channel Channel to check expired interdigit timer on. + * + * \return Nothing + */ +static void bridge_channel_handle_feature_timeout(struct ast_bridge_channel *bridge_channel) +{ + if (!bridge_channel->dtmf_hook_state.collected[0] + || 0 < ast_tvdiff_ms(bridge_channel->dtmf_hook_state.interdigit_timeout, + ast_tvnow())) { + /* Not within a sequence or not timed out. */ + return; } + + ast_bridge_channel_feature_digit(bridge_channel, 0); } /*! @@ -2077,6 +2106,25 @@ static void bridge_channel_handle_control(struct ast_bridge_channel *bridge_chan /*! * \internal + * \param bridge_channel Channel to read wr_queue alert pipe. + * + * \return Nothing + */ +static void bridge_channel_read_wr_queue_alert(struct ast_bridge_channel *bridge_channel) +{ + char nudge; + + if (read(bridge_channel->alert_pipe[0], &nudge, sizeof(nudge)) < 0) { + if (errno != EINTR && errno != EAGAIN) { + ast_log(LOG_WARNING, "read() failed for alert pipe on %p(%s): %s\n", + bridge_channel, ast_channel_name(bridge_channel->chan), + strerror(errno)); + } + } +} + +/*! + * \internal * \brief Handle bridge channel write frame to channel. * \since 12.0.0 * @@ -2087,21 +2135,49 @@ static void bridge_channel_handle_control(struct ast_bridge_channel *bridge_chan static void bridge_channel_handle_write(struct ast_bridge_channel *bridge_channel) { struct ast_frame *fr; - char nudge; struct sync_payload *sync_payload; ast_bridge_channel_lock(bridge_channel); - if (read(bridge_channel->alert_pipe[0], &nudge, sizeof(nudge)) < 0) { - if (errno != EINTR && errno != EAGAIN) { - ast_log(LOG_WARNING, "read() failed for alert pipe on %p(%s): %s\n", - bridge_channel, ast_channel_name(bridge_channel->chan), strerror(errno)); + + /* It's not good to have unbalanced frames and alert_pipe alerts. */ + ast_assert(!AST_LIST_EMPTY(&bridge_channel->wr_queue)); + if (AST_LIST_EMPTY(&bridge_channel->wr_queue)) { + /* No frame, flush the alert pipe of excess alerts. */ + ast_log(LOG_WARNING, "Weird. No frame from bridge for %s to process?\n", + ast_channel_name(bridge_channel->chan)); + bridge_channel_read_wr_queue_alert(bridge_channel); + ast_bridge_channel_unlock(bridge_channel); + return; + } + + AST_LIST_TRAVERSE_SAFE_BEGIN(&bridge_channel->wr_queue, fr, frame_list) { + if (bridge_channel->dtmf_hook_state.collected[0]) { + switch (fr->frametype) { + case AST_FRAME_BRIDGE_ACTION: + case AST_FRAME_BRIDGE_ACTION_SYNC: + /* Defer processing these frames while DTMF is collected. */ + continue; + default: + break; + } } + bridge_channel_read_wr_queue_alert(bridge_channel); + AST_LIST_REMOVE_CURRENT(frame_list); + break; } - fr = AST_LIST_REMOVE_HEAD(&bridge_channel->wr_queue, frame_list); + AST_LIST_TRAVERSE_SAFE_END; + ast_bridge_channel_unlock(bridge_channel); if (!fr) { + /* + * Wait some to reduce CPU usage from a tight loop + * without any wait because we only have deferred + * frames in the wr_queue. + */ + usleep(1); return; } + switch (fr->frametype) { case AST_FRAME_BRIDGE_ACTION: bridge_channel_handle_action(bridge_channel, fr->subclass.integer, fr->data.ptr); @@ -2128,38 +2204,36 @@ static void bridge_channel_handle_write(struct ast_bridge_channel *bridge_channe static struct ast_frame *bridge_handle_dtmf(struct ast_bridge_channel *bridge_channel, struct ast_frame *frame) { struct ast_bridge_features *features = bridge_channel->features; - struct ast_bridge_hook_dtmf *hook; + struct ast_bridge_hook_dtmf *hook = NULL; char dtmf[2]; - /* See if this DTMF matches the beginning of any feature hooks. */ + /* + * See if we are already matching a DTMF feature hook sequence or + * if this DTMF matches the beginning of any DTMF feature hooks. + */ dtmf[0] = frame->subclass.integer; dtmf[1] = '\0'; - hook = ao2_find(features->dtmf_hooks, dtmf, OBJ_PARTIAL_KEY); - if (hook) { + if (bridge_channel->dtmf_hook_state.collected[0] + || (hook = ao2_find(features->dtmf_hooks, dtmf, OBJ_SEARCH_PARTIAL_KEY))) { enum ast_frame_type frametype = frame->frametype; bridge_frame_free(frame); frame = NULL; - ao2_ref(hook, -1); + ao2_cleanup(hook); - /* 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); + /* Just eat the frame. */ break; case AST_FRAME_DTMF_END: - bridge_channel_feature(bridge_channel, dtmf); + ast_bridge_channel_feature_digit(bridge_channel, dtmf[0]); break; default: /* Unexpected frame type. */ ast_assert(0); break; } - ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE); - bridge_channel_unsuspend(bridge_channel); #ifdef TEST_FRAMEWORK } else if (frame->frametype == AST_FRAME_DTMF_END) { /* Only transmit this event on DTMF end or else every DTMF @@ -2259,6 +2333,60 @@ static int bridge_channel_next_interval(struct ast_bridge_channel *bridge_channe /*! * \internal + * \brief Determine how long till the DTMF interdigit timeout. + * \since 12.8.0 + * + * \param bridge_channel Channel to determine how long can wait. + * + * \retval ms Number of milliseconds to wait. + * \retval -1 to wait forever. + */ +static int bridge_channel_feature_timeout(struct ast_bridge_channel *bridge_channel) +{ + int ms; + + if (bridge_channel->dtmf_hook_state.collected[0]) { + ms = ast_tvdiff_ms(bridge_channel->dtmf_hook_state.interdigit_timeout, + ast_tvnow()); + if (ms < 0) { + /* Expire immediately. */ + ms = 0; + } + } else { + /* Timer is not active so wait forever. */ + ms = -1; + } + + return ms; +} + +/*! + * \internal + * \brief Determine how long till a timeout. + * \since 12.8.0 + * + * \param bridge_channel Channel to determine how long can wait. + * + * \retval ms Number of milliseconds to wait. + * \retval -1 to wait forever. + */ +static int bridge_channel_next_timeout(struct ast_bridge_channel *bridge_channel) +{ + int ms_interval; + int ms; + + ms_interval = bridge_channel_next_interval(bridge_channel); + ms = bridge_channel_feature_timeout(bridge_channel); + if (ms < 0 || (0 <= ms_interval && ms_interval < ms)) { + /* Interval hook timeout is next. */ + ms = ms_interval; + } + + return ms; +} + +/*! + * \internal * \brief Wait for something to happen on the bridge channel and handle it. * \since 12.0.0 * @@ -2286,7 +2414,7 @@ static void bridge_channel_wait(struct ast_bridge_channel *bridge_channel) } else { ast_bridge_channel_unlock(bridge_channel); outfd = -1; - ms = bridge_channel_next_interval(bridge_channel); + ms = bridge_channel_next_timeout(bridge_channel); chan = ast_waitfor_nandfds(&bridge_channel->chan, 1, &bridge_channel->alert_pipe[0], 1, NULL, &outfd, &ms); if (ast_channel_unbridged(bridge_channel->chan)) { @@ -2303,11 +2431,17 @@ static void bridge_channel_wait(struct ast_bridge_channel *bridge_channel) && bridge_channel->state == BRIDGE_CHANNEL_STATE_WAIT) { if (chan) { bridge_handle_trip(bridge_channel); - } else if (-1 < outfd) { - bridge_channel_handle_write(bridge_channel); } else if (ms == 0) { - /* An interval expired. */ + /* An interdigit timeout or interval expired. */ + bridge_channel_handle_feature_timeout(bridge_channel); bridge_channel_handle_interval(bridge_channel); + } else if (-1 < outfd) { + /* + * Must do this after checking timeouts or may have + * an infinite loop due to deferring write queue + * actions while trying to match DTMF feature hooks. + */ + bridge_channel_handle_write(bridge_channel); } } bridge_channel->activity = BRIDGE_CHANNEL_THREAD_IDLE; |