Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 5, 2014, 8:06 a.m.)
Status
--
This change has been marked as submitted.
Review request for Asterisk Developers.
Changes
---
Committed in revision 411768
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 411684
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,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 4, 2014, 3:42 p.m.)
Review request for Asterisk Developers.
Changes
---
updated based on reviews and re-tested
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 (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411684
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,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11498
---
Ship it!
All my concerns are addressed.
- Corey Farrell
On April 4, 2014, 11:42 a.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 4, 2014, 11:42 a.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 411684
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,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On April 2, 2014, 7:44 p.m., rmudgett wrote:
/trunk/funcs/func_periodic_hook.c, lines 48-57
https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line48
Document the (On write) hook_id.
Is this the first function that has different parameters for read and
write?
I don't know of another one off hand that has different args.
On April 2, 2014, 7:44 p.m., rmudgett wrote:
/trunk/funcs/func_periodic_hook.c, lines 85-87
https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line85
These should not be uppercase. That indicates they are macros.
Hrm, I think of UPPERCASE() as a macro and UPPERCASE as a constant. That's why
I did it this way.
On April 2, 2014, 7:44 p.m., rmudgett wrote:
/trunk/funcs/func_periodic_hook.c, lines 301-302
https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line301
You need to check if args.interval is NULL. Also the sscanf should be
using %30u instead. Also passing a NULL string pointer to printf will
crash on Solaris.
Missing \n
Ah, I thought args.interval would be guaranteed to at least be an empty string.
That doesn't seem to be the case.
On April 2, 2014, 7:44 p.m., rmudgett wrote:
/trunk/funcs/func_periodic_hook.c, lines 384-386
https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line384
Maybe use ast_true() and ast_false() instead.
Done
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11476
---
On April 1, 2014, 4:43 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 1, 2014, 4:43 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 411583
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,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 4, 2014, 12:05 a.m.)
Review request for Asterisk Developers.
Changes
---
Thanks for the reviews!
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 (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411684
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,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11494
---
/trunk/funcs/func_periodic_hook.c
https://reviewboard.asterisk.org/r/3362/#comment21224
The test for ast_strlen_zero needs to be reversed.
- Corey Farrell
On April 3, 2014, 8:05 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 3, 2014, 8:05 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 411684
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,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On April 2, 2014, 3:44 p.m., rmudgett wrote:
/trunk/funcs/func_periodic_hook.c, lines 85-87
https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line85
These should not be uppercase. That indicates they are macros.
Russell Bryant wrote:
Hrm, I think of UPPERCASE() as a macro and UPPERCASE as a constant.
That's why I did it this way.
AST_MODULE is also a macro (#define). I normally expect all UPPERCASE
identifiers to be replaced by the C preprocessor.
- Corey
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11476
---
On April 3, 2014, 8:05 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 3, 2014, 8:05 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 411684
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,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11496
---
/trunk/funcs/func_periodic_hook.c
https://reviewboard.asterisk.org/r/3362/#comment21228
extension?
/trunk/funcs/func_periodic_hook.c
https://reviewboard.asterisk.org/r/3362/#comment21226
The variable tags cause the ${} to be added by output formatting. They
should not be in the raw text.
/trunk/funcs/func_periodic_hook.c
https://reviewboard.asterisk.org/r/3362/#comment21229
ran
/trunk/funcs/func_periodic_hook.c
https://reviewboard.asterisk.org/r/3362/#comment21227
ast_strdupa() is not NULL tollerant.
Doing
ast_strdupa(S_OR(data, ))
will then cause the interval message to go out.
- rmudgett
On April 3, 2014, 7:05 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 3, 2014, 7:05 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 411684
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,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11472
---
/trunk/funcs/func_periodic_hook.c
https://reviewboard.asterisk.org/r/3362/#comment21198
The xmldoc states that these parameters are required so I think it would be
better to report failure if either are ast_strlen_zero.
I have tagged this issue here instead of the documentation since I don't
agree with them having defaults. I would not object strongly if args.exten
were optional / default s, but only if the parameters were reordered to
PERIODIC_INTERVAL(interval,context,exten). My preference is to require the
caller to be explicit with all parameters.
/trunk/funcs/func_periodic_hook.c
https://reviewboard.asterisk.org/r/3362/#comment21200
I just had a thought about AMI (and ARI?) security. This function should
be unavailable to users without originate permissions. I'm not aware of a way
to require Originate permissions from someone who is using AMI to write to this
function, so I think this might need to use
ast_custom_function_register_escalating.
I am very sure with AMI access to read this function and channel events I
could perform a DoS attack against the local system while harassing 1 or more
individuals with a continuous stream of silent calls (they hangup one call,
another call comes within 1 second). Normally a lack of permission to
originate channels should prevent me from doing this.
- Corey Farrell
On April 1, 2014, 12:43 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 1, 2014, 12:43 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 411583
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,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
rmudgett wrote:
I haven't checked, but would calling this be appropriate here to avoid
the adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct
ast_channel *sub_chan, const char *sub_args, int ignore_hangup)
Russell Bryant wrote:
Awesome. That sounds like the API call needed here.
Well ... it's not *that* straight forward. GoSub (or Macro) is being used as
one side of an originated call. I'd have to add a new application to make it
work. GoSubInternal (for internal use only) or something.
Any preference between just using Macro vs adding a new GoSubInternal app?
I know Macro is considered deprecated but it does work fine here. Besides,
Macro has been around *so* long (10+ years?), removing it would just be evil.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 8:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
rmudgett wrote:
I haven't checked, but would calling this be appropriate here to avoid
the adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct
ast_channel *sub_chan, const char *sub_args, int ignore_hangup)
Russell Bryant wrote:
Awesome. That sounds like the API call needed here.
Russell Bryant wrote:
Well ... it's not *that* straight forward. GoSub (or Macro) is being
used as one side of an originated call. I'd have to add a new application to
make it work. GoSubInternal (for internal use only) or something.
Any preference between just using Macro vs adding a new GoSubInternal app?
I know Macro is considered deprecated but it does work fine here.
Besides, Macro has been around *so* long (10+ years?), removing it would just
be evil.
rmudgett wrote:
Macro has been removed from being a requirement of Asterisk core and is
deprecated. It needs to go away. It sounds like you are originating to an
application which is a Macro. Why not just originate to an exten since that
seems to be what Macro is doing for you.
It's originating with dialplan on both ends of the call. Macro is used on one
end as a way to run dialplan, but also pass arguments (variables) to that
dialplan. Earlier revs of the patch originated to an extension directly, but
with that I couldn't pass variables to that dialplan (channel name and hook ID
right now).
BTW, somewhat unrelated, I think removing Macro() is a terrible idea. I'd be
all for re-implementing it using GoSub internally, but removing a syntax that
has been supported for 10+ years, breaking countless examples on the internet,
does not seem worth it.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 8:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
rmudgett wrote:
I haven't checked, but would calling this be appropriate here to avoid
the adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct
ast_channel *sub_chan, const char *sub_args, int ignore_hangup)
Russell Bryant wrote:
Awesome. That sounds like the API call needed here.
Russell Bryant wrote:
Well ... it's not *that* straight forward. GoSub (or Macro) is being
used as one side of an originated call. I'd have to add a new application to
make it work. GoSubInternal (for internal use only) or something.
Any preference between just using Macro vs adding a new GoSubInternal app?
I know Macro is considered deprecated but it does work fine here.
Besides, Macro has been around *so* long (10+ years?), removing it would just
be evil.
rmudgett wrote:
Macro has been removed from being a requirement of Asterisk core and is
deprecated. It needs to go away. It sounds like you are originating to an
application which is a Macro. Why not just originate to an exten since that
seems to be what Macro is doing for you.
Russell Bryant wrote:
It's originating with dialplan on both ends of the call. Macro is used
on one end as a way to run dialplan, but also pass arguments (variables) to
that dialplan. Earlier revs of the patch originated to an extension
directly, but with that I couldn't pass variables to that dialplan (channel
name and hook ID right now).
BTW, somewhat unrelated, I think removing Macro() is a terrible idea.
I'd be all for re-implementing it using GoSub internally, but removing a
syntax that has been supported for 10+ years, breaking countless examples on
the internet, does not seem worth it.
Actually ... maybe this should work without using GoSub or Macro? I'm passing
variables in already via the originate API call, but I was only expecting them
to be available to the dialplan running behind the Local channel for some
reason. Maybe they're available on both sides of the Local channel? I'll have
to test it out.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 8:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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)
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
rmudgett wrote:
I haven't checked, but would calling this be appropriate here to avoid
the adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct
ast_channel *sub_chan, const char *sub_args, int ignore_hangup)
Russell Bryant wrote:
Awesome. That sounds like the API call needed here.
Russell Bryant wrote:
Well ... it's not *that* straight forward. GoSub (or Macro) is being
used as one side of an originated call. I'd have to add a new application to
make it work. GoSubInternal (for internal use only) or something.
Any preference between just using Macro vs adding a new GoSubInternal app?
I know Macro is considered deprecated but it does work fine here.
Besides, Macro has been around *so* long (10+ years?), removing it would just
be evil.
rmudgett wrote:
Macro has been removed from being a requirement of Asterisk core and is
deprecated. It needs to go away. It sounds like you are originating to an
application which is a Macro. Why not just originate to an exten since that
seems to be what Macro is doing for you.
Russell Bryant wrote:
It's originating with dialplan on both ends of the call. Macro is used
on one end as a way to run dialplan, but also pass arguments (variables) to
that dialplan. Earlier revs of the patch originated to an extension
directly, but with that I couldn't pass variables to that dialplan (channel
name and hook ID right now).
BTW, somewhat unrelated, I think removing Macro() is a terrible idea.
I'd be all for re-implementing it using GoSub internally, but removing a
syntax that has been supported for 10+ years, breaking countless examples on
the internet, does not seem worth it.
Russell Bryant wrote:
Actually ... maybe this should work without using GoSub or Macro? I'm
passing variables in already via the originate API call, but I was only
expecting them to be available to the dialplan running behind the Local
channel for some reason. Maybe they're available on both sides of the Local
channel? I'll have to test it out.
Um, yes. It works. I can drop the usage of Macro() or GoSub() completely.
Oops.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 8:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20)
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 1, 2014, 4:43 p.m.)
Review request for Asterisk Developers.
Changes
---
Fixed to no longer use Macro() and just run the extension directly
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 (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411583
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing (updated)
---
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,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 30, 2014, 12:12 a.m., Corey Farrell wrote: 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. I don't have any data. Compared to the cost of all of the other things involved in call processing, it seems relatively low impact to me. Usage of this feature implies Asterisk is processing the media for that call, which is going to be a lot more expensive than this, presumably. (potentially lots of DB hits during setup/teardown, transcoding, disk I/O, ...) On March 30, 2014, 12:12 a.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, lines 54-56 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line54 Need to document and enforce a minimum interval. Also need to document that interval is whole seconds only. Updated to enforce that the interval is non-zero. On March 30, 2014, 12:12 a.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, line 133 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line133 state-interval could be added to hook_thread_arg so we can use it for dial timeout. I don't think the timeout is important here. The extension called is something this module defines. On March 30, 2014, 12:12 a.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, line 206 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line206 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. Fair. I had originally intended for it to run immediately, but I think it makes more sense to delay. On March 30, 2014, 12:12 a.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, line 273 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line273 Need checks for !ast_strlen_zero on all args. Maybe also a verify the exten@context exists? I think this is all handled. For interval, sscanf() will just fail on an empty string. For exten and context, defaults of s and default are applied. Also, I don't think it should check for the exten to exist. If it doesn't, errors should show up when it tries to run. Extensions can be added/removed dynamically, so I think we should just assume the best here. On March 30, 2014, 12:12 a.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, line 299 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line299 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? As you said, the callback does just about nothing, so I don't think it really hurts to keep it. But honestly, I would have done it this way if the audiohooks API was built for dynamic adding/removing. It really isn't. No other code does that. It all relies on channel destruction to tear it down. On March 30, 2014, 12:12 a.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, lines 345-352 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line345 Should we have an option to delete a hook? Something to free all memory from a periodic_hook that will not be used again. It's a pretty tiny amount of memory, I think. Also, the hook can't really be deleted without work to the audiohooks API from my testing. On March 30, 2014, 12:12 a.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, lines 394-396 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line394 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. Good call on adding the ID here. The intention is to prevent a hook from running if it's already running. It was done before I made it so you could have more than one hook at a time.
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 12:56 p.m.)
Review request for Asterisk Developers.
Changes
---
Updated to address comments.
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 (updated)
-
/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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 1:14 p.m.)
Review request for Asterisk Developers.
Changes
---
updated testing extensions
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 (updated)
---
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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 30, 2014, 12:12 a.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, line 206
https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line206
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.
Russell Bryant wrote:
Fair. I had originally intended for it to run immediately, but I think
it makes more sense to delay.
So is the implication then if I were to set the interval to 30 seconds that the
hook wouldn't run for 30 seconds and then run every 30 seconds? That seems
more counter intuitive. I'd think that it should run and then repeat every
interval seconds.
- jamuel
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11441
---
On March 31, 2014, 1:14 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 1:14 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 30, 2014, 12:12 a.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, line 206
https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line206
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.
Russell Bryant wrote:
Fair. I had originally intended for it to run immediately, but I think
it makes more sense to delay.
jamuel wrote:
So is the implication then if I were to set the interval to 30 seconds
that the hook wouldn't run for 30 seconds and then run every 30 seconds?
That seems more counter intuitive. I'd think that it should run and then
repeat every interval seconds.
Yes, that's right. I keep going back and forth but I think I agree with you
that running immediately makes the most sense.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11441
---
On March 31, 2014, 1:14 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 1:14 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 4:11 p.m.)
Review request for Asterisk Developers.
Changes
---
moved back to original behavior of running hook immediately, and then every
interval seconds.
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 (updated)
-
/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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 30, 2014, 12:12 a.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, line 206
https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line206
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.
Russell Bryant wrote:
Fair. I had originally intended for it to run immediately, but I think
it makes more sense to delay.
jamuel wrote:
So is the implication then if I were to set the interval to 30 seconds
that the hook wouldn't run for 30 seconds and then run every 30 seconds?
That seems more counter intuitive. I'd think that it should run and then
repeat every interval seconds.
Russell Bryant wrote:
Yes, that's right. I keep going back and forth but I think I agree with
you that running immediately makes the most sense.
It seems to make more sense especially when the interval is really long.
When the interval is just a few seconds it doesn't really matter but when it's
say 30s or 60s (or even longer) then it seems weird to have to wait that long
for it to run the first time--especially in your original use case of wanting
to beep to indicate call recording . . .
- jamuel
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11441
---
On March 31, 2014, 4:11 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 4:11 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 29, 2014, 8:12 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, line 299
https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line299
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?
Russell Bryant wrote:
As you said, the callback does just about nothing, so I don't think it
really hurts to keep it.
But honestly, I would have done it this way if the audiohooks API was
built for dynamic adding/removing. It really isn't. No other code does
that. It all relies on channel destruction to tear it down.
I took a closer look at audiohook.c, I agree the removal isn't very pretty.
On March 29, 2014, 8:12 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, line 206
https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line206
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.
Russell Bryant wrote:
Fair. I had originally intended for it to run immediately, but I think
it makes more sense to delay.
jamuel wrote:
So is the implication then if I were to set the interval to 30 seconds
that the hook wouldn't run for 30 seconds and then run every 30 seconds?
That seems more counter intuitive. I'd think that it should run and then
repeat every interval seconds.
Russell Bryant wrote:
Yes, that's right. I keep going back and forth but I think I agree with
you that running immediately makes the most sense.
jamuel wrote:
It seems to make more sense especially when the interval is really
long. When the interval is just a few seconds it doesn't really matter but
when it's say 30s or 60s (or even longer) then it seems weird to have to wait
that long for it to run the first time--especially in your original use case
of wanting to beep to indicate call recording . . .
I'm not going to hold up this review for this finding, but I disagree. Please
point me to one implementation of an interval where the first run is
immediately. I can't think of any.
Also with first run delayed by interval, if you want a beep immediately you can
just play one. With no delay on first interval it's impossible to prevent the
immediate beep.
On March 29, 2014, 8:12 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, line 273
https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line273
Need checks for !ast_strlen_zero on all args. Maybe also a verify the
exten@context exists?
Russell Bryant wrote:
I think this is all handled. For interval, sscanf() will just fail on an
empty string. For exten and context, defaults of s and default are
applied.
Also, I don't think it should check for the exten to exist. If it
doesn't, errors should show up when it tries to run. Extensions can be
added/removed dynamically, so I think we should just assume the best here.
same = n,Set(BEEP_ID=${PERIODIC_HOOK()})
I believe the above dialplan would crash. With no parameters all fields of
args remain NULL.
same = n,Set(BEEP_ID=${PERIODIC_HOOK(5)})
This will result in a value for args.interval, but all other parameters NULL.
- Corey
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11441
---
On March 31, 2014, 12:11 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 12:11 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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
/trunk/funcs/func_periodic_hook.c
https://reviewboard.asterisk.org/r/3362/#comment21144
I'm unsure the purpose of beep in macro_arg.
Also think chan_name + hook_id can never be close to 1024 chars
(AST_CHANNEL_NAME is defined as 80).
/trunk/funcs/func_periodic_hook.c
https://reviewboard.asterisk.org/r/3362/#comment21143
Macro is deprecated, why not use Gosub?
- Corey Farrell
On March 31, 2014, 12:11 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 12:11 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 136-139
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line136
I'm unsure the purpose of beep in macro_arg.
Also think chan_name + hook_id can never be close to 1024 chars
(AST_CHANNEL_NAME is defined as 80).
Oops. It's supposed to be arg-macro_name. This was just a mistake from hard
coding something while testing.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
I tried that. GoSub() doesn't actually work in this case. Perhaps because
there's no PBX running on the channel? In any case, that's why.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 4:11 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 4:11 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 p.m.)
Review request for Asterisk Developers.
Changes
---
Fixed left over hard coded macro name from testing. Re-tested to make sure
macro name passed in is used as expected.
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 (updated)
-
/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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 12:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
I haven't checked, but would calling this be appropriate here to avoid the
adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct ast_channel
*sub_chan, const char *sub_args, int ignore_hangup)
- rmudgett
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 3:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 3:33 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
rmudgett wrote:
I haven't checked, but would calling this be appropriate here to avoid
the adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct
ast_channel *sub_chan, const char *sub_args, int ignore_hangup)
Awesome. That sounds like the API call needed here.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 8:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 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.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 29, 2014, 8:06 p.m.)
Review request for Asterisk Developers.
Changes
---
Update diff against CHANGES to current trunk
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.
Playback+ChanSpy to accomplish the task.
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 (updated)
-
/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,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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 29, 2014, 8:11 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description (updated)
---
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,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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 29, 2014, 8:27 p.m.)
Review request for Asterisk Developers.
Changes
---
Added ability turn on the hook again later after it has been turned off.
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 (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing (updated)
---
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
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
--- 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/#comment2 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
