[Home]

Summary:ASTERISK-05689: [branch] Bridge two channels via a Dialplan App or an AMI event
Reporter:gnudialer gnudialer (gnudialer)Labels:
Date Opened:2005-11-23 21:24:05.000-0600Date Closed:2007-07-09 21:20:47
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) bridge-1.2.12.1.patch
( 1) bridge-trunk-rev48286.patch
( 2) bridge-trunk-rev-49865.patch
Description:This is probably a piece of crap.  Probably can be done much more efficiently as well.

It does work though.

It's used to bridge another channel into the current channel right in the dialplan.

I basically stole everything from mattf's manager bridge action and uncommented and commented stuff until it worked. (In fact, his name should probably be in the copyright notice instead of mine.)

example usage...

exten => s,1,Set(BRIDGECHAN=sip/tahaitianprincess69_1994-asdf)
exten => s,2,Bridge

That would bridge my internet girlf... I mean a person in an active call with my active call.

It prints some weird garbage to the screen...

Nov 23 22:08:39 NOTICE[21612]: app_bridge.c:98 bridge_exec: Attempting to bridge Local/13053249999@faketelco-fe90,1 with Local/1@onwait-076f,2 Playtone: 0xb62b2e9ct
Nov 23 22:08:39 NOTICE[21612]: app_bridge.c:205 bridge_exec: Channels Bridge/Local/13053249999@faketelco-fe90,1<ZOMBIE> and é·é·ocal/1@onwait-076f,2<ZOMBIE> were successfully bridged

Also prints these errors (even though they don't seem to cause any trouble)

Nov 23 22:08:46 ERROR[21612]: ../include/asterisk/lock.h:324 __ast_pthread_mutex_unlock: app_bridge.c line 255 (bridge_exec): mutex '&tmpchana->lock' freed more times than we've locked!
Nov 23 22:08:46 ERROR[21612]: ../include/asterisk/lock.h:337 __ast_pthread_mutex_unlock: app_bridge.c line 255 (bridge_exec): Error releasing mutex: Operation not permitted
Nov 23 22:08:46 ERROR[21612]: ../include/asterisk/lock.h:324 __ast_pthread_mutex_unlock: app_bridge.c line 257 (bridge_exec): mutex '&tmpchanb->lock' freed more times than we've locked!
Nov 23 22:08:46 ERROR[21612]: ../include/asterisk/lock.h:337 __ast_pthread_mutex_unlock: app_bridge.c line 257 (bridge_exec): Error releasing mutex: Operation not permitted
Nov 23 22:08:46 ERROR[21612]: ../include/asterisk/lock.h:324 __ast_pthread_mutex_unlock: app_bridge.c line 259 (bridge_exec): mutex '&chana->lock' freed more times than we've locked!
Nov 23 22:08:46 ERROR[21612]: ../include/asterisk/lock.h:337 __ast_pthread_mutex_unlock: app_bridge.c line 259 (bridge_exec): Error releasing mutex: Operation not permitted
Nov 23 22:08:46 ERROR[21612]: ../include/asterisk/lock.h:324 __ast_pthread_mutex_unlock: app_bridge.c line 261 (bridge_exec): mutex '&chanb->lock' freed more times than we've locked!
Nov 23 22:08:46 ERROR[21612]: ../include/asterisk/lock.h:337 __ast_pthread_mutex_unlock: app_bridge.c line 261 (bridge_exec): Error releasing mutex: Operation not permitted


****** ADDITIONAL INFORMATION ******

I faxed a disclaimer back in February or something.

p.s. i had to disable "playtones" to get it to work

p.s.s. i know it would make more sense to have the BRIDGECHAN be an arg instead of a channel var.

p.s.s.s. the reason for the "goto" statements is because in matt's it returns in a bunch of different places, but for an app we want to make sure we do LOCAL_USER_REMOVE.. so i just took all the returns and set res and sent them down to the bottom... without the goto's the code would keep executing even if there was an error at the beginning.
Comments:By: twisted (twisted) 2005-11-23 22:56:46.000-0600

I see quite a bit of my code in there too...  

I did notice a few things...

1) we should always unlock something that we call locked.  You're not unlocking the channels once we grab the lock on them if the bridge fails.  This must happen, otherwise you wind up with dead channels laying around, and possibly a deadlocked asterisk.

2) if you're going to include the code to do the tones, at least make that a command line option for the application.  As a side note, there's no reason to init it to a size of 80 if it's only 2 bytes.  There's no reason that disabling it is the only way to make it work - not if we're seizing and bridging the channels properly.

3) Actually, in my code, we would return if we failed...  

4)  You seem to have used my code and not mflorrel's, although they're quite similar.   If you're going to give credit, then it should be to both of us.  I obviously don't mind people taking my code and doing things with it, but if you're going to credit things, give credit where it's due.

5) Don't you think this would be better if merged with the manager event?  Perhaps we can break the bridging code out into res_bridge, and then register both the dialplan app and the manager event into one file...  

6)  don't sprintf messages when running apps from the dialplan.  Bad idea.  Use ast_log() instead.

7) be careful with res.  The return codes in the manager event would be sufficient.   The pbx checks the value of the return to decide what to do.  Generally, we want to set res to 0 unless we want to stop dialplan execution.  

8)  A better alternative than just continuing on in the dialplan (which is what this will do pass or fail) would be to set a result variable, such as we do in most other apps now.  see app_hasnewvoicemail.c for a simple example.

All in all, I think this is a good start, although I can't imagine how we're going to determine what channel to run this app against from within the dialplan..  You must be doing evil things in the dialplan to need this >:)

By: gnudialer gnudialer (gnudialer) 2005-11-23 23:30:49.000-0600

Holy, 8 bullet points; and you were probably going easy on me in reality. :)

1) ok
2) ok
3) ?
4) my bad.. would not do on purpose.. will be more careful in the future
5) i don't care either way, but i have no idea how to do it like you're saying
6) ok
7) don't fully understand, but i'll look into it
8) same answer as 7

last paragraph) exten => x,x,MYSQL(Query asdf SELECT id, channel, server FROM agents WHERE status='ONWAIT' ORDER BY waittime ASC LIMIT 1)

In other words.. can do a distributed/sandboxed dialplan driven (easily fixable) emulation of chan_agent.so/app_queue.so with virtually any feature i can dream up.. also don't have to sit and cross my fingers/do magic spells to keep manager connections working properly.. i could go on and on and on :)

By: twisted (twisted) 2005-11-23 23:42:59.000-0600

Okay, I can see your reasoning for this then ;)

as far as the points you didn't understand:

3)  You're goto'ing and removing the local user on fail.  This is fine, but it more relates to another point, and was kinda useless in the list.

4) it's okay.  if I was mad, you'd know it *wink*

5) This was more a point for discussion.  Currently the bridging functionality is located in res_features.c, and is probaby better off there unless we break it out into it's own module, to allow res_features to be strictly for calling features.  If we break out into res_bridge, and do the bridging control from there, we can link directly with the bridging code, and supply both the manager event and dialplan app from the same location.  

7)  Basically, in the dialplan if we return -1, we disconnect the call. If we return 0 we continue.  if we return 1, i'm not exactly sure what happens.

8) apps can report their status after execution, or jump to a different priority.  You could set a variable based on the action such as BRIDGERESULT with a value of 1 or 0, PASS or FAIL, or whatever.  This way we could branch using a conditional goto from there, and do something else, rather than just moving along.


Hope that made a little more sense..

By: Olle Johansson (oej) 2005-11-24 01:03:58.000-0600

Twisted: Jumping to a different priority is not acceptable for new apps.

By: gnudialer gnudialer (gnudialer) 2005-11-24 02:20:23.000-0600

((these changes are not uploaded yet))

i changed it so it always returns 0

also populates BRIDGERESULT with SUCCESS or FAIL

also got rid of the snprintf stuff (except where they are populating vars in two places)

also made the usage more friendly..

Bridge(sip/blah-asdf)

or

Bridge(sip/blah-asdf|PLAYTONES)

i think playtones should be working now but haven't tested it

only prob now is that i tried to fix the unlocking issues and now it seems to be crashing left and right and i'm having a heck of a time getting a freakin backtrace to work.. i'll just keep hammering away at it

