> On April 8, 2014, 1:49 p.m., Corey Farrell wrote:
> > /trunk/main/app.c, line 3109
> > <https://reviewboard.asterisk.org/r/3424/diff/1/?file=57128#file57128line3109>
> >
> >     Can we create the context from func_periodic_hook?  My complaint is 
> > that for anyone not using func_periodic_hook this context is just wasted 
> > memory.
> >     
> >     Another way to deal with this would be for ast_app_get_beep_* to call 
> > init_beep_helper and track if we've created the extensions or not.
> >     
> >     In any case please ensure the internal dialplan is removed at shutdown.

I was actually hoping that the code in init_beep_helper would move to 
func_periodic_hook.  This is more of a nit, you've addressed my major concern 
which is to lifetime of the beep context.

On the topic of what code goes where, almost all the code could be moved to 
optional_api functions in func_periodic_hook.  One function to start and one to 
stop beeping on a channel.  This would have a few benefits:
* De-duplication of code to be used by res_monitor and app_mixmonitor.
* No code added to asterisk core.
* Beep dial-plan becomes a private implementation detail, and can use the 
func_periodic_hook context.


> On April 8, 2014, 1:49 p.m., Corey Farrell wrote:
> > /trunk/apps/app_mixmonitor.c, line 1075
> > <https://reviewboard.asterisk.org/r/3424/diff/1/?file=57126#file57126line1075>
> >
> >     I think this should be an error, and we should abort recording.  The 
> > main reason to use this flag will be for policy compliance reasons.  In 
> > that case it might be unacceptable or illegal to continue recording without 
> > beeps.

No LOG_ERROR?  Same comment for the other place we return -1.


- Corey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3424/#review11515
-----------------------------------------------------------


On April 8, 2014, 7:49 p.m., Russell Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3424/
> -----------------------------------------------------------
> 
> (Updated April 8, 2014, 7:49 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Add an option to enable a periodic beep to be played into a call if it
> is being recorded.  If enabled, it uses the PERIODIC_HOOK() function
> internally to play the 'beep' prompt into the call at a specified
> interval.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/app.c 412023 
>   /trunk/include/asterisk/app.h 412023 
>   /trunk/funcs/func_periodic_hook.c 412023 
>   /trunk/apps/app_mixmonitor.c 412023 
>   /trunk/CHANGES 412023 
> 
> Diff: https://reviewboard.asterisk.org/r/3424/diff/
> 
> 
> Testing
> -------
> 
> exten => 103,1,Answer()
>     same => n,MixMonitor(test.gsm,B(5))
>     same => n,MusicOnHold()
> 
> exten => 104,1,Answer()
>     same => n,MixMonitor(test.gsm,B)
>     same => n,MusicOnHold()
> 
> exten => 105,1,Answer()
>     same => n,MixMonitor(test.gsm,B(3))
>     same => n,StartMusicOnHold()
>     same => n,Wait(15)
>     same => n,StopMusicOnHold()
>     same => n,StopMixMonitor()
>     same => n,Wait(5)
>     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

Reply via email to