Summary:	ASTERISK-06505: [patch][post 1.4] Random Periodic Announcements in app_queue
Reporter:	Phil Buescher (alt_phil)	Labels:
Date Opened:	2006-03-08 13:33:59.000-0600	Date Closed:	2008-03-18 13:56:18
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Applications/NewFeature
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 6681.patch ( 1) random_periodic_announce_v4.patch
Description:	Just wanted to submit this little addition to Periodic Announcements while on hold. This patch allows you to specify a directory instead of a file for periodic-announce in queues.conf. Doing so will randomly choose and play a file in that directory, however often is specified with periodic-announce-frequency.  A different random file from that directory is played each time.
Comments:	By: Phil Buescher (alt_phil) 2006-03-08 13:36:08.000-0600 Sorry, heh, generated the patch file backwards.  I'm having a bad day. By: Olle Johansson (oej) 2006-03-08 13:47:07.000-0600 Without a disclaimer, we won't look at this patch. Please confirm that you have a disclaimer with Digium. Thanks. By: Phil Buescher (alt_phil) 2006-03-08 15:00:33.000-0600 Ok, just faxed the disclaimer in. By: Olle Johansson (oej) 2006-03-08 15:26:17.000-0600 Thanks for fixing that quickly! By: Olle Johansson (oej) 2006-03-18 05:36:17.000-0600 Please read the coding guidelines - no names in the source code files like you have in the comments... - Curly bracket placement does not follow coding guidelines. Please fix. - use tabs instead of spaces for intendation. - Error messages should be sent to LOG_ERROR, not LOG_DEBUG I think it's a cool addition, so please fix this and I'll include it in the test branch. Thanks! By: Phil Buescher (alt_phil) 2006-03-21 12:21:01.000-0600 Ok, I think I've got it to the standards now.  Sorry - first submission to this project. By: Olle Johansson (oej) 2006-04-05 11:12:30 Seems like app_queue was recently changed. Please update your patch again... Thanks! By: opsys (opsys) 2006-04-29 01:43:27 Has this been updated to reflect changes in app_queue? By: Phil Buescher (alt_phil) 2006-05-03 08:50:17 Sorry, been on vacation and blissfully unplugged :) Here's a patch for 1.2.7.1. By: Serge Vecher (serge-v) 2006-05-08 15:08:38 alt_phil: since this is a new feature, the patch needs to be against the /trunk, so it can make it into the next stable release. Thanks. By: BJ Weschke (bweschke) 2006-05-09 09:36:14 There is a thread safe and asterisk standard way to get a random number, and say_periodic will probably be seeing some changes as a result of the latest patch on ASTERISK-6594 to accomodate another bug with being able to "exitwithkey" correctly during periodic announcements. We'll probably want to accomodate that into any patches here. By: Serge Vecher (serge-v) 2006-06-07 16:01:35 ok, since 6776 has been committed, what do we need to do here? By: BJ Weschke (bweschke) 2006-06-08 07:20:34 I'm going to work on this a bit today. I've got a client request for similar functionality. Aside from the random threading issue, I'd think we'd probably also be better off to check for the existence of the files at the time of a reload of app_queue and just save that info as a list for others to get at. It seems kind of expensive to be doing this in realtime everytime we're trying to playback a periodic announcement now. By: Serge Vecher (serge-v) 2006-09-01 13:28:29 ping By: jmls (jmls) 2006-10-31 12:47:16.000-0600 any news on the updated patch for trunk ? By: jmls (jmls) 2006-11-20 14:24:42.000-0600 were you able to work on this patch ? Thanks By: jmls (jmls) 2007-02-11 03:25:54.000-0600 um, ping .. By: jmls (jmls) 2007-05-28 02:38:15 please don't say that this patch is dead - it would be really useful to get this in ... By: Mark Michelson (mmichelson) 2007-10-18 14:39:14 I've uploaded a new version of this patch. Here are some differences between it and the old patch: 1. It applies cleanly to the current SVN trunk (rev 86329) 2. I replaced uses of insecure functions like sprintf and strcat with the asterisk dynamic string functions. 3. The path to the directory specified is not hardcoded. Instead, you may specify any absolute path to a directory that contains the sound files. 4. I replaced the random functions used with the thread-safe ast_random function. I pondered bweschke's comments about the expense of doing this in realtime, but ended up not changing the patch to reflect those concerns. I can certainly agree with the computational concerns of stat'ing and scandir'ing every time an announcement is played. The thing is, I can also see how someone might want to occasionally change the contents of their periodic-announce directory without the need to reload app_queue. Perhaps another option should be introduced to allow caching of the directory entries at module load time for those who can't afford the CPU hit or who don't change periodic announcement files around often? By: Brandon Kruse (bkruse) 2008-01-27 23:36:38.000-0600 Any new response on this? Just doing some housekeeping, let me know the status. Thanks guys, -bk By: Mark Michelson (mmichelson) 2008-03-18 13:48:44 All right, after a long discussion, I have implemented this feature, though admittedly quite differently than was originally done. Here's the deal: Since periodic-announce already allows you to specify a comma-separated list of announcements, it only makes sense to allow for these messages to be played randomly or sequentially (currently only sequential playback is allowed). Doing things this way makes for the easiest set of changes to the code, allows for the use of language-specific sound files to be played, and is a very inexpensive operation to perform. Expect a commit soon. By: Digium Subversion (svnbot) 2008-03-18 13:54:35 Repository: asterisk Revision: 109621 U   trunk/CHANGES U   trunk/apps/app_queue.c U   trunk/configs/queues.conf.sample ------------------------------------------------------------------------ r109621 | mmichelson | 2008-03-18 13:54:34 -0500 (Tue, 18 Mar 2008) | 9 lines Add option 'randomperiodicannounce' to queues.conf. Setting this will allow the list of periodic announcments specified to be played in a random order instead of being played sequentially. (closes issue ASTERISK-6505) Reported by: alt_phil Tested by: putnopvut ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=109621 By: Digium Subversion (svnbot) 2008-03-18 13:56:18 Repository: asterisk Revision: 109622 _U  branches/1.6.0/ ------------------------------------------------------------------------ r109622 | mmichelson | 2008-03-18 13:56:17 -0500 (Tue, 18 Mar 2008) | 16 lines Blocked revisions 109621 via svnmerge ........ r109621 | mmichelson | 2008-03-18 13:58:42 -0500 (Tue, 18 Mar 2008) | 9 lines Add option 'randomperiodicannounce' to queues.conf. Setting this will allow the list of periodic announcments specified to be played in a random order instead of being played sequentially. (closes issue ASTERISK-6505) Reported by: alt_phil Tested by: putnopvut ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=109622