[Home]

Summary:ASTERISK-06175: [patch] Realtime call control
Reporter:Kaloyan Kovachev (knk)Labels:
Date Opened:2006-01-24 02:45:44.000-0600Date Closed:2011-07-12 09:03:12
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_dial
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) How2Test.1.5-56340.doc
( 1) rtcc_trunk_327742.diff
( 2) rtcc.1.5-56340.diff
( 3) rtcc.diff
( 4) rtcc-1.5-63405.diff
( 5) rtcc1.diff
( 6) rtcc-curl-1.4.11.patch
( 7) rtcc-curl-1.4.12.patch
( 8) rtcc-curl-1.4.13.patch
( 9) rtcc-curl-patch.readme
Description:Adds new variables to the L option of Dial application to recheck the call limit during the call.

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

Mostly usefull for prepaid applications when more than one call per account is allowed.
Also fixes the bug for a call not being disconnected when the time limit is less than the warning time, but after the warning time.
Comments:By: Olle Johansson (oej) 2006-01-24 03:15:40.000-0600

Since you have no disclaimer on file, this patch can not be handled until you confirm that you have sent a disclaimer to digium. Please read the bug guide lines for more information!

Looking forward to a confirmation of your disclaimer, so we can move forward. Thanks.

By: Kaloyan Kovachev (knk) 2006-01-24 05:01:25.000-0600

Disclaimer faxed.
Should i upload a sample application that uses the new feature here or open a separate issue in apps group?

By: Matt Riddell (zx81) 2006-01-25 19:20:26.000-0600

Is the application that you use supposed to return the number of seconds that the recheck should be reassigned for?

Could you upload an example?

By: Kaloyan Kovachev (knk) 2006-01-26 00:44:52.000-0600

I am uploading the application here. If there should be separate issue posted will be happy to do it.
There is also a MySQL version (which calls a function to get the data), which I am using over 2 weeks now. Tested with single call and 1 sec recheck interval - works fine, but you wouldn't use such small value on a loaded server.
If there is an interest - will upload it too, but there is some additional code now, that i should cleanup first.

By: Matt Riddell (zx81) 2006-01-26 07:18:01.000-0600

Cool,

although I'm not sure I understand the lathering:

static char *limitrecheck_descrip =
" LimitRecheck(): Sends an event to the manager and reads NewLimit\n"
"channel variable a second lather to give you some processing time.\n"
"Then it returns the value to the calling thread, which in turn increases\n"
"or decreases the call duration limit according the value returned.\n";

By: Kaloyan Kovachev (knk) 2006-01-26 07:51:57.000-0600

Sorry about my english.
What i mean is that, this app just sends a manager event, then some custom script which is connected to the manager should check if the timelimit should be changed and if so may use manager action to set the channel variable "NewLimit" with the difference in seconds: negative number decreases the duration of the call, positive - increases it.
After sending the event this app sleeps for one second and reads the variable "NewLimit".
Hope it is more clear now, but suggestions of making it more clear are welcome.

By: Matt Riddell (zx81) 2006-01-26 09:43:14.000-0600

Ah, should have been later :) Now it all makes sense.  So just swap lather for later. Although maybe the one second wait should be an option passed to the command?

By: Kaloyan Kovachev (knk) 2006-01-26 10:22:13.000-0600

Yes, it's a good idea.
The data passed from the calling function is the new type i have added - bridge_combo (to pass all the info one may need), so to make the delay configurable there are two options:
1. config file - for a single variable is not a good idea
2. global or channel variable, but if it is a channel variable the channel will need to be locked twice, so will make the change of using a global variable only and will post the new version tomorrow

By: Kaloyan Kovachev (knk) 2006-01-28 02:06:40.000-0600

updated app_limitrecheck-2 with the delay (sleep time) to be read from 'RecheckWaitTime' global variable. Also fixed the crash, caused form the improper assigment of the 'res' value.

By: Kevin P. Fleming (kpfleming) 2006-02-14 15:54:12.000-0600

Architecturally this is rather ugly... there are vague windows of time that will occur between the recheck firing, the manager event being delivered, the time adjustment being sent and then applied, etc. In other words, this is prone to a great deal of inaccuracy.

