----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/#review11441 -----------------------------------------------------------
Do you have any ideas on how this will effect CPU load? I was thinking of this as an example: A system with 1 inbound call per second, each call lasting 5 minutes. Every inbound channel gets a 60 second timer for beep Playback (assume the timer runs 5 times). The inbound channel then dials outbound. You would reach about 600 concurrent "real" channels (300 in, 300 out), 2 real channels created and destroyed every second. Of the 300 timers on inbound channels, 5 will fire every second. This means 10 local channels are created and destroyed every second. This gives us a total of 610 concurrent channels, 12 channels created and destroyed every second. I know channel creation/destruction isn't a lightweight operation, but I don't know if it will have a big impact overall. /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21128> Need to document and enforce a minimum interval. Also need to document that interval is whole seconds only. /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21114> Can we get a documented variable for the original channel name or uniqueid? I'd prefer something defined by this module (so not SPY_CHANNEL). /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21127> state->interval could be added to hook_thread_arg so we can use it for dial timeout. /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21109> state->last_hook is never pre-initialized to ast_tvnow(), so hooks would always trigger on first check after being created or re-enabled. Unless this behaviour is documented I would expect the first run after create or resume to be delayed state->interval seconds. /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21121> If do_hook fails we should log an error but still update state->last_hook. Otherwise we would try running do_hook for every audio frame until it succeeds. /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21120> This confused me for a minute, hook_on is run when the hook is created, hook_re_enable is called when someone writes "on". /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21126> Need checks for !ast_strlen_zero on all args. Maybe also a verify the exten@context exists? /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21111> Can we detach the audio hook here? I know we escape the hook callback very early but it still feels wasteful to keep it enabled. Possibly remove state->disabled variable? /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21125> Check for NULL chan here. /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21122> Check for NULL chan here too /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21112> Should we have an option to delete a hook? Something to free all memory from a periodic_hook that will not be used again. /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21118> AST_MODULE for registrar? /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21119> AST_MODULE for registrar? /trunk/funcs/func_periodic_hook.c <https://reviewboard.asterisk.org/r/3362/#comment21116> Is this due to a technical limitation of ChanSpy? If not we should incorporate hook_id into the group variable so we limit 1 channel per hook_id. Maybe add to documentation that a timer interval will be skipped if the channel from the previous interval hasn't ended. - Corey Farrell On March 29, 2014, 4:27 p.m., Russell Bryant wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3362/ > ----------------------------------------------------------- > > (Updated March 29, 2014, 4:27 p.m.) > > > Review request for Asterisk Developers. > > > Repository: Asterisk > > > Description > ------- > > This commit introduces a new dialplan function, PERIODIC_HOOK(). > It allows you run to a dialplan hook on a channel periodically. The > original use case that inspired this was the ability to play a beep > periodically into a call being recorded. The implementation is much > more generic though and could be used for many other things. > > The implementation makes heavy use of existing Asterisk components. > It uses a combination of Local channels and ChanSpy() to run some > custom dialplan and inject any audio it generates into an active call. > > The other important bit of the implementation is how it figures out > when to trigger the beep playback. This implementation uses the > audiohook API, even though it's not actually touching the audio in any > way. It's a convenient way to get a callback and check if it's time > to kick off another beep. It would be nice if this was timer event > based instead of polling based, but unfortunately I don't see a way to > do it that won't interfere with other things. > > > Diffs > ----- > > /trunk/funcs/func_periodic_hook.c PRE-CREATION > /trunk/CHANGES 411572 > > Diff: https://reviewboard.asterisk.org/r/3362/diff/ > > > Testing > ------- > > Called the following extension (100@test), both letting it run all the way > through, as well as hanging up at various points in the middle. > > [hooks] > > exten => beep,1,Answer() > same => n,Playback(beep) > > [test] > > exten => 100,1,Answer() > same => n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)}) > same => n,Wait(20) > same => n,Set(PERIODIC_HOOK(${BEEP_ID})=off) > same => n,Wait(20) > same => n,Set(PERIODIC_HOOK(${BEEP_ID})=on) > same => n,Wait(20) > same => n,Hangup() > > > Thanks, > > Russell Bryant > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
