[asterisk-dev] [Code Review] Music on Hold for external applications try to rescan whatever directory they are looking in for source files, and this can make them blow up under certain circumstances.

[jrose]

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

Review request for Asterisk Developers, Russell Bryant and David Vossel.


Summary
-------

Someone made a patch so that 'moh reload' would scan for changes within the directory using moh_scan_files() if musiconhold.conf had not been touched because they wanted to be able to use that cli command in order to refresh the playlist.

Normally however, moh_scan_files() is only used during initial config and reload if the moh class in question is of mode "files".  This patch changes this addendum to the code so that it also only runs moh_scan_files if the class in question is in mode "files"


Additional Notes:  It seems to me like during normal initialization, if a class is of mode "files" and moh_scan_files returns a failure, the normal action would be to destroy that music on hold class.  This may or may not be desirable behavior if we want to be able to repopulate the playlist at any time like this.  Currently reload won't destroy the channel unless the config has been touched and the directory linked is empty, but I wonder if this is a proper approach at all.

MOH needs to stream the files from the directory regardless of whether or not we run moh reload or not, so it seems to me like it might be a good idea to make the whole thing more dynamic.  I think it might be prudent for MOH classes in files mode to check for additions and removals to the directory every time they prepare to play the next file in the stored playlist.  I don't think the performance hit for this sort of thing would be especially significant.

Alternatively, it might be a good idea to simply separate the idea of reload and rescanning.  Something like "moh <class> refresh" might be an idea.  Honestly, I can think of about a million different things to do with the music on hold class if it were a priority.


This addresses bug ASTERISK-17730.
    https://issues.asterisk.org/jira/browse/ASTERISK-17730


Diffs
-----

  /branches/1.8/res/res_musiconhold.c 324847 

Diff: https://reviewboard.asterisk.org/r/1290/diff


Testing
-------

Tested behavior using madcap before and after changes.  Honestly, this change seemed fairly obvious at the time, so I'm not really all that worried about this.


Thanks,

jrose

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110627/1a7f2098/attachment.htm>