Why do you need this to be triggered by the bridging code? The external application knows when the call got answered, and knows that it is still up. If it wants to decrease the amount of time remaining, then I think the right thing to do is to add a new manager action to do just that: adjust the call's total time limit to a new value and send a SOFTHANGUP_UNBRIDGE to the channel. This will cause the bridge to break temporarily and recalculate the amount of time remaining.

Is there some application you want do to where this won't be sufficient?

By: Kaloyan Kovachev (knk) 2006-02-15 04:17:58.000-0600

I am not using manager application actually, and it is just as sample app here.
Instead of Manager action i am using different app with a direct call to a MySQL function, which updates the record for the call and gets the new time limit based on the credit left.
The other app is based on the mysql CDR add-on code (just uploaded), but it may still need some cleanup and i would like to make it more flexible - like custom SQL from the config file instead of hardcoded one, configurable option to (not) hangup the call in case of database connection error and probably to change the HANGUPCAUSE instead of CAUSECODE.
The inaccuracy in this case is less than 1 second, while 1 second is the minimum change applied to the timelimit and there is no dropped adio even if it takes longer for the MySQL to precess the query.

By: Kaloyan Kovachev (knk) 2006-02-28 00:49:32.000-0600

Over a month it was working perfectly for me, but yesterday i had 2 coredumps in the recheck function (while experimenting with the local channel, which is closed quickly), so it will need some changes, which i will post soon.
...MOH here :)

By: Kaloyan Kovachev (knk) 2006-03-02 12:26:16.000-0600

Got it. Sometimes the "if (config->timelimit)" block is entered several times for an event which causes the last "else" to be executed more than once.
I guess it is a bug (changed behavior) in test-this-brach (used to test them both), as it will also cause the warning sound part to be executed several times, but maybe i just didn't note this before.
The event comes exactly 1 voice frame (20ms - 3 to 5ms) ealrier, which is the reason for the name "FRAME_EVENT_INACCURACY".
The patch also fixes the nexteventts schedule for warning_freq sound when there is less than 5sec, so you won't hear "you-have" just before being discconnected :)
Now the thread will not start more than once for "timelimit_recheck" interval and the channel is kept locked until the application is started and reads all its variables (which was the reason for the crash).
If "timelimit_recheck" is not defined or 0 the code will never run, so it will not affect the call at all if not used.

By: Serge Vecher (serge-v) 2006-05-09 13:12:14

Kaloyan: since the change here is non-trivial, can you please get on the #asterisk-dev channel on IRC or mail the asterisk-dev list for feedback?

Thanks.

By: Kaloyan Kovachev (knk) 2006-05-10 09:00:07

I guess it will be easier if i separate the patch to a bug fix and the new functionality which is the more complicated part.
As the new functionality also requires changes in the header files it won't go in before (if ever) 1.4 is released, but fixing the 'time limit' and 'warning sound' can be done earlier.
Uploading the fix_only.patch for now.

By: Serge Vecher (serge-v) 2006-05-26 15:36:27

Who wants karma? Can anybody monitoring this bug please confirm that the patch does actually fix a problem in the latest 1.2 branch code?

Thank you! Gracias! Obrigado! Spasibo! Xie Xie! Blagodarya!

By: Kaloyan Kovachev (knk) 2006-06-06 02:04:29

Sorry, i was probably blind
 if (caller_warning || callee_warning)
   nexteventts = ast_tvsub(nexteventts, ast_samp2tv(config->play_warning, 1000));
it will actual play the wrning immedeately and the timelimit is correct, so there is no bug at all.



By: Serge Vecher (serge-v) 2006-06-20 09:32:31

KNK: so it is safe to delete fix_only.patch?

Do you want to post the enhanced patch that you've originally posted for trunk or are we ready to close this issue?

By: Kaloyan Kovachev (knk) 2006-06-20 09:53:40

Yes, it is safe to delete fix_only.patch

The bugreport is for the realtime control actualy, so i will post the full path, but as it requires changes in the headers it won't go in before 1.4 is released.
I'd prefer to post it after 1.4 is realeased ... mark it post 1.4?
If it is preferable to close the issue and i should post a new one later, that's fine for me too.

By: Serge Vecher (serge-v) 2006-06-20 09:58:27