as for merging with res_features discussion... like i said, doesn't really matter to me at all, but IMHO i'd say why not...

1. keep this a separate app
2. put the manager event in manager.c
3. put the bridge stuff in channel.c with function prototypes in channel.h

Either way it's totally your guys' call, just thought I'd throw that out there.



By: gnudialer gnudialer (gnudialer) 2005-11-24 03:50:13.000-0600

app_bridge2.c:

i took out the unlocks at the end for chana/b because those are already unlocked (i think) that seemed to do the trick

i still get some mutex errors, but it doesn't crash and it doesn't have clingers

# transfer and * hangup both seem to work great

the BRIDGERESULT var seems to work fine

i left some commented out code so yall can see what i did to make it not crash

here's the errors i still get...

Nov 24 04:32:08 ERROR[21946]: ../include/asterisk/lock.h:324 __ast_pthread_mutex_unlock: app_bridge.c line 294 (bridge_exec): mutex '&tmpchana->lock' freed more times than we've locked!
Nov 24 04:32:08 ERROR[21946]: ../include/asterisk/lock.h:337 __ast_pthread_mutex_unlock: app_bridge.c line 294 (bridge_exec): Error releasing mutex: Operation not permitted
Nov 24 04:32:08 ERROR[21946]: ../include/asterisk/lock.h:324 __ast_pthread_mutex_unlock: app_bridge.c line 296 (bridge_exec): mutex '&tmpchanb->lock' freed more times than we've locked!
Nov 24 04:32:08 ERROR[21946]: ../include/asterisk/lock.h:337 __ast_pthread_mutex_unlock: app_bridge.c line 296 (bridge_exec): Error releasing mutex: Operation not permitted



By: gnudialer gnudialer (gnudialer) 2005-11-25 20:09:59.000-0600

Verified that it works with touch monitor as well.

By: gnudialer gnudialer (gnudialer) 2005-12-14 20:04:24.000-0600

latest file has major cleanups...

also noticed that it was crashing if the channel didn't exist...

i found that taking out the unlocks (in the case of non-existant channel only) fixed that in a jiffy after which i proceeded to beat the crap out out it

cleaned it up real nice for example, added \n to all the ast_log() calls

also got rid of all compiler warnings

i've made quite a few calls with it with no problems or clinger channels.. this includes complicated situations where it is bridging two Local channels



By: gnudialer gnudialer (gnudialer) 2005-12-16 11:53:21.000-0600

First of all, can someone delete app_bridge.c and app_bridge2.c?

...

((All pathches are against latest SVN Head))

Now we have three working options:

1. We still have app_bridge3.c

2. res_bridge.c, new file which brings the manager event (4297) over to res_bridge with app_bridge

3. res_features.app_bridge.c.patch which puts both the manager event and the app into res_features.c

As far as I can tell, I've addressed every issue concerning this bug, so any feedback or testers would be greatly appreciated.

I would like to reiterate that I'm unaware of any issues regarding these patches.

Thanks!

By: Olle Johansson (oej) 2006-01-04 05:25:43.000-0600

Please mail to the mailing lists asking for testing and feedback here in the bug tracker. We need to move this issue forward.

/Housekeeping :-)

By: Edward Eastman (whisk) 2006-01-04 17:31:27.000-0600

app_bridge3.c tested with Asterisk SVN-trunk-r7221 and seems to work pretty well :)

Used the following dialplan to test, dialling *5 logging in as an agent from a cisco 7940 and *6 from an eyebeam softphone:

exten => *5,1,DBPut(BRIDGETEST/CHAN=${CHANNEL})
exten => *5,n,AgentLogin()

exten => *6,1,Answer
exten => *6,2,SayDigits(1234)
exten => *6,3,DBGet(CHANTOBRIDGE=BRIDGETEST/CHAN)
exten => *6,4,Bridge(${CHANTOBRIDGE})

This bridges the two channels perfectly with no errors/warnings to the console (running with set debug 10 and set verbose 10).

I did manage to break it by doing the following tho:

exten => *6,1,Bridge(${CHANNEL})

And got the following crash:

   -- Executing Bridge("SIP/ed-softphone-c8f3", "SIP/ed-softphone-c8f3|playtones") in new stack
Jan  5 01:23:40 WARNING[8528]: channel.c:3039 ast_do_masquerade: Channel type 'SIP' does not have a fixup routine (for Bridge/SIP/ed-softphone-c8f3<ZOMBIE>)!  Bad things may happen.
Jan  5 01:23:40 WARNING[8528]: channel.c:929 ast_channel_free: PBX may not have been terminated properly on 'Bridge/Bridge/SIP/ed-softphone-c8f3<ZOMBIE><ZOMBIE>'
Jan  5 01:23:40 WARNING[8528]: channel.c:900 ast_channel_free: Unable to find channel in list
x1-6-00-02-b3-4b-61-bb*CLI>
Disconnected from Asterisk server
Executing last minute cleanups
Asterisk cleanly ending (0).


I suspect that it's pretty easy to handle this case so I haven't attached a core although i can do if required.

HTH

Ed

By: gnudialer gnudialer (gnudialer) 2006-01-04 17:37:10.000-0600

Cool, thanks...

I know I tested non-existent channels pretty thoroghly, so I hope it only crashed cuz you tried to bridge to yourself...

If you get a chance, can you try just a nonexistent channel?

exten => x,x,Bridge(somegarbage)

I'll do a fix for the ${CHANNEL} problem as soon as I get a chance.

Thanks,

Heath

By: Edward Eastman (whisk) 2006-01-05 02:46:54.000-0600

Yep - bridging a nonsense channel fails gracefully:

   -- Executing Bridge("SIP/ed-softphone-1747", "asdf") in new stack
Jan  5 10:45:32 WARNING[9984]: app_bridge.c:135 bridge_exec: Channel does not exist: asdf
 == Auto fallthrough, channel 'SIP/ed-softphone-1747' status is 'UNKNOWN'

By: twisted (twisted) 2006-01-05 09:04:26.000-0600

Heath, perhaps a simple check to say if channela == channelb then fire off a warning stating "you can't bridge a channel to itself" and have it exit with a result code such as BRIDGERESULT=FAIL.  Note that my result outputs were simply a suggestion.  I would love to see accurate result codes stating the reason for failure, such as INCOMPATIBLE, DEAD, NONEXISTANT, etc.

By: gnudialer gnudialer (gnudialer) 2006-01-05 16:58:55.000-0600

Yeah I did something like that to my local copy but totally forgot about BRIDGERESULT...

I'll mod all three versions and get them posted as soon as I get a chance (got tied up on the phone all day).

I also have some outstanding questions from the mailing list to address.

By: gnudialer gnudialer (gnudialer) 2006-01-06 17:51:02.000-0600

Latest files add more BRIDGERESULT responses...

SUCCESS
FAILURE
LOOP
NONEXISTENT
INCOMPATIBLE
MISSINGPARAM

Also added appropriate manager events.
Please delete the following...

app_bridge3.c
res_bridge.c
res_features.app_bridge.c.patch

Here's the Manager Action syntax...

Action: Bridge
Channel1: chan
Channel2: chan
Tone: (Yes|No)

I think I'm caught up on this, but if I've had a lot of questions on this lately, so let me know if I'm forgetting anything.

Thanks!

Heath

By: xiribitata (xiribitata) 2006-01-09 11:45:59.000-0600

I've been testing this patch and I saw that you are sending the response together with the Event to the manager API. Wouldn't it be better if sending the response (including the action id to which it refers) separated from the event ? All other manager api actions are doing it ...

By: Paulo Mendes da Silva (kanelbullar) 2006-01-09 11:59:30.000-0600

Xiribitata has a good point here. I have also tested this patch (and also previous versions) and noticed there was no separate response to the Bridge request. It would be more coherent to send a separate response, containing the requested ActionID, as it happens with most AMI requests (Originate, Redirect, etc).

By: gnudialer gnudialer (gnudialer) 2006-01-09 12:05:00.000-0600

Then I'll definately look into it, but I'll warn you right now that I have no idea how to do that.  It can't be THAT hard though.

Can you two comment on your testing?  Have you tested the latest version?  Any issues?

