Summary:	ASTERISK-08972: [patch] sip attended transfer - xfersound
Reporter:	Erik Sundberg (sunder)	Labels:
Date Opened:	2007-03-08 12:42:45.000-0600	Date Closed:	2008-03-25 13:07:22
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/Transfers
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) patch-sip_xfersound-asterisk-1.2.15.tar ( 1) xfersound-r1.patch
Description:	I have had multiple requests for asterisk to play an xfersound sound when the call is bridged during a attend transfer, from our customers. So I sat down it wrote it. I works great so far, but I had to make some changes to channel.c, channel.h, and chan_sip.so. The code that I wrote is mainly a proof of concept. I would like everyone to review the code real quick to see if this I will break anything else,  if I am going about it the wrong way, or if i need to change something. The way that I wrote this, it will easily be able to move the code over to other channels, especially IAX. I have developed this on asterisk-1.2.15, since I am the most familiar with 1.2, and have not touched 1.4 yet. After I get thing nailed down and working properly I will get it working in the trunk version and then submit it to bugs.digium.com. Plays a ?beep? only on the phone that is receiving the transferred call. Related bug tracker item http://bugs.digium.com/view.php?id=3819 , I found this after I wrote the code. How it works. File: channels.h Added xfersound variable to the ast_channel structure File: Chan_sip.c Added xfersound variable to the ast_channel structure If there is a attend transfer request set sip_pvt->xfersound to 1 Then the function attempt_transfer() is call to try and bridge the channels. If the sip_pvt->xfersound flag is set to 1 Then set ast_channel->xfersound to 1 File: Channels.c During ast_generic_bridge() During the infinite ?for(;;)? loop If the xfersound flag is set in the ast_channel structure of either peer.                       Bridge_playfile(?beep?) ****** ADDITIONAL INFORMATION ****** Still to do. -Get the sound file from features.conf -Make xfersound=on/off variable in sip.conf as a general, peer, and user setting. -Potential option is to set the xfersound to the file you want to play, instead of on/off. So in sip.conf under any context would be ?xfersound=beep?. And if the xfersound channel variable is set and the peer xfersound value is set and has a valid sound file play. Then play the sound file that was defined in sip.conf the sound file on the channel. -In ast_generic_bridge, I don?t know if the ?if? statement to play the sound is the best place. I might move it before the infinite ?for? loop. Not sure? your option? -Next channel to work on is iax.
Comments:	By: Serge Vecher (serge-v) 2007-03-08 12:58:23.000-0600 since this is a new feature, you must include a patch for trunk in order for this to be considered. Also, please make a single patch for all three files. this is done using 'svn diff' By: Olle Johansson (oej) 2007-04-27 09:55:42 You can really do this in the dialplan. I don't see any reason to do it in the source, when it's perfectly possible to do in the dialplan. By: Miz Jones (mizatservercave) 2007-04-27 18:54:10 oej, perhaps a quick note on how to implement it in the dialplan? I'm not sure which application would be appropriate to catch all transfer types -- from Zap to SIP, SIP to IAX, SIP to SIP, and so-on, where Dial() with the A flag does not seem, at first glance, to be the appropriate solution for all cases. I will run a few experiments in our environment tonight to see what I can come up with -- I seem to recall SIP to SIP transfers or conferencing (three-way SIP-based, beep on call join?) being a corner case which I'd like to be caught in the source, if even possible. By: Olle Johansson (oej) 2007-04-30 02:00:31 Run dumpchan() on a match-all in TRANSFER_CONTEXT to see the variables I set. It's easy to see that a TRANSFER is going on and play a sound, checking whether the transfer is ok and then transfer the call. By: Miz Jones (mizatservercave) 2007-04-30 14:12:01 I'm still a little fuzzy on this. During a blind transfer, DumpChan() shows that BLINDTRANSFER is set, which is handy for passing into Dial's A() when dialing via macro (as we do) -- but I'm not sure that addresses the goal. In our environment at least, the idea is to have a soundfile play to the destination channel (and optionally the transferee) as soon as the bridge between transferee and destination is complete. I may be misunderstanding the usage the Transfer() app when attempting to apply it to this scenario: overriding TRANSFER_CONTEXT by Set()'ing __TRANSFER_CONTEXT, and using Transfer to attempt logic afterward.  I have been able to use that method with ... very mixed-results, none of which have allowed me to get a soundfile played at the appropriate time. Call parking is not a valid option for us, which, admittedly, could provide the 'okay, now you're connected to the transferee and you know precisely when it happens', but people here don't want such "indirect" transfers. By: Miz Jones (mizatservercave) 2007-04-30 14:17:42 I should mention that I'm using 1.2.18, upgraded form 1.2.4 earlier today. By: Erik Sundberg (sunder) 2007-05-01 23:46:30 I hope this clarifies what we are trying to do with this patch.. ------------------------------------------------------------------------------ The main purpose of this patch is to. A Calls B B Presses Transfer Key on phone, so A now is listening to Music on Hold B Calls C C Wants to take call B Press Transfer key C Hears XFERSOUND A Hears XFERSOUND (Optionial) A and C are now Bridged What is patch is addressing. Currently C doesn't know when they are bridge to A (there are no sounds or notifications). A knows he is bridge or something happen because he stops hearing Music On Hold. We don't want to do Call Parking and Meetme. I will also test the TRANSFER_CONTEXT in the next couple of days to see if i can get it working. And i know that i have to get this in to TRUNK. -------------------------------------------------------------------------- I could See the Dial(Channel,x,A(soundfile)) Working with a blind transfer, but we are doing attended/supervised transfers. Also i don't think that we would need to play a beep on a blind tranfer because C is answering a ring phone and is going to say "Hello", and doesn't need to hear the beep. Example ------- A calls B B blind transfer to C B is disconnected A hears MOH or Ringing() C phone is ringing C Answers and A MOH or ringing is stoped. C hears Beep (C really doesn't need to hear the beep b/c he is answering a ringing phone.) A and C are bridge By: Tom L (toml) 2007-07-05 19:58:38 What's the status here? This feature is something that we urgently need. What needs to be done for this to be included? By: Erik Sundberg (sunder) 2007-07-31 20:29:04 I am back and have time to work on this is should be able to push this out in about a week By: Erik Sundberg (sunder) 2007-07-31 23:37:10 Here is a patch for trunk: xfersound-r1.patch It is untested................. It complies..................... It should******* Work............. I will test when i am back at the office..... When you apply this patch, xfersound always turn on no matter want. After i verify this, i will create a setting in general to turn on and off the xfersound, and choose what file is to be played. Right now it just plays "beep" By: Olle Johansson (oej) 2007-12-10 07:58:21.000-0600 So why can't you do this in the dialplan? I'm still confused. By: Olle Johansson (oej) 2008-01-22 09:36:07.000-0600 No answer from reporter, and it's doable in dialplan. By: Olle Johansson (oej) 2008-01-24 01:27:52.000-0600 re-opening thanks to reminder from Nic Bellamy on the asterisk-dev list. By: Olle Johansson (oej) 2008-01-24 01:30:07.000-0600 Anyway to do this in a more simple way or do it more generic? This patch involves the channel structure and channel.c - yet it's written as a SIP feature. Let's see if we can do it only in chan_sip or create a generic function that also works in res_features attended transfer. The xfersound needs to be configurable. By: Nic Bellamy (nic_bellamy) 2008-01-24 16:33:12.000-0600 I'll see if I can find some time at some point to work up a version of this that is more generic so it can work with both SIP attended transfers and also the res_features version. It's something I and a lot of others would really like to have... By: Joshua C. Colp (jcolp) 2008-03-11 15:20:24 What about a dialplan variable? By: Digium Subversion (svnbot) 2008-03-25 10:14:23 Repository: asterisk Revision: 110631 U   trunk/CHANGES U   trunk/channels/chan_sip.c U   trunk/configs/sip.conf.sample U   trunk/main/channel.c ------------------------------------------------------------------------ r110631 | file | 2008-03-25 10:14:22 -0500 (Tue, 25 Mar 2008) | 4 lines Add a special dialplan variable to chan_sip which will cause an audio file to be played upon completion of an attended transfer. (closes issue ASTERISK-8972) Reported by: sunder ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=110631 By: Digium Subversion (svnbot) 2008-03-25 10:16:05 Repository: asterisk Revision: 110632 _U  branches/1.6.0/ ------------------------------------------------------------------------ r110632 | file | 2008-03-25 10:16:05 -0500 (Tue, 25 Mar 2008) | 11 lines Blocked revisions 110631 via svnmerge ........ r110631 | file | 2008-03-25 12:18:41 -0300 (Tue, 25 Mar 2008) | 4 lines Add a special dialplan variable to chan_sip which will cause an audio file to be played upon completion of an attended transfer. (closes issue ASTERISK-8972) Reported by: sunder ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=110632 By: Digium Subversion (svnbot) 2008-03-25 12:59:05 Repository: asterisk Revision: 110693 _U  team/group/cdr_backend_ast_str/ U   team/group/cdr_backend_ast_str/CHANGES U   team/group/cdr_backend_ast_str/Makefile U   team/group/cdr_backend_ast_str/channels/chan_iax2.c U   team/group/cdr_backend_ast_str/channels/chan_sip.c U   team/group/cdr_backend_ast_str/configs/extensions.conf.sample U   team/group/cdr_backend_ast_str/configs/sip.conf.sample U   team/group/cdr_backend_ast_str/configs/voicemail.conf.sample U   team/group/cdr_backend_ast_str/include/asterisk/options.h U   team/group/cdr_backend_ast_str/main/app.c U   team/group/cdr_backend_ast_str/main/asterisk.c U   team/group/cdr_backend_ast_str/main/channel.c ------------------------------------------------------------------------ r110693 | tilghman | 2008-03-25 12:58:50 -0500 (Tue, 25 Mar 2008) | 140 lines Merged revisions 110610,110615,110619,110621,110625,110629,110631,110636,110639,110689,110691 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r110610 | file | 2008-03-24 10:28:25 -0500 (Mon, 24 Mar 2008) | 6 lines Only print out the set_address_from_contact host verbose message if debugging is enabled on the dialog. (closes issue ASTERISK-11703) Reported by: rjain Patches:      chan_sip.c.diff uploaded by rjain (license 226) ................ r110615 | russell | 2008-03-24 12:36:04 -0500 (Mon, 24 Mar 2008) | 10 lines Merged revisions 110614 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r110614 | russell | 2008-03-24 12:34:56 -0500 (Mon, 24 Mar 2008) | 2 lines Turn a NOTICE into a DEBUG message. ........ ................ r110619 | mmichelson | 2008-03-24 14:19:37 -0500 (Mon, 24 Mar 2008) | 23 lines Merged revisions 110618 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r110618 | mmichelson | 2008-03-24 14:17:41 -0500 (Mon, 24 Mar 2008) | 15 lines This is a revert for revision 108288. The reason is that that revision was not for an actual bug fix per se, and so it really should not have been in 1.4 in the first place. Plus, people who compile with DO_CRASH are more likely to encounter a crash due to this change. While I think the usage of DO_CRASH in ast_sched_del is a bit absurd, this sort of change is beyond the scope of 1.4 and should be done instead in a developer branch based on trunk so that all scheduler functions are fixed at once. I also am reverting the change to trunk and 1.6 since they also suffer from the DO_CRASH potential. (closes issue ASTERISK-11695) Reported by: qq12345 ........ ................ r110621 | mmichelson | 2008-03-24 15:14:07 -0500 (Mon, 24 Mar 2008) | 11 lines Remove the "Event: registration" header from Asterisk-generated SIP REGISTER requests. rjain points out that RFC 3265 specifies that the Event: header is not a valid header for REGISTER requests and that the "registration" value is not defined at IANA. (closes issue ASTERISK-11702) Reported by: rjain Patches:      chan_sip.c.diff uploaded by rjain (license 226) ................ r110625 | oej | 2008-03-25 05:54:07 -0500 (Tue, 25 Mar 2008) | 6 lines Use the "Server" header when responding to SIP requests. (closes issue ASTERISK-11701) Reported by: rjain Patches:      chan_sip.c.diff uploaded by rjain (license 226) ................ r110629 | file | 2008-03-25 09:39:45 -0500 (Tue, 25 Mar 2008) | 12 lines Merged revisions 110628 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r110628 | file | 2008-03-25 11:37:35 -0300 (Tue, 25 Mar 2008) | 4 lines Add an option (transmit_silence) which transmits silence during both Record() and DTMF generation. The reason this is an option is that in order to transmit silence we have to setup a translation path. This may not be needed/wanted in all cases. (closes issue ASTERISK-9755) Reported by: tracinet ........ ................ r110631 | file | 2008-03-25 10:18:41 -0500 (Tue, 25 Mar 2008) | 4 lines Add a special dialplan variable to chan_sip which will cause an audio file to be played upon completion of an attended transfer. (closes issue ASTERISK-8972) Reported by: sunder ................ r110636 | mmichelson | 2008-03-25 10:41:33 -0500 (Tue, 25 Mar 2008) | 15 lines Merged revisions 110635 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r110635 | mmichelson | 2008-03-25 10:40:33 -0500 (Tue, 25 Mar 2008) | 7 lines When reverting a commit, I accidentally left in this bit which was an experiment to see what would happen. It passed the compile test, and I didn't notice I had left this change in too. So this is a revert of a revert...sort of. ........ ................ r110639 | mmichelson | 2008-03-25 10:44:01 -0500 (Tue, 25 Mar 2008) | 3 lines Oops here too. I need to stop coding for a while... ................ r110689 | tilghman | 2008-03-25 12:40:28 -0500 (Tue, 25 Mar 2008) | 6 lines Update the sample configuration, to use Macro less (since it's now deprecated). (closes issue ASTERISK-11716) Reported by: pprindeville Patches:       bugid-0012293.1.6.patch uploaded by pprindeville (license 347) ................ r110691 | tilghman | 2008-03-25 12:46:34 -0500 (Tue, 25 Mar 2008) | 6 lines Update sample configurations to make virtual hosting more obvious. (closes issue ASTERISK-11414) Reported by: pprindeville Patches:       acme-virtualpbx.1.6.patch uploaded by pprindeville (license 347) ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=110693 By: Digium Subversion (svnbot) 2008-03-25 13:07:22 Repository: asterisk Revision: 110694 _U  team/murf/bug11210/ U   team/murf/bug11210/CHANGES U   team/murf/bug11210/Makefile U   team/murf/bug11210/channels/chan_iax2.c U   team/murf/bug11210/channels/chan_sip.c U   team/murf/bug11210/configs/extensions.conf.sample U   team/murf/bug11210/configs/sip.conf.sample U   team/murf/bug11210/configs/voicemail.conf.sample U   team/murf/bug11210/include/asterisk/options.h U   team/murf/bug11210/main/app.c U   team/murf/bug11210/main/asterisk.c U   team/murf/bug11210/main/channel.c ------------------------------------------------------------------------ r110694 | murf | 2008-03-25 13:07:18 -0500 (Tue, 25 Mar 2008) | 140 lines Merged revisions 110610,110615,110619,110621,110625,110629,110631,110636,110639,110689,110691 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r110610 | file | 2008-03-24 09:28:25 -0600 (Mon, 24 Mar 2008) | 6 lines Only print out the set_address_from_contact host verbose message if debugging is enabled on the dialog. (closes issue ASTERISK-11703) Reported by: rjain Patches:      chan_sip.c.diff uploaded by rjain (license 226) ................ r110615 | russell | 2008-03-24 11:36:04 -0600 (Mon, 24 Mar 2008) | 10 lines Merged revisions 110614 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r110614 | russell | 2008-03-24 12:34:56 -0500 (Mon, 24 Mar 2008) | 2 lines Turn a NOTICE into a DEBUG message. ........ ................ r110619 | mmichelson | 2008-03-24 13:19:37 -0600 (Mon, 24 Mar 2008) | 23 lines Merged revisions 110618 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r110618 | mmichelson | 2008-03-24 14:17:41 -0500 (Mon, 24 Mar 2008) | 15 lines This is a revert for revision 108288. The reason is that that revision was not for an actual bug fix per se, and so it really should not have been in 1.4 in the first place. Plus, people who compile with DO_CRASH are more likely to encounter a crash due to this change. While I think the usage of DO_CRASH in ast_sched_del is a bit absurd, this sort of change is beyond the scope of 1.4 and should be done instead in a developer branch based on trunk so that all scheduler functions are fixed at once. I also am reverting the change to trunk and 1.6 since they also suffer from the DO_CRASH potential. (closes issue ASTERISK-11695) Reported by: qq12345 ........ ................ r110621 | mmichelson | 2008-03-24 14:14:07 -0600 (Mon, 24 Mar 2008) | 11 lines Remove the "Event: registration" header from Asterisk-generated SIP REGISTER requests. rjain points out that RFC 3265 specifies that the Event: header is not a valid header for REGISTER requests and that the "registration" value is not defined at IANA. (closes issue ASTERISK-11702) Reported by: rjain Patches:      chan_sip.c.diff uploaded by rjain (license 226) ................ r110625 | oej | 2008-03-25 04:54:07 -0600 (Tue, 25 Mar 2008) | 6 lines Use the "Server" header when responding to SIP requests. (closes issue ASTERISK-11701) Reported by: rjain Patches:      chan_sip.c.diff uploaded by rjain (license 226) ................ r110629 | file | 2008-03-25 08:39:45 -0600 (Tue, 25 Mar 2008) | 12 lines Merged revisions 110628 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r110628 | file | 2008-03-25 11:37:35 -0300 (Tue, 25 Mar 2008) | 4 lines Add an option (transmit_silence) which transmits silence during both Record() and DTMF generation. The reason this is an option is that in order to transmit silence we have to setup a translation path. This may not be needed/wanted in all cases. (closes issue ASTERISK-9755) Reported by: tracinet ........ ................ r110631 | file | 2008-03-25 09:18:41 -0600 (Tue, 25 Mar 2008) | 4 lines Add a special dialplan variable to chan_sip which will cause an audio file to be played upon completion of an attended transfer. (closes issue ASTERISK-8972) Reported by: sunder ................ r110636 | mmichelson | 2008-03-25 09:41:33 -0600 (Tue, 25 Mar 2008) | 15 lines Merged revisions 110635 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r110635 | mmichelson | 2008-03-25 10:40:33 -0500 (Tue, 25 Mar 2008) | 7 lines When reverting a commit, I accidentally left in this bit which was an experiment to see what would happen. It passed the compile test, and I didn't notice I had left this change in too. So this is a revert of a revert...sort of. ........ ................ r110639 | mmichelson | 2008-03-25 09:44:01 -0600 (Tue, 25 Mar 2008) | 3 lines Oops here too. I need to stop coding for a while... ................ r110689 | tilghman | 2008-03-25 11:40:28 -0600 (Tue, 25 Mar 2008) | 6 lines Update the sample configuration, to use Macro less (since it's now deprecated). (closes issue ASTERISK-11716) Reported by: pprindeville Patches:       bugid-0012293.1.6.patch uploaded by pprindeville (license 347) ................ r110691 | tilghman | 2008-03-25 11:46:34 -0600 (Tue, 25 Mar 2008) | 6 lines Update sample configurations to make virtual hosting more obvious. (closes issue ASTERISK-11414) Reported by: pprindeville Patches:       acme-virtualpbx.1.6.patch uploaded by pprindeville (license 347) ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=110694