KNK: I will just mark this as [post 1.4]. Please post the patch as soon as 1.4 is forked in the coming weeks/day. Thanks.

By: Kaloyan Kovachev (knk) 2006-07-13 09:14:13

I have started working on the patch before the 1.4 beta release, as there are lots of changes in the block where the patch was previously made, and found that there is an error for the timelimit at line 3580 (rev 37506)
  "if (config->warning_freq) {"
should became
  "if (config->warning_freq && time_left_ms > config->warning_freq) {"
or else the call may not be terminated in time. For example if the warning time is 30sec and warning_freq is 20sec - the call will be 10sec longer than it should be.

The change is too simple to deserve a patch

By: Serge Vecher (serge-v) 2006-07-13 09:36:47

KNK: if you open a new report and provide a patch, I will give you some karma ;)

By: Kaloyan Kovachev (knk) 2006-07-13 10:10:59

new report for such simple thing ... i would give myself negitve karma :), but you are right Mantis is for tracking such things - bugid 7531



By: Serge Vecher (serge-v) 2006-09-27 13:26:23

KNK: the 1.4 branch is forked now and trunk is open for new features, so this bug needs an updated patch for trunk ...

By: Kaloyan Kovachev (knk) 2006-09-28 10:10:22

I am having some issues with my installation and can't test it right now, but the patch is ready.
The problem is that the call is dropped after 30sec and even without the patch being applied and with both branch/1.4 and trunk.
I will make some more tests, but it was working fine with a previous versions of trunk and is working fine during the 30sec interval (set on 5sec recheck) so i don't expect problems.
I am testing it with custom app, but another option is to use:
  SET(LIMIT_RECHECK_APP=Log(NOTICE|${EXTEN},${UNIQUEID},\\$\\{RAND()\\}))
before the Dial().

By: Kaloyan Kovachev (knk) 2006-09-29 02:15:44

asterisk hangups the call instead of playing the warning sound (which in my case is after 30sec). i will dig into this and will try to provide a patch (in a new bug)

By: Kaloyan Kovachev (knk) 2006-09-29 09:17:23

a small fix for coredump when LIMITRECHECK is NULL

By: Kaloyan Kovachev (knk) 2006-11-01 07:15:59.000-0600

as there is a new branch which is open for new features, is it possible to have this patch applied to trunk?

By: P. Christeas (xrg) 2007-01-04 12:03:42.000-0600

please look at issue ASTERISK-8485 !
Seems the logic was broken.

By: Kaloyan Kovachev (knk) 2007-01-05 02:51:07.000-0600

Trunk have changed allot since September and this patch should also be updated, but as it seems there is no interest in this feature (yet) i haven't done it so far.
If someone would like to test it i will provide a diff against 1.4 in few days and against recent trunk later.

By: mystik (mystik) 2007-02-22 19:30:09.000-0600

KNK, do you use this patch also on a 1.2 production server, or only on the 1.4 asterisk tree / trunk ?
If you do use 1.2, could you upload the patch somewhere ? I'm very interested in using it.

By: Kaloyan Kovachev (knk) 2007-02-23 02:19:14.000-0600

I am using in production an older version (patching channel.c instead of app_dial), which is not updated since revision 13279 and there are too many changes exactly in the part i am patching (because of the L option), so it won't be easy to apply it to a later 1.2 release.

If you would like to test this version (which should work better) it will be easy for me to provide a patch for 1.2 (the link at asterisk-backports soon) or i may send you the old version by email

By: Kaloyan Kovachev (knk) 2007-02-23 03:00:38.000-0600

and here is the link for the backported patch:
http://asterisk-backports.org/wiki/index.php/User_talk:KNK

By: Kaloyan Kovachev (knk) 2007-02-23 03:06:48.000-0600

and just a side note:
if you are passing "<bridge_combo>" to your app you may change the timelimit from inside your own app instead of passing back "CALL_LIMIT"

By: Serge Vecher (serge-v) 2007-02-23 09:36:12.000-0600

KNK: please also provide the updated trunk patch, so this could be put in the queue for trunk candidate review.

By: Kaloyan Kovachev (knk) 2007-02-23 09:49:18.000-0600

Here it comes. :)

as trunk is considered 1.5 i have decided to name it 1.5 folowed by the revision number, so it will be easier to track it.