Thanks!

Heath

By: Matt O'Gorman (mogorman) 2006-01-09 12:53:07.000-0600

hello

I was testing this patch as well as the earlier one.  I could not get the application to work in the manager mode however I was able to get 4297 working.  I n your version when it errored out it would create
**Unkown**           s@default:1          Down    (None)
each time.  This is obviously bad as well. I'd be happy to work on who ever is doing this to help get this bug commited.  you can find me on irc as mog_work or mog_home

By: gnudialer gnudialer (gnudialer) 2006-01-10 12:03:16.000-0600

Per my convo with mog*, unless there's any protest, the tentative plan is to go with the res_features version...

When I get a chance, I'm gonna try to track down the discrepancy between this and twisted's patch that may be causing the issue.

By: gnudialer gnudialer (gnudialer) 2006-01-12 06:57:37.000-0600

sorry i haven't been around, been super busy

might not be till this weekend till i can give it more attention, but i'll definately get around to it

By: xiribitata (xiribitata) 2006-01-13 04:13:07.000-0600

Hello,

I am only testing this patch through the manager API and it seems to do what it is supposed to do, despite the thing about not sending the response (with the action id field) separated from the event. I was looking through asterisk code and there is a manager function that sends the response to the manager API: astman_send_response or astman_send_ack . Perhaps this could be used in here. :)

By: Paulo Mendes da Silva (kanelbullar) 2006-01-13 08:22:57.000-0600

I am also only testing this patch using manager. The actual bridging appears to be working fine. The only strange behavior that I have noticed is in the manager scenarios, as I wrote before. When I tested the original mflorell's patch (0004297), the request response scenario was like this:

Action: Bridge
Channel1: SIP/sigrid-69b7
Channel2: SIP/olaf-5a62
ActionID: 86

Response: Success
ActionID: 86
Message: Sending Action Bridge: SIP/sigrid-69b7 and SIP/olaf-5a62

A response was received for the Bridge request with the appropriate ActionID, allowing users to relate the request and the response and to learn if the request was successful or not. This behavior was more consistent with the remaining manager scenarios.

Perhaps astman_send_response is indeed a good option to send the response.

By: Matt O'Gorman (mogorman) 2006-02-01 15:50:28.000-0600

any chance to update heath?

By: gnudialer gnudialer (gnudialer) 2006-02-02 00:29:56.000-0600

New versions of SVN trunk (is that how you say it? not "head"??) spit out a couple of compile warnings which stick out like a sore thumb (never used to do that), but those are pretty easy to fix, so I'll try to get an updated patch out tomorrow sometime.

Other than that, I haven't been able to reproduce any issues.

Amazing that all 5 hunks still succeed on the mammoth res_features :) :)

By: Jerry Fan (tainted) 2006-02-02 15:07:25.000-0600

Just patched a staging asterisk box to play around with this bridge app and I have to say -- twisted, mattf, and heath -- you guys are gods among mortals.  Finally, no more hacking call files, local channels, and dragging MeetMe out just to bridge a damn call.  Thank you thank you thank you!!  What version can we start seeing Bridge()? :D

By: gnudialer gnudialer (gnudialer) 2006-02-03 19:58:17.000-0600

************** Someone please delete the first three files ***************

I found and fixed the problems with the manager action..

chana/b are already unlocked so there's no reason to unlock them at the end

i moved the alloc for tmpchana/b down a bit so that there's no ***Unknowns in case of bad channel name etc (before it was allocating then exiting if the channel name was bad so then * was like "what are these channels?")

I also got the ActionIDs working.

By: gnudialer gnudialer (gnudialer) 2006-02-05 10:34:19.000-0600

Is there anything else I should do before requesting a review?

By: opsys (opsys) 2006-02-08 21:29:49.000-0600

heath1444: What files do you want deleted?

Please specify.

By: gnudialer gnudialer (gnudialer) 2006-02-08 21:31:45.000-0600

app_bridge.c
res_bridge2.c
res_features_with_bridge.patch

Thanks!

By: opsys (opsys) 2006-02-08 21:34:52.000-0600

Files Deleted as per heath144.

By: gnudialer gnudialer (gnudialer) 2006-02-08 22:11:30.000-0600

How do I turn on the "request review" thingy? or does someone else have to do that?  I don't see any options for it.

thanks

Heath Schultz

By: opsys (opsys) 2006-02-08 22:15:40.000-0600

Before I make a 'request' can you make sure that your code passes the coding-guidelines document.  Most of the time formatting errors cause the most delay.

By: gnudialer gnudialer (gnudialer) 2006-02-08 23:29:06.000-0600

Do you mean like timeval/time_t stuff? The patch doesn't use do of that.

Also, I read "Developer Guidelines" and "Bug Guidelines" in their entirety and was not available to identify any formatting issues etc.

Thanks!

Heath

By: boprey (boprey) 2006-02-09 16:12:38.000-0600

I have tried changrab and the Bridge action from the manager api. They both work great except one thing. How am I going to break the bridge and the send the channels back to an extension? I tried 'Redirect'. It only works when sending one channel to an extension and terminate the other. When sending both channels, * report with NewExten event on both channels, but they are still "bridged" together. Anyway out of this?

By: gnudialer gnudialer (gnudialer) 2006-02-09 16:22:43.000-0600

BOPrey have them dial a Local/ channel with the "g" option prior to being bridged.  When you do the bridge, bridge those Local/ channels together.  Then you can hang them up without losing the call, and then do whatever you want with them after that.  It is stable as a rock this way.

Can you verify that you used *this* patch for the Manager bridge?

By: boprey (boprey) 2006-02-09 18:19:26.000-0600

heath1444,

"Hang them up"  do you meann issuing hangup for each channel? How could that not losing both legs of the call?

By: gnudialer gnudialer (gnudialer) 2006-02-09 18:59:08.000-0600

like this...

[agentloggedin]
exten => _X.,1,Dial(Local/${EXTEN}@onwait/n||g) ; <- 'g' is the key here
exten => _X.,2,Wait(5) ; wrapup
exten => _X.,3,Goto(${EXTEN},1)
exten => h,1,NoOp(AGENT LOGGED OUT!)

[onwait]
; Channel will Local/x@onwait-asdf,1 ; or ,2 Can't remember off hand
; Bridge to that channel
exten => _X.,1,Answer
exten => _X.,2,Set(TIMEOUT(response)=300)
exten => t,1,Goto(s,2)
exten => h,1,NoOp(AGENT GOT OFF HER LAST CALL; BETTER BE A SALE OR SHE'S FIRED!)

; now think of all the fun extras you can add to this context... Toggle music on hold via an extension, pause, unpause, check your sales stats, start recording, vote for your favorite American Idol; sky's the limit

See?

That's the beauty of this patch.. 100% control of who's talking to who, and when and where and why and how.

By: boprey (boprey) 2006-02-09 19:05:44.000-0600

Ah, the 'g' option. Thnx.

By: boprey (boprey) 2006-02-10 09:09:12.000-0600

heath1444,

Make some exten simular to the ones you posted. Works great. Big thanks to you. One thing I want to point out is that the 'Answer' priority in '_X' is essential for the internal channels to be created which is important if you want to regain control of both channels of the call. Learnt a lot here.

By: boprey (boprey) 2006-02-10 20:54:46.000-0600

I thought it over again. How much over does it incur doing this way?

By: mike240se (mike240se) 2006-02-15 00:09:00.000-0600

This is great work, I am a little confused how it could be used....  Is it specifically made for manager api?  I dont use any managers, but it looks like this could be useful in a small office enviroment but I am not sure... Could this help with switching calls between users in a small office, instead of parking, like when the call is on hold, you want to pick it up from another desk without having to park....

By: gnudialer gnudialer (gnudialer) 2006-02-15 17:40:10.000-0600

mike240se: I think most of us are looking at it for call center applications, but it can be useful anytime you want to bridge two active calls together.

BOPrey: not sure what you mean; I can tell you that I haven't noticed much difference between this and using chan_agent.

By: Haiden Frank (hai_2k) 2006-02-15 20:56:04.000-0600

Where can i download app_bridge.c and other files that go with it? I am running 1.2.3. I checked on asterisk CVS Trunk. Did not see it there.

By: Henning Holtschneider (hehol) 2006-02-21 05:01:11.000-0600

hai_2k: the patch is attached to this bug. Have a look at the top of the page 8-)

