[Asterisk-code-review] ARI: Allow for early bridging on dialed ARI channels. (asterisk[master])
Mark Michelson
asteriskteam at digium.com
Thu Apr 14 17:43:02 CDT 2016
Mark Michelson has uploaded a new change for review.
https://gerrit.asterisk.org/2620
Change subject: ARI: Allow for early bridging on dialed ARI channels.
......................................................................
ARI: Allow for early bridging on dialed ARI channels.
The Dial API and Bridge API in Asterisk have never had to play nice with
each other. Both have always operated independently. This meant that
either could assume direct control over involved channels. They could
poll the channels, read frames from the channels, and potentially even
hang up channels.
With the latest ARI additions, it is possible for an application writer
to add a channel to a bridge before or during a dial attempt. This means
now that the two APIs have to play nice with each other.
The Dial API becomes the frame handler for channels that are bridged.
This is mainly because the Dial API is already familiar with
dial-related events that need to be sent based on the reception of
certain frame types. The Dial API gets to be the boss of frames by
suspending the channel in the bridge. However, the Dial API does not get
to claim sole ownership of the channel. This means that the Dial API
does not get the liberty of getting to hang up the channel if things go
wrong. Instead, the Dial API unsuspends the bridge, leaving the fate of
the channel to the bridge itself.
In order to allow the Dial API the bridge access it required, a new
function has been added to allow for a generic frame to be written to a
bridge channel. Without this, passing early media to the bridge was not
possible.
Call forwarding is disabled on ARI dialed calls. Automatic call
forwarding does not play well with the threading model used by ARI
bridges. The next changeset will introduce events that allow for
application writers to perform the call forward themselves if that is
what they desire.
ASTERISK-25925
Reported by Mark Michelson
Change-Id: Ic3b05862644ca5080c7743a1b9b22465486e56ba
---
M include/asterisk/bridge_channel.h
M main/bridge_channel.c
M main/dial.c
M res/ari/resource_channels.c
4 files changed, 188 insertions(+), 45 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/20/2620/1
diff --git a/include/asterisk/bridge_channel.h b/include/asterisk/bridge_channel.h
index 797be4e..21064a3 100644
--- a/include/asterisk/bridge_channel.h
+++ b/include/asterisk/bridge_channel.h
@@ -404,6 +404,8 @@
*/
int ast_bridge_channel_write_control_data(struct ast_bridge_channel *bridge_channel, enum ast_control_frame_type control, const void *data, size_t datalen);
+int ast_bridge_channel_write_frame(struct ast_bridge_channel *bridge_channel, struct ast_frame *fr);
+
/*!
* \brief Write a hold frame into the bridge.
* \since 12.0.0
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 8172660..3a2a38b 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -1038,6 +1038,11 @@
return bridge_channel_write_frame(bridge_channel, &frame);
}
+int ast_bridge_channel_write_frame(struct ast_bridge_channel *bridge_channel, struct ast_frame *fr)
+{
+ return bridge_channel_write_frame(bridge_channel, fr);
+}
+
int ast_bridge_channel_write_hold(struct ast_bridge_channel *bridge_channel, const char *moh_class)
{
struct ast_json *blob;
diff --git a/main/dial.c b/main/dial.c
index fc66af5..7066e46 100644
--- a/main/dial.c
+++ b/main/dial.c
@@ -45,6 +45,8 @@
#include "asterisk/causes.h"
#include "asterisk/stasis_channels.h"
#include "asterisk/max_forwards.h"
+#include "asterisk/bridge.h"
+#include "asterisk/bridge_channel.h"
/*! \brief Main dialing structure. Contains global options, channels being dialed, and more! */
struct ast_dial {
@@ -74,6 +76,7 @@
char *assignedid2; /*!< UniqueID to assign 2nd channel */
struct ast_channel *owner; /*!< Asterisk channel */
AST_LIST_ENTRY(ast_dial_channel) list; /*!< Linked list information */
+ int bridge_suspended; /*!< Indicates if we have suspended the bridge this dial_channel is in */
};
/*! \brief Typedef for dial option enable */
@@ -502,6 +505,22 @@
return success;
}
+static void unsuspend_bridge(struct ast_dial_channel *dial_channel, struct ast_channel *chan)
+{
+ struct ast_bridge *bridge;
+
+ ast_channel_lock(chan);
+ bridge = ast_channel_get_bridge(chan);
+ ast_channel_unlock(chan);
+
+ if (bridge) {
+ ast_bridge_unsuspend(bridge, chan);
+ ao2_ref(bridge, -1);
+ }
+
+ dial_channel->bridge_suspended = 0;
+}
+
/*! \brief Helper function to handle channels that have been call forwarded */
static int handle_call_forward(struct ast_dial *dial, struct ast_dial_channel *channel, struct ast_channel *chan)
{
@@ -512,7 +531,12 @@
/* If call forwarding is disabled just drop the original channel and don't attempt to dial the new one */
if (FIND_RELATIVE_OPTION(dial, channel, AST_DIAL_OPTION_DISABLE_CALL_FORWARDING)) {
- ast_hangup(original);
+ if (channel->bridge_suspended) {
+ unsuspend_bridge(channel, original);
+ ast_channel_unref(original);
+ } else {
+ ast_hangup(original);
+ }
channel->owner = NULL;
return 0;
}
@@ -584,7 +608,8 @@
}
/*! \brief Helper function that handles frames */
-static void handle_frame(struct ast_dial *dial, struct ast_dial_channel *channel, struct ast_frame *fr, struct ast_channel *chan)
+static void handle_frame(struct ast_dial *dial, struct ast_dial_channel *channel,
+ struct ast_frame *fr, struct ast_channel *chan, struct ast_bridge_channel *bridge_chan)
{
if (fr->frametype == AST_FRAME_CONTROL) {
switch (fr->subclass.integer) {
@@ -600,24 +625,40 @@
AST_LIST_UNLOCK(&dial->channels);
ast_channel_publish_dial(chan, channel->owner, channel->device, "ANSWER");
set_state(dial, AST_DIAL_RESULT_ANSWERED);
+ if (bridge_chan) {
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_ANSWER, NULL, 0);
+ }
break;
case AST_CONTROL_BUSY:
ast_verb(3, "%s is busy\n", ast_channel_name(channel->owner));
ast_channel_publish_dial(chan, channel->owner, channel->device, "BUSY");
- ast_hangup(channel->owner);
+ if (bridge_chan) {
+ unsuspend_bridge(channel, channel->owner);
+ ast_channel_unref(channel->owner);
+ } else {
+ ast_hangup(channel->owner);
+ }
channel->cause = AST_CAUSE_USER_BUSY;
channel->owner = NULL;
break;
case AST_CONTROL_CONGESTION:
ast_verb(3, "%s is circuit-busy\n", ast_channel_name(channel->owner));
ast_channel_publish_dial(chan, channel->owner, channel->device, "CONGESTION");
+ if (bridge_chan) {
+ unsuspend_bridge(channel, channel->owner);
+ ast_channel_unref(channel->owner);
+ } else {
+ ast_hangup(channel->owner);
+ }
ast_hangup(channel->owner);
channel->cause = AST_CAUSE_NORMAL_CIRCUIT_CONGESTION;
channel->owner = NULL;
break;
case AST_CONTROL_INCOMPLETE:
ast_verb(3, "%s dialed Incomplete extension %s\n", ast_channel_name(channel->owner), ast_channel_exten(channel->owner));
- if (chan) {
+ if (bridge_chan) {
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_INCOMPLETE, NULL, 0);
+ } else if (chan) {
ast_indicate(chan, AST_CONTROL_INCOMPLETE);
} else {
ast_hangup(channel->owner);
@@ -627,8 +668,13 @@
break;
case AST_CONTROL_RINGING:
ast_verb(3, "%s is ringing\n", ast_channel_name(channel->owner));
- if (chan && !dial->options[AST_DIAL_OPTION_MUSIC])
- ast_indicate(chan, AST_CONTROL_RINGING);
+ if (!dial->options[AST_DIAL_OPTION_MUSIC]) {
+ if (bridge_chan) {
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_RINGING, NULL, 0);
+ } else if (chan) {
+ ast_indicate(chan, AST_CONTROL_RINGING);
+ }
+ }
set_state(dial, AST_DIAL_RESULT_RINGING);
break;
case AST_CONTROL_PROGRESS:
@@ -637,41 +683,52 @@
ast_indicate(chan, AST_CONTROL_PROGRESS);
} else {
ast_verb(3, "%s is making progress\n", ast_channel_name(channel->owner));
+ if (bridge_chan) {
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_PROGRESS, NULL, 0);
+ }
}
set_state(dial, AST_DIAL_RESULT_PROGRESS);
break;
case AST_CONTROL_VIDUPDATE:
- if (!chan) {
- break;
+ if (bridge_chan) {
+ ast_verb(3, "%s requested a video update\n", ast_channel_name(channel->owner));
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_VIDUPDATE, NULL, 0);
+ } else if (chan) {
+ ast_verb(3, "%s requested a video update, passing it to %s\n", ast_channel_name(channel->owner), ast_channel_name(chan));
+ ast_indicate(chan, AST_CONTROL_VIDUPDATE);
}
- ast_verb(3, "%s requested a video update, passing it to %s\n", ast_channel_name(channel->owner), ast_channel_name(chan));
- ast_indicate(chan, AST_CONTROL_VIDUPDATE);
break;
case AST_CONTROL_SRCUPDATE:
- if (!chan) {
- break;
+ if (bridge_chan) {
+ ast_verb(3, "%s requested a source update\n", ast_channel_name(channel->owner));
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_SRCUPDATE, NULL, 0);
+ } else if (chan) {
+ ast_verb(3, "%s requested a source update, passing it to %s\n", ast_channel_name(channel->owner), ast_channel_name(chan));
+ ast_indicate(chan, AST_CONTROL_SRCUPDATE);
}
- ast_verb(3, "%s requested a source update, passing it to %s\n", ast_channel_name(channel->owner), ast_channel_name(chan));
- ast_indicate(chan, AST_CONTROL_SRCUPDATE);
break;
case AST_CONTROL_CONNECTED_LINE:
- if (!chan) {
- break;
- }
- ast_verb(3, "%s connected line has changed, passing it to %s\n", ast_channel_name(channel->owner), ast_channel_name(chan));
- if (ast_channel_connected_line_sub(channel->owner, chan, fr, 1) &&
- ast_channel_connected_line_macro(channel->owner, chan, fr, 1, 1)) {
- ast_indicate_data(chan, AST_CONTROL_CONNECTED_LINE, fr->data.ptr, fr->datalen);
+ if (bridge_chan) {
+ ast_verb(3, "%s connected line has changed\n", ast_channel_name(channel->owner));
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_CONNECTED_LINE, fr->data.ptr, fr->datalen);
+ } else if (chan) {
+ ast_verb(3, "%s connected line has changed, passing it to %s\n", ast_channel_name(channel->owner), ast_channel_name(chan));
+ if (ast_channel_connected_line_sub(channel->owner, chan, fr, 1) &&
+ ast_channel_connected_line_macro(channel->owner, chan, fr, 1, 1)) {
+ ast_indicate_data(chan, AST_CONTROL_CONNECTED_LINE, fr->data.ptr, fr->datalen);
+ }
}
break;
case AST_CONTROL_REDIRECTING:
- if (!chan) {
- break;
- }
- ast_verb(3, "%s redirecting info has changed, passing it to %s\n", ast_channel_name(channel->owner), ast_channel_name(chan));
- if (ast_channel_redirecting_sub(channel->owner, chan, fr, 1) &&
- ast_channel_redirecting_macro(channel->owner, chan, fr, 1, 1)) {
- ast_indicate_data(chan, AST_CONTROL_REDIRECTING, fr->data.ptr, fr->datalen);
+ if (bridge_chan) {
+ ast_verb(3, "%s redirecting info has changed\n", ast_channel_name(channel->owner));
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_REDIRECTING, fr->data.ptr, fr->datalen);
+ } else if (chan) {
+ ast_verb(3, "%s redirecting info has changed, passing it to %s\n", ast_channel_name(channel->owner), ast_channel_name(chan));
+ if (ast_channel_redirecting_sub(channel->owner, chan, fr, 1) &&
+ ast_channel_redirecting_macro(channel->owner, chan, fr, 1, 1)) {
+ ast_indicate_data(chan, AST_CONTROL_REDIRECTING, fr->data.ptr, fr->datalen);
+ }
}
break;
case AST_CONTROL_PROCEEDING:
@@ -680,40 +737,54 @@
ast_indicate(chan, AST_CONTROL_PROCEEDING);
} else {
ast_verb(3, "%s is proceeding\n", ast_channel_name(channel->owner));
+ if (bridge_chan) {
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_PROCEEDING, NULL, 0);
+ }
}
set_state(dial, AST_DIAL_RESULT_PROCEEDING);
break;
case AST_CONTROL_HOLD:
- if (!chan) {
- break;
+ if (bridge_chan) {
+ ast_verb(3, "Call to %s placed on hold\n", ast_channel_name(channel->owner));
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_HOLD, fr->data.ptr, fr->datalen);
+ } else if (chan) {
+ ast_verb(3, "Call on %s placed on hold\n", ast_channel_name(chan));
+ ast_indicate_data(chan, AST_CONTROL_HOLD, fr->data.ptr, fr->datalen);
}
- ast_verb(3, "Call on %s placed on hold\n", ast_channel_name(chan));
- ast_indicate_data(chan, AST_CONTROL_HOLD, fr->data.ptr, fr->datalen);
break;
case AST_CONTROL_UNHOLD:
- if (!chan) {
- break;
+ if (bridge_chan) {
+ ast_verb(3, "Call to %s left from hold\n", ast_channel_name(channel->owner));
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_UNHOLD, NULL, 0);
+ } else if (chan) {
+ ast_verb(3, "Call on %s left from hold\n", ast_channel_name(chan));
+ ast_indicate(chan, AST_CONTROL_UNHOLD);
}
- ast_verb(3, "Call on %s left from hold\n", ast_channel_name(chan));
- ast_indicate(chan, AST_CONTROL_UNHOLD);
break;
case AST_CONTROL_OFFHOOK:
case AST_CONTROL_FLASH:
break;
case AST_CONTROL_PVT_CAUSE_CODE:
- if (chan) {
+ if (bridge_chan) {
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_PVT_CAUSE_CODE, fr->data.ptr, fr->datalen);
+ } else if (chan) {
ast_indicate_data(chan, AST_CONTROL_PVT_CAUSE_CODE, fr->data.ptr, fr->datalen);
}
break;
case -1:
- if (chan) {
- /* Prod the channel */
+ /* Prod the channel */
+ if (bridge_chan) {
+ ast_bridge_channel_write_control_data(bridge_chan, -1, NULL, 0);
+ } else if (chan) {
ast_indicate(chan, -1);
}
break;
default:
break;
}
+ } else if (bridge_chan) {
+ /* Non control types can be written directly to the bridge. */
+ ast_bridge_channel_write_frame(bridge_chan, fr);
}
}
@@ -737,7 +808,12 @@
/* Go through dropping out channels that have met their timeout */
AST_LIST_TRAVERSE(&dial->channels, channel, list) {
if (dial->state == AST_DIAL_RESULT_TIMEOUT || diff >= channel->timeout) {
- ast_hangup(channel->owner);
+ if (channel->bridge_suspended) {
+ unsuspend_bridge(channel, channel->owner);
+ ast_channel_unref(channel->owner);
+ } else {
+ ast_hangup(channel->owner);
+ }
channel->cause = AST_CAUSE_NO_ANSWER;
channel->owner = NULL;
} else if ((lowest_timeout == -1) || (lowest_timeout > channel->timeout)) {
@@ -765,6 +841,17 @@
case AST_CAUSE_NO_ANSWER:
default:
return "NOANSWER";
+ }
+}
+
+static void indicate_current_state(struct ast_bridge_channel *bridge_chan, struct ast_dial *dial)
+{
+ if (dial->state == AST_DIAL_RESULT_RINGING) {
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_RINGING, NULL, 0);
+ } else if (dial->state == AST_DIAL_RESULT_PROCEEDING) {
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_PROCEEDING, NULL, 0);
+ } else if (dial->state == AST_DIAL_RESULT_PROGRESS) {
+ ast_bridge_channel_write_control_data(bridge_chan, AST_CONTROL_PROGRESS, NULL, 0);
}
}
@@ -803,6 +890,8 @@
while ((dial->state != AST_DIAL_RESULT_UNANSWERED) && (dial->state != AST_DIAL_RESULT_ANSWERED) && (dial->state != AST_DIAL_RESULT_HANGUP) && (dial->state != AST_DIAL_RESULT_TIMEOUT)) {
int pos = 0, count = 0;
struct ast_frame *fr = NULL;
+ struct ast_bridge_channel *bridge_channel = NULL;
+ struct ast_bridge *bridge = NULL;
/* Set up channel structure array */
pos = count = 0;
@@ -862,16 +951,39 @@
if (chan)
ast_poll_channel_del(chan, channel->owner);
ast_channel_publish_dial(chan, who, channel->device, ast_hangup_cause_to_dial_status(ast_channel_hangupcause(who)));
- ast_hangup(who);
+ if (channel->bridge_suspended) {
+ unsuspend_bridge(channel, who);
+ ast_channel_unref(who);
+ } else {
+ ast_hangup(who);
+ }
channel->owner = NULL;
continue;
}
+ ast_channel_lock(who);
+ if (ast_channel_is_bridged(who)) {
+ if (!channel->bridge_suspended) {
+ bridge = ast_channel_get_bridge(who);
+ }
+ bridge_channel = ast_channel_get_bridge_channel(who);
+ }
+ ast_channel_unlock(who);
+
+ if (bridge) {
+ ast_bridge_suspend(bridge, who);
+ ao2_ref(bridge, -1);
+ channel->bridge_suspended = 1;
+ /* Get the bridge up to speed w.r.t the current dial status */
+ indicate_current_state(bridge_channel, dial);
+ }
+
/* Process the frame */
- handle_frame(dial, channel, fr, chan);
+ handle_frame(dial, channel, fr, chan, bridge_channel);
/* Free the received frame and start all over */
ast_frfree(fr);
+ ao2_cleanup(bridge_channel);
}
/* Do post-processing from loop */
@@ -879,12 +991,24 @@
/* Hangup everything except that which answered */
AST_LIST_LOCK(&dial->channels);
AST_LIST_TRAVERSE(&dial->channels, channel, list) {
- if (!channel->owner || channel->owner == who)
+ int hangup_channel;
+
+ if (channel->bridge_suspended) {
+ unsuspend_bridge(channel, channel->owner ?: who);
+ ast_channel_cleanup(channel->owner);
+ hangup_channel = 0;
+ } else{
+ hangup_channel = 1;
+ }
+ if (!channel->owner || channel->owner == who) {
continue;
+ }
if (chan)
ast_poll_channel_del(chan, channel->owner);
ast_channel_publish_dial(chan, channel->owner, channel->device, "CANCEL");
- ast_hangup(channel->owner);
+ if (hangup_channel) {
+ ast_hangup(channel->owner);
+ }
channel->cause = AST_CAUSE_ANSWERED_ELSEWHERE;
channel->owner = NULL;
}
@@ -904,12 +1028,23 @@
/* Hangup everything */
AST_LIST_LOCK(&dial->channels);
AST_LIST_TRAVERSE(&dial->channels, channel, list) {
+ int hangup_channel;
+
if (!channel->owner)
continue;
+ if (channel->bridge_suspended) {
+ unsuspend_bridge(channel, channel->owner);
+ ast_channel_unref(channel->owner);
+ hangup_channel = 0;
+ } else {
+ hangup_channel = 1;
+ }
if (chan)
ast_poll_channel_del(chan, channel->owner);
ast_channel_publish_dial(chan, channel->owner, channel->device, "CANCEL");
- ast_hangup(channel->owner);
+ if (hangup_channel) {
+ ast_hangup(channel->owner);
+ }
channel->cause = AST_CAUSE_NORMAL_CLEARING;
channel->owner = NULL;
}
diff --git a/res/ari/resource_channels.c b/res/ari/resource_channels.c
index c838bc3..d344c56 100644
--- a/res/ari/resource_channels.c
+++ b/res/ari/resource_channels.c
@@ -1629,6 +1629,7 @@
ast_dial_set_user_data(dial, control);
ast_dial_set_global_timeout(dial, args->timeout * 1000);
+ ast_dial_option_global_enable(dial, AST_DIAL_OPTION_DISABLE_CALL_FORWARDING, NULL);
if (stasis_app_control_dial(control, dial)) {
ast_dial_destroy(dial);
--
To view, visit https://gerrit.asterisk.org/2620
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic3b05862644ca5080c7743a1b9b22465486e56ba
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-code-review
mailing list