By: Kaloyan Kovachev (knk) 2007-02-23 10:16:02.000-0600

and a small howto without the need of an external app.
Setting CALL_LIMIT variable via manager will cause the limit to change

By: Russell Bryant (russell) 2007-05-07 17:47:35

I think this functionality is a good idea.  At this point in time, I think it would make the most sense to get this functionality into the bridging API branch, which is in development right now.

I'm going to assign this to file for him to take a look at since he is working on that code.

By: Kaloyan Kovachev (knk) 2007-05-08 02:47:56

I don't see any changes in this branch yet and the patch for trunk should be OK - will update the diff in few hours for the current trunk.

I had the idea to extend this later by moving the warning sounds (for Dial's L() option) inside the call_governor thread and to freeup the bridging code from this task ... hope this does not interfere with the bridging API branch idea.

P.S.
Got it successfuly to play the warning file to the caller, but not to the callee ...

By: Kaloyan Kovachev (knk) 2007-05-08 11:23:47

new in rtcc-1.5-63405 - added configurable delay between the execution of the application (or manager event) and the check for the new limit value.

P.S.
1.2 and 1.4 version at asterisk-backports

By: jmls (jmls) 2007-09-12 16:36:55

can anyone have a look at this ?

By: Grey VoIP (greyvoip) 2007-09-17 08:15:07

I've played around with this patch and it works well. The only problem I have with it is related to load issues. The approach used creates an extra channel to do the real-time call control, so for example if two SIP channels are bridged on a call the patch would create a third channel to execute the AGI application and update the original SIP channel timeouts. The load implications of this have me a bit worried as channels are not light weight.

I have written a new patch based on KNK's approach that replaces the extra channel and AGI approach with a Curl and scheduler approach. This lets a single thread do the real-time call control for all in progress calls and should mean the real-time call control mechanism has a negligible increase on system load. In case it's useful I'll upload the patch once I've got diff working properly.

By: Kaloyan Kovachev (knk) 2007-09-17 08:46:15

Do you mean new thread for each call with 'third channel to execute the application', because it is a thread not channel - "pbx_exec(bridge_data->ch0, app, data);" is executing the application on the calling channel, not a new one.

The thread is sleeping most of the time and will not increase the load, but you are right there should be a single thread for all calls instead of each bridge having its own thread.

By: Grey VoIP (greyvoip) 2007-09-17 19:28:39

Sorry my mistake I should have said extra thread not extra channel. It was a couple of months ago I tested the patch. I've checked my notes as well and one other minor thing I came across was that since the AGI app was executing on the same channel I used to get the occassional channel lock error messages flash up on the console. I believe this was being caused because of a lock that pbx_exec uses conflicting with the bridge activity. I did not ever have a situation where the call duration did not get updated properly I was jsut a bit nervous that maybe an audio frame did not get bridged because a lock was missed (or maybe I just didn't grasp what was going on well enough).

The channel lock error message was:
channel.c:1608 ast_waitfor_nandfds: Thread xxx BLocking 'SIP/xx-xxx', already blocked by thread xxx in procedure ast_waitfor_nandfds



By: Kaloyan Kovachev (knk) 2007-09-18 02:03:30

AGI is a heavy app and probably the errors are shown when it is starting. If you make your AGI script custom application (or call it as system command may also do the trick in case you don't need the channel, but only return the change) you probably won't get error messages.

By: Grey VoIP (greyvoip) 2007-10-01 11:05:34

I've uploaded my patch that implements the real-time call control mechanism using a Curl approach. The mechanism used to set the timelimit has been copied from KNK (thanks KNK) but the approach used to get the new timelimit has been changed to use a Curl request and a scehduling mechanism instead of a per call AGI request.

Some sort of real-time call control is something I think a lot of people would like to see incorporated into Asterisk irrespective of the exact approach that is used.

By: Kaloyan Kovachev (knk) 2008-01-29 04:06:47.000-0600

Any chance for this go in 1.6?

By: Joshua C. Colp (jcolp) 2008-02-29 12:05:51.000-0600

After discussing with others this is something we are not going to put in, but will work in support to the bridging API slated for 1.6.

By: Kaloyan Kovachev (knk) 2011-07-12 09:03:12.596-0500

Latest trunk update for the new bridging API