> On April 10, 2014, 3:35 p.m., Mark Michelson wrote: > > /trunk/apps/app_mixmonitor.c, line 1067 > > <https://reviewboard.asterisk.org/r/3424/diff/2/?file=57155#file57155line1067> > > > > Suggestion: > > > > Instead of doing ast_module_check(), use ast_custom_function_find() to > > find the PERIODIC_HOOK function. I have a couple of reasons for this: > > > > 1) Currently, if func_periodic_hook.so is loaded, it also means that > > PERIODIC_HOOK is registered, but that assumption may not always hold. > > > > 2) More importantly, getting the ast_custom_function allows for you to > > bump the module refcount for func_periodic_hook.so so that the module > > cannot be unloaded while there is an active mixmonitor. You can decrement > > the module refcount when the mixmonitor is destroyed.
I started looking at this, but I'm not sure the alternative is better. They both have downsides. ast_custom_function_find() may be worse. Re: #1, you're right, but ... Re: #2, the module ref count is now always incremented on func_periodic_hook.so when PERIODIC_HOOK() is active. Also, it looks like ast_custom_function_find() isn't safe. There is a race condition between finding a function and incrementing the module ref count where the module could be unloaded and cause a crash. At least with ast_module_check(), ast_func_read() may fail, but it's handled gracefully without crashing. - Russell ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/#review11562 ----------------------------------------------------------- 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