By: King (kho) 2006-02-22 10:14:29.000-0600

I am testing this with 1.2.4 release and is working very well.

I have noticed a couple of issues with this:
1) The event log from the manager api is not formatted correctly. This
is what I got:

Event: Event: Bridge
Response: Success
Channel1: %s
Channel2: %s
Tone: %s

I think the call to manager_event has an extra "Event:" and a missing comma (,).

2) I have uncommented the following lines in features.conf:

xfersound = beep
xferfailsound = beeperr

However, I still cannot get any beep when the call is bridged. I looked at
the code and I can't find out why.

Besides these minor problem, I think this is a very very useful patch.

Thanks!!

By: Brad Templeton (bradtem) 2006-02-24 14:49:44.000-0600

It seems to bomb on actionids, segfaulting here:

if (useactionid) {
               strcpy(sActionid,strcat(strcat(strcat("","ActionID: "),actionid),"\r\n\r\n"));

This code makes no sense -- you can't strcat into  a string constant!  Even the use of fixed width buffers is discouraged here (use strncat in that case)

I don't know the asterisk code base very well but surely there are some libraries for handling and returning the actionids?

I'm trying to use this from the manager interface, in asterisk-java.  It reminds me why I stopped coding in C :-)

By: Brad Templeton (bradtem) 2006-02-24 14:58:04.000-0600

Oh yeah, changing that strcpy(sActionid,strcat(strcat(strcat...  line to:

snprintf( sActionid, sizeof(sActionid), "ActionID: %s\r\n\r\n", actionid );

Seems to work and do what I think you wanted with the strcats.  It now does a merge from the manager interface though I don't think I'm getting the response back right.  No segfault at least.

By: gnudialer gnudialer (gnudialer) 2006-02-24 15:16:37.000-0600

Yeah I always manage to screw up string manip in C, so I really appreciate it.

Do you think your fix fixes kho's problem?

By: King (kho) 2006-02-25 11:57:51.000-0600

heath1444:
I found out what is causing my problem and possibly bradtem problem with manager response.

(1) Need to change all the manager_event from:
manager_event(EVENT_FLAG_CALL, "Event: Bridge\r\n"
    "Response: Success\r\n"
    "Channel1: %s\r\n"
    "Channel2: %s\r\n"
    "Tone: %s\r\n"
    "\r\n",
    channela, channelb, playtone ? "Yes" : "No");
to
manager_event(EVENT_FLAG_CALL, "Bridge",
    "Response: Success\r\n"
    "Channel1: %s\r\n"
    "Channel2: %s\r\n"
    "Tone: %s\r\n"
    "\r\n",
    channela, channelb, playtone ? "Yes" : "No");

(2) Delete this line:
      static char xfersound[256];
I think it is overriding the one that is read in from the config file.

(3) Change the variable tmpchana to tmpchanb in the following line in bridge_exec (I think the action one is OK):
              if (!ast_strlen_zero(xfersound) && !ast_streamfile(tmpchana, xfersound, tmpchana->language)) {
                      if (ast_waitstream(tmpchana, "") < 0)

I am now having the "beep" and the manager event output is OK. I am only using
bridge_exec though.

By: Brad Templeton (bradtem) 2006-02-26 19:46:58.000-0600

Some more notes.  I have got it doing bridging using manager interface, but I get rather unusual activity on the events after a bridge.  

First I originate calls to two phones, one IAX2 the other SIP.  They are playing dialplan scripts and then I merge them when the scripts both reach a certain point.  This is for click-to-call, in that both phones get an announcement and then are bridged.

First, I get a newChannel event, creating a new channel called 'Bri' (state up)
Then I get a series of odd rename events.    First the iax2 channel foo-5 gets a rename event with newname ->  iax2/foo-5<MASQ>.  

Then another rename, old channel bri -> iax2/foo-5
Then anotehr rename, old channel iax2/foo-5<MASQ> -> Bri<ZOMBIE>
Then a hangup event on bri<zombie>

Then this whole process repeats for the SIP channel.

Mind you, the channels appear to continue to exist under their original names.

What does all this mean, and is it what we should expect?  Is this only because connecting SIP to IAX needs audio routed through asterisk? I also get it connecting two sip phones (which have reinvite=no because they are behind nat).

Second point:

It's important to me that when I bridge two calls like this, that I control the hangup of either end, and can do things with the other phone that was not hung up.   Ie. if one person hangs up, I want to redirect the other line to some new dialplan scripts.  (For example, the way a calling card works.)  One could do this using the meetme bridge but of course the whole point of this is to not use the meetme, since it sucks bandwidth and adds latency connecting remote endpoints who could have the audio bridged between them.

By: Brad Templeton (bradtem) 2006-02-27 19:50:12.000-0600

I now see from the code that for better or worse, the creation of the masquerade channels and other stuff is deliberate, though I don't know enough about * to know why this is needed.    Direct channel connection is found in other areas of the system (Agent Queue, Call Park, Attended Transfer), does it also use these techniques?

I've been thinking more about the problem for a bit and the apps I have in mind, and wrote up some of the goals I think are important.  You may want to consider them...

http://www.templetons.com/tech/merge.html

By: gnudialer gnudialer (gnudialer) 2006-02-28 13:34:38.000-0600

The whole masq channel business came into play before I got involved with this app when it was just the manager action.  I had to take it out for the dialplan app because it seemed to create several catch 22s with locking/unlocking.

Your page makes sense and seems well thought out; please keep in mind that controlling what happens on hangup can be controlled via some creative dialplan logic (see some of the above posts).

I think for now our best bet is to keeping trying to get the current functionality into SVN.  I'll do a patch with you and kho's fixes as soon as I get a chance.

Thanks,

Heath Schultz

By: Brad Templeton (bradtem) 2006-02-28 14:21:35.000-0600

Forgive my ignorance, but the "g" option is for the Dial command, and causes it to fall to the next priority in a dialplan.

This merge commany, I presume, is primarily for use in Manager applications (it seems quite convoluted in dialplan apps needing access to live channel names as it does) and they start calls with the Originate manager action, and connect them to a dialplan script.   Is there a way to get the equivalent of the "g" option in Manager created calls.   Think click-to-call -- manager program calls two phones, talks to them, then joins them.

By: gnudialer gnudialer (gnudialer) 2006-02-28 14:30:58.000-0600

heh,

yeah...

redirect them to an exten that dials a local channel with 'g' , then bridge :)

Heath

By: gnudialer gnudialer (gnudialer) 2006-02-28 14:41:07.000-0600

as in "Redirect" action.. realized that wasn't clear

By: Brad Templeton (bradtem) 2006-02-28 15:03:33.000-0600

Alas, not yet too clear.  The masqueraded channels are perhaps confusing me.

Originate connects a channel to a dialplan extension.  It's like the phone picked up and called that extension.   I can redirect the channel to other extensions with a redirect action (not sure what difference that makes over the original extension I sent them to with the originate action).   Either way, the extension can then attempt a dial to a local channel with yet another extension in it, which would, I presume, create a bridge on its own between the local channel and the phone's channel.

My understanding, perhaps poor, was that the Bridge action rips the phone channel away from the extension it is executing in the dialplan and connects it to the other bridge channel, which is similarly ripped away from what it's doing.

Are you suggesting I connect the phone's channel to a local channel (using dial with "g") and then bridge the local channel to something?  To the local channel the other phone was connected to?  To the other phone's channel?

And if I do this, will all the extra channels vanish, so that the two phones are directly connected, and their audio will flow directly between them rather than through the PBX?   (remember, the whole point of this, instead of a 2 person meetme room, is to make that happen.)

By: King (kho) 2006-03-05 12:37:10.000-0600

Anyone know how I can do an attended transfer from a bridged channel without hanging up the call?

By: Brad Templeton (bradtem) 2006-03-13 03:50:15.000-0600

I am just delving into Asterisk but I still can't make sense of the code here, and how much it's been tested.

From what I can see, an action should return a response on the manager interface, and may also generate managerevents for more asynchronous results, or due to an executed appl.

Looking at action_bridge, it seems to somtimes return in error conditions with no response on the manager channel (which would cause timeouts I suppose).  When it does do a response, it calls manager_event rather than ast_cli which most other actions seem to call.  It calls it with a long string in the event parameter rather than the event name, and then a series of printf strings are in the event parameter, rather than in the format string.   A comma is missing and there are extra newlines -- can this code have been even tested at all?

I would really like this function but it seems that perhaps a more experienced asterisk coder needs to look at doing it?   Perhaps it can be done without all the temporary channels and masquerading as well.  I am trying my own patches but this is my first dive into asterisk.

By: Brad Templeton (bradtem) 2006-03-13 03:57:03.000-0600

Let me add I would be willing to pay a bounty for a well implemented bridge action, especially one that does some of my other goals (DTMF handling, independent hangup etc. as noted at
http://www.templetons.com/tech/merge.html

If you might want to take that on, send me a note (btm@templetons.com) to work out a suitable bounty price.  Others who might like to add to the bounty can also contact me.

By: opsys (opsys) 2006-04-05 12:13:20

/HOUSEKEEING/

We get an offer for a bounty and it stalls??? :-)

Has anyone picked this up lately?

By: gnudialer gnudialer (gnudialer) 2006-04-07 13:45:17

Under insane load it would seg if you tried to bridge two channels to the same channel at almost exactly the same time.  Under normal circumstances, this is almost impossible, but in a high load call center environment, it can and did happen.

I have a version that can get it to bail in this case (catches error from ast_do_masquerade and bails).  It also has kho's and bradtem's fixes...

I'll get it up ASAP.

With the fix in it can handle hundreds of bridges at the same time.

Heath

By: Brad Templeton (bradtem) 2006-04-07 14:13:47

There's more I have had to patch before I gave up.  A manager action needs to have a manager response, usually sent with astman_send_ack() or astman_send_error().  If it's an async action, it can always ack and later send a manager_event() with the real result.    This code as originally done did not send a response to the action.  I don't know how quickly you need to send the response, but you do have a lock on the manager stream while running an action, I believe.

In addition, there are things in the code that I am pretty sure are scary such as
snprintf((char *) tmpchana->name, sizeof(tmpchana->name), "Bridge/%s", chana->name); which does an sprintf into a char *!  It's not going to overflow, because the sizeof a char* is going to be 4, but nor is it going to work if I read this right.   The code has many other examples of this.

I made mods to remove the hangup and they don't reliably work.

Anyway, word is that internal asterisk developers are going to do a bridge action, but not until the summer.

However, if I could ask...

What purpose do the masquerade channels serve?  Why can't we just bridge the real channels?   I know this is meant primarily for call transfer.  How does the  use of masquerading affect the crucial issue of whether the audio is hairpinned through the pbx?

By: Serge Vecher (serge-v) 2006-05-04 10:24:44

any progress on development here?

By: Serge Vecher (serge-v) 2006-05-26 11:43:00

the deadline to get this is coming up very quickly. If no updated patch against latest trunk is not made in the next couple of days, this feature will unfortunately not make it into 1.4 ...

By: Brad Templeton (bradtem) 2006-05-29 02:14:32

My understanding is that another implementation of this functionality is underway for 1.4

By: Serge Vecher (serge-v) 2006-06-01 15:19:14

bradterm: I'm not sure that is the case. Do you have a more specific information, i.e. who is working on this ...

By: Brad Templeton (bradtem) 2006-06-07 20:47:02

Check with Kevin Fleming

By: Matt King, M.A. Oxon. (kebl0155) 2006-06-16 14:49:54

For consistency, this feature also requested on the asterisk-users and asterisk-dev lists today.

It does seem odd that a system as advanced as Asterisk has no simple/clean way to connect two calls.

By: Serge Vecher (serge-v) 2006-06-16 15:18:14

well, as kpf has indicated on the dev-list, I believe this is waiting for an updated patch that has no segfault issues reported in 0043920. Thanks for your patience, guys.

By: Stefan Larsson (stela) 2006-06-17 10:06:12

The line
 strcpy(sActionid,strcat(strcat(strcat("","ActionID: "),actionid),"\r\n\r\n"));
causes a segfault, appending to the constant string "" with strcat doesn't work.
 strcpy(sActionid, "ActionID: ");
 strcat(sActionid, actionid);
 strcat(sActionid, "\r\n\r\n");
instead will work somewhat better. Preferably it should also guard against a buffer overrun due to a too long actionid. I'm not sure if snprintf or strncat is the best solution.

To me it seems that the sprintf(...sizeof(...)) as mentioned in (0043925) really does work as intended, the sizeof argument is a char array inside a struct, the array has a fixed length and is part of the struct. I made a tiny test program with a similar sizeof, and it returns the proper length.

(0043925) is correct about the missing manager acknowledge/error, without it e.g. asterisk-java will timeout waiting for a response to a manager bridge action. I added one to both branches of the if statment after ast_channel_make_compatible() in the action_bridge function, and then it works fine.

By: gnudialer gnudialer (gnudialer) 2006-06-20 19:55:15

Stela's changes sound right (although I don't think it's a good idea to go back to that crazy syntax that I had in there earlier.)

The seg issue is like this...  Two channels have to bridge at almost exactly the same time.  *Iff* one starts executing before the other finishes, the possibility of a seg exists.  Other than that, it should fail gracefully.

The only thing I can think of to fix it is to lock the whole damn thing.

I never fixed the code because I made sure outside the dialplan in my own app that it wouldn't try to bridge the same channel.

By: Serge Vecher (serge-v) 2006-06-28 14:33:13

I'm marking this as [post 1.4]. Hopefully this can get into the trunk once those segfault issues are worked out, so this can be merged into trunk.

By: gnudialer gnudialer (gnudialer) 2006-07-12 02:01:32

I don't know how to do/don't have time to do the synchronization properly.

This is probably quite an easy fix for a more involved developer.

By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2006-07-25 18:18:16

The unlocking of tmpchana and b at the end of the the manager function seems wrong, you unlock them earlier.

By: davetroy (davetroy) 2006-08-06 10:43:36

I had trouble applying patch file as is to a current SVN trunk so I applied it by hand and made a new diff to bring this patch up to date.

By: Serge Vecher (serge-v) 2006-08-07 09:28:11

davetroy: thanks for the update here. Can you please follow the svn diff procedure when making a patch against svn trunk (see http://www.asterisk.org/developers/Patch_Howto)? Also, please always report your disclaimer status when posting patches in somebody's report. Thanks!

By: davetroy (davetroy) 2006-08-18 11:24:36

Heath --

With this Local channel setup is there any way to have the agents listen to MOH instead of dead air?

I was experimenting with it but it seems to get pretty wacky... I would end up with both the agent and the called channel listening to MOH separately, which is clearly nuts.

[onwait]
; Channel will Local/x@onwait-asdf,1 ; or ,2 Can't remember off hand
; Bridge to that channel
exten => _X.,1,Answer
exten => _X.,2,Set(TIMEOUT(response)=300)
exten => t,1,Goto(s,2)
exten => h,1,NoOp(AGENT GOT OFF HER LAST CALL; BETTER BE A SALE OR SHE'S FIRED!)

By: Ulrich Holeschak (uholeschak) 2006-08-25 02:19:38

I have upgraded from asterisk 1.2.10 to asterisk 1.2.11 and found, that the bridge patch is not working properly any more in the new version.

If i bridge two sip channels (one is alaw the other g729) i get many error messages which say:

Aug 24 23:44:54 WARNING[841] chan_sip.c: Asked to transmit frame type 64, while native formats is 8 (read/write = 64/64)

I tried both patches ( res_features.c.bridge.patch and res_features.c.bridge.patch-20060806) but both give the same results.

By: Serge Vecher (serge-v) 2006-08-25 08:47:13

uholeschak: this is a new feature, so it will never be applied to the release branch (1.2). Please only test patches against trunk and do not test this feature on any release code, unless you are prepared to do so at your own risk. If you choose the latter, please do not use the bug-tracker to ask for help with the patch. Thanks.

By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2006-09-30 14:46:41

I just uploaded tr_app_and_ami_bridge.diff

I combined the two code paths, removing a lot of duplicated code. I cleaned up the code a little, fixed some calls to the manager, and removed some double unlocks.

This is currently untested by me (except that it compiles [it compiles, ship it!]), and I may not get a chance to test it until Monday. But you all are welcome to test it for me, hopefully there's no stupid segfaults.

By: Serge Vecher (serge-v) 2006-10-02 08:54:53

tim_ringenbach: Thank you! Please confirm you disclaimer status ...

By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2006-10-02 09:04:01

I disclaim this patch, I'm working for Asteria and Asteria has a disclaimer on file.

By: Moises Silva (moy) 2006-10-02 13:54:43

mmm... a couple of comments about tr_app_and_ami_bridge.diff ...

I still see this lines that will surely segfault.

+ if (useactionid)
+ strcpy(sActionid,strcat(strcat(strcat("","ActionID: "),actionid),"\r\n\r\n"));

 Somebody else has suggested, to use sprintf() instead. Actually I backported this to 1.2.12.1 with snprintf and have working action_bridge() just fine.

I just tested today exec_bridge() for dial plan Bridge() application, but it seems is not returning after the bridge to dial plan execution. I think anyone will have this same problem even using SVN trunk because after masquerading on original channel, the channel is flagged as ZOMBIE by ast_do_masquerade(), and in the patch is still used for calls to pbx_builtin_setvar_helper(chan, "BRIDGERESULT", "SUCCESS").

 Does this should be working at all? if chan have been masquerade ,ie, into tmpchanb, I think that should be replaced by pbx_builtin_setvar_helper(tmpchanb..) ??

 Finally tmpchana and tmpchanb shouldnt be hanged up always, as currently done at the end of do_bridge(). Since the caller of Bridge() from the dialplan should return to execution of extensions.

 Im fixing this in 1.2.12.1 because I have to use it there. I will wait any comments before trying to fix it into trunk.

Regards

By: Moises Silva (moy) 2006-10-03 18:47:31

New patch: bridge-1.2.12.1.patch, I know this is a new feature and I must post patches for SVN trunk. But i think it will apply without too much changes to trunk, and for now I just have testing asterisk boxes with 1.2.12.1, so is easier for me to test and contribute.

All the other patches posted before did not returned the bridged channels to the original location where they were before some other channel masquerade into them. Im now calling ast_pbx_start() when the bridge end to return the live channels (if any) to the PBX, to the same context, exten and priority + 1 they were before. Suggestions? may be letting the user decide where they want to go after bridge with some arguments? To do this when Bridge is called from the manager,

+ bridge_exec no longer masquerades 2 channels, since is not needed at all. It only needs to masquerade into the channel specified in the application arguments. The other channel is already in the application because is executing the application.

+ removed some "goto"'s in favor of "return".

+ new member to the structure ast_bridge_thread_obj named "int return_to_pbx", meant to be set to 1 when is desired go back to the PBX after the bridge.

+ added AST_DECLARE_APP_ARGS and AST_STANDARD_APP_ARGS for arguments handling into bridge_exec().

+ Corrected the use of manager_event() and Renamed event from Bridge to BridgeExec

I think that is most of it. Please comments. If no one have the time to adapt the patch for trunk, I will do it, im just waiting for the weekend to do it and test it.

Regards

By: Brad Templeton (bradtem) 2006-10-03 19:07:11

Certainly the less masquerading the better.  My own take is this is far more likely to be used from the manager interface, where the masquerading and renaming of channels is largely a pain, than from a dialplan interface, where you would need to jump some hoops to get the identity of the second live channel you're briding your existing channel to.

Before working form the code base here, check for improvements.  The original stuff was written by somebody who had clearly not coded C before and was full of things like strcpys into fixed length buffers and the like.

From a manager interface standpoint, you want to take live channels you are already managing, bridge them, and then be informed of any events after that (such as one of the channels disconnecting, or, if possible DTMFs generated on channels) ideally under the same channel name.

By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2006-10-04 09:42:39

OK....but that ignores the combined code path in my patch tr_app_and_ami_bridge.diff. Are you not interested in that, or I should attempt to  include your changes in my version and go from there?

By: Moises Silva (moy) 2006-10-04 10:00:31

bradtem:
I agree. However I have a patch known as MAGI() that allows execution of dial plan applicacions and any AGI command thorugh the manager interface, so is like a dynamic dial plan depending on our manager system. Thats why I was interested in Bridge() returning to the dial plan.

Improvements? hum, im kind of newbie in C too :p ... ( but no so newbie to miss the strncpy stuff in static buffers ), so I hear suggestions :), anyway, the code is working so well for me, but I would like other people to test it.


>> From a manager interface standpoint, you want to take live channels you are >> already managing, bridge them
Yeah, thats right, and already can be done :)

>> then be informed of any events after that (such as one of the channels
>> disconnecting, or, if possible DTMFs generated on channels) ideally under
>> the same channel name.

Also can be done, anything else?

Actually im testing this with a project were working on, we havent made a public release because we want a consistent API, but you can look at what we have here:

http://opencallmanager.ivsol.net/

Im heavily rewritting the API, so lots of things may be broken :p, but there is a good documentation about how it works.

----------------------

tim_ringenbach:

 Im very sorry, i just ignored your changes because I had already a working system with the older patch, but I think you basically merged the parts where the 2 masqueradings ocurr. Thats no longer valid because app_bridge only do 1 masquerade, and the manager action 2 masquerades. But sure im missing stuff that can be done with less code :), i will be happy of hearing your suggestions.

Regards

By: jmls (jmls) 2006-11-02 12:12:06.000-0600

ping. are we able to make any more progress on this ? Thanks.

By: Moises Silva (moy) 2006-11-02 14:17:56.000-0600

jmls:

Just give me a few days, im going to try to port the patch I have for 1.2.12.1 to trunk in the hope this is accepted, this issue has almost one year here

Regards

By: Moises Silva (moy) 2006-12-03 20:51:46.000-0600

I have uploaded a new patch for trunk. Can anyone test it please? I will have access to some test server until the next weekend, but hopefully some one else have an available server to try it out.

By: Tony Mancill (tmancill) 2006-12-05 20:16:36.000-0600

I'm running the patch against trunk, and initially I couldn't get two SIP calls to to bridge (one inbound, the second generated by Originate via the AMI - not sure if that matters).  The bridge would fail, complaining that it couldn't make the channels compatible, which seemed odd because ast_channel_make_compatible() wasn't issuing any warnings.

After modifying how the patch compares the result of ast_channel_make_compatible() in bridge_exec(), it's working in my environment.

I'm not sure if it's better to submit the entire patch with the change, or just the interdiff between the patches.  For the time being, I've done the latter (see: bridge-trunk-rev48224_bridge_exec.interdiff).

More testing to come.

By: Moises Silva (moy) 2006-12-05 23:18:27.000-0600

tmancill:

Yep, it seems I made a mistake there. Also I think is better if you post the whole patch with that little fix you did.

By: Tony Mancill (tmancill) 2006-12-05 23:42:24.000-0600

Okay, the entire patch including the updated comparison has been uploaded as bridge-trunk-rev48286.patch.

By: Moises Silva (moy) 2007-01-03 13:09:27.000-0600

Bump! god, this have more than 1 year here, can we do something else to help?

By: Serge Vecher (serge-v) 2007-01-03 13:14:13.000-0600

moy: the best thing to do, still, is testing, reporting results, and ironing bugs out. I see one successful report by tmancill, but for a patch this big we need several successful reports. Keep 'em coming!!!

By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2007-01-03 13:21:09.000-0600

My problem is I've been using a 1.2 version of this patch for a while, but I haven't had any time to test it against HEAD, or even 1.4.

By: Ulrich Holeschak (uholeschak) 2007-01-03 13:21:49.000-0600

You want positive results?
I am working with the patch bridge-trunk-rev48286.patch
and some older versions with no problems for quite a long time now.

By: Moises Silva (moy) 2007-01-03 13:26:23.000-0600

serge-v: Ok, you are right, I will try to get more people involved in testing this.

uholeschak: Good news!, now 2 people ( you and me ) confirm positive results so far with the latest patch.

By: Tony Mancill (tmancill) 2007-01-03 13:40:44.000-0600

I can report (reiterate) a few more positive results.  First, the patch applies cleanly to the 1.4.0 release.  Also, somewhere between 1.4.0-beta3 and the 1.4.0 release (sorry, I don't know exactly when), the behavior of the bridge improved - i.e. either channel will survive the hangup of the bridged channel.  (Before, the channel initiating the bridge would die if the bridged channel hung up first, but the bridged channel could continue.  Now either channel can continue after falling out of the bridge.)

My testing has been solely with SIP, typically bridging an inbound call to an outbound call originated over AMI.  The test app included invoking MixMonitor() on the channel invoking the bridge.  Aside from the issue with hangups early on, the bridge has worked flawlessly.  I have not done any sort of stress testing on it, nor has the application be used in production (yet).

By: Serge Vecher (serge-v) 2007-01-03 13:53:14.000-0600

ok, guys, I've mentioned this before in 0050766 -- test results are for trunk only.

By: Moises Silva (moy) 2007-01-03 14:09:44.000-0600

known issue:

So far does not work with Agent channels. so a call like this:

Bridge(Agent/something)

WILL fail. Im requesting comments on asterisk-dev to see how implement that.

By: Serge Vecher (serge-v) 2007-01-03 14:50:32.000-0600

while we are at it, let's fix the comments/text:
1. /* Find out wether were supposed to play tone or not */ ->
/* Find out whether we're supposed to play a tone or not */
2a.  snprintf(buf, sizeof(buf), "Channel does not exists: %s", channela); ->
snprintf(buf, sizeof(buf), "Channel1 does not exist: %s", channela);
2b. snprintf(buf, sizeof(buf), "Channel does not exists: %s", channelb); ->
snprintf(buf, sizeof(buf), "Channel2 does not exist: %s", channelb);
3. ast_log(LOG_WARNING, "Failed play courtesy tone on chan %s\n", tmpchanb->name)
ast_log(LOG_WARNING, "Failed to play a courtesy tone on chan %s\n", tmpchanb->name)
4.
+ ast_log(LOG_WARNING, "Unable to spawn new bridge thread on %s and %s: %s\n", tmpchana->name, tmpchanb->name, strerror(errno));
+ astman_send_error(s, m, "Unable to spawn new bridge thread");
-> "Unable to spawn a new bridge ..."
5. "The current channel is bridged to the specified channel." ->
The current channel is bridged to the specified CHANNEL.\
6. +"Adding PLAYTONE will give an audio notice of the bridge.\n" ->
"Adding PLAYTONE will play a courtesy audio tone on CHANNEL when bridged successfully."
7. "Bridge require at least 1 argument specifying the other end of the bridge"
  -> A required CHANNEL argument is missing
8. BRIDGERESULT will contain SUCCESS, FAILURE, LOOP, NONEXISTENT or INCOMPATIBLE. -> "BRIDGERESULT dialplan variable ..."
9. "Channel %s does not exists or cannot get its lock, cannot Bridge()" ->
  Bridge failed because channel %s does not exist or we cannot get its lock.
10. + /* now we have 2 valid channel to bridge, is jsut matter of setting up the bridge config and start the bridge */ ->
/* we have 2 valid channels to bridge, now it is just a matter of setting up the bridge config and starting the bridge */
11. /* the bridge has end, set BRIDGERESULT to success, and if the other channel has not been hangup, return it to the PBX */ ->
"/* the bridge has ended, set BRIDGERESULT to SUCCESS. If the other channel has not been hung up, return it to the PBX */
12. + ast_log(LOG_DEBUG, "hang up chan %s since the other endpoint has hung up\n", final_dest_chan->name);
+ ast_hangup(final_dest_chan);
13.+ ast_manager_register2("Bridge", EVENT_FLAG_COMMAND, action_bridge, "Bridge to live channels", mandescr_bridge);
-> "Bridge two channels already in the PBX."
14.
in the following code block, should the second block refer to tmpchanb/chanb?
+ if ( tmpchana )
+ do_bridge_masquerade(chana, tmpchana);
+ else {
+ astman_send_error(s, m, "Unable to create temporary channels!");
+ ast_hangup(tmpchana);
+ ast_hangup(chana);
+ return 1;
+ }
+
+ if ( tmpchana )
+ do_bridge_masquerade(chana, tmpchana);
+ else {
+ astman_send_error(s, m, "Unable to create temporary channels!");
+ ast_hangup(tmpchana);
+ ast_hangup(chana);
+ return 1;
+ }

15. Shouldn't we use final_dest_chan instead of chan here?
+ if ( playtone && !ast_strlen_zero(xfersound) )
+ if ( !ast_streamfile(chan, xfersound, chan->language) )
+ if ( ast_waitstream(chan, "") < 0 )
+ ast_log(LOG_WARNING, "Failed to play courtesy tone on %s\n", chan->name);



By: Moises Silva (moy) 2007-01-07 22:56:58.000-0600

bridge-trunk-rev-49865 is the new patch with comments fixes and the small error on tmpchan.

serge-v, can you delete the other patches, except for the one for 1.2.12.1, that still could be usefull for other people.

im testing this tomorrow, hopefully more people can start testing as well.

By: Serge Vecher (serge-v) 2007-01-09 16:12:29.000-0600

an interesting development on the topic ...
http://lists.digium.com/pipermail/svn-commits/2007-January/020425.html

By: Leif Madsen (lmadsen) 2007-01-25 21:28:53.000-0600

This looks super useful for the ability to simulate call parking in the dialplan as I have done recently. I've been using MeetMe() to bridge the channels together, which has the limitation of not being able to park the call after picking up the parked call.

If I can bridge the two channels together directly instead of using a MeetMe(), I should be able to avoid the <ZOMBIE> channel I am getting after transferring the user to a Queue() for the parking.

Excited to play tomorrow :)

By: Leif Madsen (lmadsen) 2007-01-29 14:59:20.000-0600

I finally got around to play with this.

I report success applying the latest patch to today's checkout of 1.4 (r51360) with only a few lines of fuzz (line offset).

I test the dialplan application (not the AMI stuff). It works great here thus far in preliminary testing.

I am using this in my dialplan to simulate multi-lot call parking, and this is much nicer than the way I was doing it before with Queue() and MeetMe(). Plus now I get music for the parked user! (Looks like I had the same issue as Dave Troy in that if I transferred a user into a Queue(), I didn't get MoH).

I'm basically doing the following:
* Receive call from outside
* Attended transfer to *700 which does a Dial(Local...), and calls MusicOnHold() application (it then writes the channel name into the DB via func_odbc, assigning it to a parking space).
* I can then go to another phone and dial *701. This will then lookup in the DB the channel associated with that parking space (701).
* I then call Bridge(${PARKED_CHANNEL}) and the calls are bridged together.

Another happy customer :)

By: Serge Vecher (serge-v) 2007-02-01 14:08:39.000-0600

blitzrage: do you mean you've applied a patch to trunk ;) ?

By: Leif Madsen (lmadsen) 2007-02-06 11:19:57.000-0600

serge-v: I applied the trunk-rev-49865 patch to 1.4 SVN (it still seems to apply cleanly). I did my testing in 1.4.

Perhaps we could get a copy on SVNcommunity for those of us using 1.4? I guess it's up to the original author to determine if they'd like to support such an effort.

Either way, in my testing of the dialplan aplication, it works great for me thus far.

By: Leif Madsen (lmadsen) 2007-02-19 16:13:40.000-0600

Can we get a code review and move this forward? This functionality it incredibly powerful! Can anyone else confirm that this works for them? I have tested (and am using) the dialplan application. Does someone want to verify the AMI side of it?

By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2007-02-19 17:24:00.000-0600

Unfortunately I can only comfirm that the manager version works with 1.2, I haven't had time to test with trunk, and luckily haven't needed to put any customers on trunk.

By: philipp2 (philipp2) 2007-02-19 22:25:59.000-0600

Works well here on 1.4.0 (tested dialplan only, not AMI)

By: Serge Vecher (serge-v) 2007-02-20 11:00:36.000-0600

ok, let's put the new "ready for testing" status flag to work. Just to be clear, while the 1.2.x/1.4.x are helpful, it is ultimately the trunk tests that carry the most weight.

By: Francisco Seratti (boch) 2007-02-28 16:29:21.000-0600

I have no audio when bridging two SIP channels on Asterisk 1.2.12.1. Could someone helpme to find the problem? My email/msn is fseratti@yahoo.com.ar. Thanks.

By: Matt King, M.A. Oxon. (kebl0155) 2007-03-01 06:19:13.000-0600

I also have no audio bridging SIP channels.  Asterisk 1.2.13.

By: Leif Madsen (lmadsen) 2007-03-01 08:03:41.000-0600

The patch for 1.2.x is quite a bit older than the latest one (01-07-07 -- my bday!).

Since this is a new feature, it will not go into the 1.2.x (or even 1.4.x) series of Asterisk.

heath1444: are you still monitoring this at all? Should the first 2 patches be removed from this bug?

By: Moises Silva (moy) 2007-03-01 11:34:26.000-0600

this is not a proper plase to request support or notify about failures in this experimental feature for the 1.2 branch. Please only post SVN trunk failures or success.

if you are interested in a backport for 1.2, please contact me at moises.silva at gmail dot com

I have a patch for 1.2.11

By: Francisco Seratti (boch) 2007-03-09 19:34:14.000-0600

I could bridge both channels, but when "breaking the bridge" (using Redirect manager command) the channel which ran Bridge() app looses the audio, and when it dies doesnt run 'h' ext. How is the proper way to break a bridge()? Does bridge() returns and dialplan continues? I need to keep initial both channels up after breaking the bridge, unless someone hangs up. Im running asterisk 1.4.1.
Please i need support, here/irc/msn <fseratti@yahoo.com.ar>, i would appreciate it very much.

By: Andrew Thompson (andrewt) 2007-04-02 11:26:49

I've tested the AMI side of things and it seems to work well, one issue I've encountered is that both sides of the channel must not be bridged to anything else before they're bridged. Currently I'm working around this by some creative use of Park, is there a better solution?

By: Moises Silva (moy) 2007-04-03 00:16:33

andrewt:

I have just tested the AMI command with 2 channels on bridge with success. Please detail  how are you testing.

Here is the log of my test:

*CLI> core show version
Asterisk SVN-trunk-r59725 built by root @ lenovocore on a i686 running Linux on 2007-04-03 03:58:36 UTC
*CLI> core show channels
Channel              Location             State   Application(Data)            
IAX2/64.79.194.86:45 (None)               Up      Bridged Call(SIP/moises-081da8
SIP/moises-081da870  123@waitforbridge:2  Up      Dial(IAX2/moylaptop:moylaptopp
IAX2/64.79.194.86:45 (None)               Up      Bridged Call(SIP/test-081d92d8
SIP/test-081d92d8    123@waitforbridge:2  Up      Dial(IAX2/moylaptop:moylaptopp
4 active channels
2 active calls
*CLI>   == Parsing '/etc/asterisk/manager.conf': Found
 == Manager 'mark' logged on from 127.0.0.1
[Apr  3 00:17:15] WARNING[21293]: cdr.c:487 ast_cdr_merge: CDR start disagreement for SIP/test-081d92d8
   -- Hungup 'IAX2/64.79.194.86:4569-3'
 == Spawn extension (waitforbridge, 123, 2) exited non-zero on 'Bridge/SIP/test-081d92d8<ZOMBIE>'
[Apr  3 00:17:15] WARNING[21293]: cdr.c:487 ast_cdr_merge: CDR start disagreement for SIP/moises-081da870
   -- Hungup 'IAX2/64.79.194.86:4569-1'
 == Spawn extension (waitforbridge, 123, 2) exited non-zero on 'Bridge/SIP/moises-081da870<ZOMBIE>'
 == Manager 'mark' logged off from 127.0.0.1
   -- Native bridging SIP/moises-081da870 and SIP/test-081d92d8
--- set_address_from_contact host '192.168.1.65'
--- set_address_from_contact host '192.168.1.64'
--- set_address_from_contact host '192.168.1.65'

*CLI> [Apr  3 00:17:47] NOTICE[21293]: chan_sip.c:15881 sip_poke_noanswer: Peer 'test' is now UNREACHABLE!  Last qualify: 2

*CLI> core show channels
Channel              Location             State   Application(Data)            
SIP/moises-081da870  123@waitforbridge:3  Up      ManagerBridge(SIP/test-081d92d
SIP/test-081d92d8    123@waitforbridge:3  Up      Bridged Call(SIP/moises-081da8
2 active channels
0 active calls
*CLI> soft hangup SIP
SIP/moises-081da870  SIP/test-081d92d8    
*CLI> soft hangup SIP/test-081d92d8
Requested Hangup on channel 'SIP/test-081d92d8'
*CLI>     -- Executing [123@waitforbridge:3] Playback("SIP/moises-081da870", "tt-monkeys") in new stack
   -- <SIP/moises-081da870> Playing 'tt-monkeys.gsm' (language 'en')
--- set_address_from_contact host '192.168.1.65'

As you can see, it works, and I have audio on both sides. After hanging up any side of the call the other side continues with the next dial plan priority.

By: Moises Silva (moy) 2007-04-03 00:30:52

boch:

I have tested your scenario this way:

1. SIP Agent1 call IAX2 with Dial(), hence we have 1 bridged call.
2. SIP Agent2 call context with Bridge(SIP/Agent1-blah), here IAX2 is hung up and we keep 1 bridge between SIP agent 1 and agent 2.
3. With manager redirect I send the 2 channels to some arbitrary context where they listen tt-monkeys. No audio lost.

Could you provide me with a more precise test case in order to reproduce and fix?

By: Matt King, M.A. Oxon. (kebl0155) 2007-04-03 04:18:19

We've been using this patch with 1.4.1 with a couple of customers for a number of days, and it's solved several problems we had when we used ParkedCall to do the bridge.  Thank you!

My one request would be that the BRIDGERESULT channel variable is set on both channels - we're currently doing this with Manager SETVAR, as otherwise we can't tell whether the bridged channel is continuing due to a bridge end, or some other cause.

Many thanks for this excellent feature.  I hope it makes it into trunk soon.

Matt.



By: Russell Bryant (russell) 2007-04-09 17:10:14

I am going to begin looking at this.

Moy, you are the only one that has contributed code here where it is not clear from the comments whether you have a license agreement (disclaimer) on file with Digium.  Could you please verify that you have submittied one?

Thanks

By: Russell Bryant (russell) 2007-04-09 17:27:04

I have created a branch for this patch ...

http://svn.digium.com/svn/asterisk/team/russell/issue_5841

By: Moises Silva (moy) 2007-04-10 09:48:15

Russel: Nice to see you are going to start helping out here, thanks for that.

I have already submited license agreement by fax, several of my small fixes have been integrated the last year.

By: Russell Bryant (russell) 2007-04-10 10:35:14

moy: Thanks for the reply.

I have another concern on the licensing side of things.  There is the following statement in the original report:

"I basically stole everything from mattf's manager bridge action and uncommented and commented stuff until it worked. (In fact, his name should probably be in the copyright notice instead of mine.)"

If that is the case, then where did this code originally come from?  Did it come from an old report where the code was disclaimed?  Unfortunately, this will need to be resolved before I can do anything more with this code ...

By: Serge Vecher (serge-v) 2007-04-10 10:56:01

russell: I believe mattf referred here is matt florell (!cresl1n). The code was "ripped off" from issue 4297. Looks like it was disclaimed there ....

By: Moises Silva (moy) 2007-04-10 15:02:12

Yes, indeed, "twisted" administrator, said in his first post, point 4:

"You seem to have used my code and not mflorrel's, although they're quite similar. If you're going to give credit, then it should be to both of us. I obviously don't mind people taking my code and doing things with it, but if you're going to credit things, give credit where it's due."

So, I think that fixes the licensing issue, right?

By: Russell Bryant (russell) 2007-04-10 15:13:50

Yeah, it's all fine now.

By: Russell Bryant (russell) 2007-04-10 15:45:42

After doing various minor fixes and code cleanup changes in the branch, I have committed this to trunk in revision 61281.

Thank you to everyone that contributed to this patch, in both writing code and testing!