----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/#review11229 -----------------------------------------------------------
Some things I found before I saw your comment about making this more generic. /trunk/funcs/func_beep.c <https://reviewboard.asterisk.org/r/3362/#comment20862> exten never changes so it should be declared next to CONTEXT_NAME, either another const or calculated by module_load. /trunk/funcs/func_beep.c <https://reviewboard.asterisk.org/r/3362/#comment20864> This is essentially a clean looking but inefficient way to do: state = (struct beep_state *)audiohook; Or the more correct method: state = (struct beep_state *)(((char*)audiohook) - offsetof(struct beep_state, audiohook)); I raise this point since more than 99% of the calls to beep_callback will take no action. - Corey Farrell On March 15, 2014, 2:15 p.m., Russell Bryant wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3362/ > ----------------------------------------------------------- > > (Updated March 15, 2014, 2:15 p.m.) > > > Review request for Asterisk Developers. > > > Repository: Asterisk > > > Description > ------- > > This commit introduces a new dialplan function, BEEP(). It allows you > to enable playing a beep tone at a regular interval into a call. The > most common use case for this is to use during call recording to > notify and remind the callers that the call is being recorded. A > future commit will update the call recording applications in Asterisk > to use this automatically if an option is specified for convenience. > > The implementation makes heavy use of existing Asterisk components. > Instead of replicating logic required to load a sound file, transcode > it if necessary, and do audio mixing, it makes use of > 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 > ----- > > /trunk/funcs/func_beep.c PRE-CREATION > /trunk/CHANGES 410649 > > Diff: https://reviewboard.asterisk.org/r/3362/diff/ > > > Testing > ------- > > Called the following extension, both letting it run all the way through, as > well as hanging up at various points in the middle. > > [test] > > exten => 100,1,Answer() > same => n,Set(BEEP(5)=on) > same => n,Wait(20) > same => n,Set(BEEP()=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
