[asterisk-commits] mmichelson: branch mmichelson/sip_transfer r387013 - /team/mmichelson/sip_tra...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Tue Apr 30 13:53:58 CDT 2013
Author: mmichelson
Date: Tue Apr 30 13:53:54 2013
New Revision: 387013
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=387013
Log:
Add comments to REFER-handling code in chan_sip.c
This is mostly so that I can go about reforming the transfers
to not suck. Giving the code an initial survey will help to ensure
I don't do something awful.
Modified:
team/mmichelson/sip_transfer/channels/chan_sip.c
Modified: team/mmichelson/sip_transfer/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/sip_transfer/channels/chan_sip.c?view=diff&rev=387013&r1=387012&r2=387013
==============================================================================
--- team/mmichelson/sip_transfer/channels/chan_sip.c (original)
+++ team/mmichelson/sip_transfer/channels/chan_sip.c Tue Apr 30 13:53:54 2013
@@ -24383,6 +24383,8 @@
XXX Should we add a wait period after streaming audio and before hangup?? Sometimes the
audio can't be heard before hangup
*/
+
+/*XXX Get rid of this atrocity */
static void *sip_park_thread(void *stuff)
{
struct ast_channel *transferee, *transferer; /* Chan1: The transferee, Chan2: The transferer */
@@ -24438,6 +24440,7 @@
}
/*! DO NOT hold any locks while calling sip_park */
+/* XXX Get rid of this atrocity */
static int sip_park(struct ast_channel *chan1, struct ast_channel *chan2, struct sip_request *req, uint32_t seqno, const char *park_exten, const char *park_context)
{
struct sip_dual *d;
@@ -26510,6 +26513,10 @@
goto handle_refer_cleanup;
}
+ /* XXX Up to this point, everything is fine. It's the stuff below this point
+ * where we can start to cull code.
+ */
+
/* If this is a blind transfer, we have the following
channels to work with:
- chan1, chan2: The current call between transferer and transferee (2 channels)
@@ -26534,6 +26541,11 @@
header in the INVITE.
*/
+ /* XXX There's no need for the 'current' structure any longer. It
+ * requires reaching across the bridge to populate and we don't need
+ * to do that any more. We just need the transferer channel(s).
+ */
+
/* Get the transferer's channel */
chans[0] = current.chan1 = p->owner;
@@ -26546,12 +26558,20 @@
}
if (sipdebug) {
+ /* XXX This message needs to be changed not to reference the transferee
+ * channel. We can't know what's on the other side of the bridge
+ */
ast_debug(3, "SIP %s transfer: Transferer channel %s, transferee channel %s\n",
p->refer->attendedtransfer ? "attended" : "blind",
ast_channel_name(current.chan1),
current.chan2 ? ast_channel_name(current.chan2) : "<none>");
}
+ /* XXX This is trying to prevent a blind transfer of an unbridged channel.
+ * This is taken care of already by the bridging core and can be removed
+ * entirely. This does raise the point that different transfer failures will
+ * need to result in different responses/history entries
+ */
if (!current.chan2 && !p->refer->attendedtransfer) {
/* No bridged channel, propably IVR or echo or similar... */
/* Guess we should masquerade or something here */
@@ -26566,6 +26586,10 @@
goto handle_refer_cleanup;
}
+ /* XXX Hm, queuing a hold frame is interesting. Should that maybe become
+ * part of the bridging core? Should we only queue a hold if the channel
+ * is bridged?
+ */
if (current.chan2) {
if (sipdebug) {
ast_debug(4, "Got SIP transfer, applying to bridged peer '%s'\n", ast_channel_name(current.chan2));
@@ -26590,6 +26614,18 @@
}
/* Fallthrough if we can't find the call leg internally */
}
+
+ /* XXX From here on, we know we're not doing a local attended transfer (i.e. attended transfer
+ * of a call on the system). We're either doing a remote attended transfer or blind transfer.
+ *
+ * In the case of a remote attended transfer, what we're supposed to do is generate an INVITE with replaces
+ * to the destination specified in the Refer-to header. We treat the destination as being an extension
+ * in the dialplan, so the treatment of remote attended transfers and blind transfers is more-or-less the
+ * same. The big difference is that the resulting INVITE on a remote attended transfer will have a Replaces
+ * header. This is making a pretty BIG assumption that the resulting dialplan will result in an outbound
+ * call and that it will be to a SIP destination, and that the SIP destination will be able to replace
+ * as appropriate.
+ */
/* Copy data we can not safely access after letting the pvt lock go. */
refer_to = ast_strdupa(p->refer->refer_to);
@@ -26607,6 +26643,11 @@
sip_pvt_unlock(p);
/* Parking a call. DO NOT hold any locks while calling ast_parking_ext_valid() */
+
+ /* XXX This takes care of blind transferring to a parking lot. The bridge transfer code
+ * does this for us. No need to do anything special here. That AMI event. yowza.
+ * Transfer2Parking? really?
+ */
if (localtransfer && ast_parking_ext_valid(refer_to, current.chan1, refer_to_context)) {
sip_pvt_lock(p);
ast_clear_flag(&p->flags[0], SIP_GOTREFER);
@@ -26653,6 +26694,12 @@
pbx_builtin_setvar_helper(current.chan1, "BLINDTRANSFER", ast_channel_name(current.chan2));
}
+ /* XXX Now we run into an interesting issue. We're used to setting channel variables on the
+ * transferee channel. Some of these appear to be intended to go onto a newly-created channel
+ * when we create a new one. These we can just put on the transferer channel since it is used
+ * as the requestor for the new channel in the bridge blind transfer code. The other diagnostic
+ * channel variables...I'm not so sure about.
+ */
if (current.chan2) {
pbx_builtin_setvar_helper(current.chan2, "BLINDTRANSFER", ast_channel_name(current.chan1));
pbx_builtin_setvar_helper(current.chan2, "SIPDOMAIN", refer_to_domain);
@@ -26668,6 +26715,11 @@
* a Diversion header present in the REFER with an appropriate reason parameter
* set. We need to update the redirecting information appropriately.
*/
+
+ /* XXX This bit should probably still exist in our transfer code. However, it really
+ * has nothing to do with the transferee channel. We could potentially change to do
+ * this only if the original call was bridged.
+ */
ast_channel_lock(p->owner);
sip_pvt_lock(p);
ast_party_redirecting_init(&redirecting);
@@ -26693,6 +26745,9 @@
p->refer->replaces_callid_fromtag ? ";from-tag=" : "",
p->refer->replaces_callid_fromtag);
+ /* XXX Set this on the transferer channel instead. The new channel in a blind
+ * transfer will get this.
+ */
if (current.chan2) {
sip_pvt_unlock(p);
pbx_builtin_setvar_helper(current.chan2, "_SIPTRANSFER_REPLACES", tempheader);
@@ -26703,6 +26758,10 @@
/* Connect the call */
/* FAKE ringing if not attended transfer */
+ /* XXX Technically, we could add a framehook to the call to ast_bridge_transfer_blind()
+ * so that we can give real ringing notification to the transferer, but since the goal
+ * is simply to convert to using the core API, we'll hold off on that for the time being
+ */
if (!p->refer->attendedtransfer) {
transmit_notify_with_sipfrag(p, seqno, "180 Ringing", FALSE);
}
@@ -26735,8 +26794,14 @@
* indicate before masquerade so the indication actually makes it to the real channel
* when using local channels with MOH passthru */
ast_indicate(current.chan2, AST_CONTROL_UNHOLD);
+
res = ast_async_goto(current.chan2, refer_to_context, refer_to, 1);
+ /* XXX This check of 'res' needs to be changed to check the potential
+ * returns of ast_bridge_transfer_blind(). Most of what's done here
+ * can be removed though. Manager and CEL events will be generated
+ * by the core.
+ */
if (!res) {
ast_manager_event_multichan(EVENT_FLAG_CALL, "Transfer", 2, chans,
"TransferMethod: SIP\r\n"
More information about the asterisk-commits
mailing list