[asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.

Sat Mar 29 19:12:00 CDT 2014

-----------------------------------------------------------
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 at 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 at 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140330/c880ea03/attachment-0001.html>