[asterisk-dev] [Code Review] Music on hold in 1.8 doesn't recover cleanly when a moh file fails to load

[Russell Bryant]

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

Ship it!



/branches/1.8/res/res_musiconhold.c
<https://reviewboard.asterisk.org/r/910/#comment5841>

    I think it would be useful to emit a LOG_NOTICE message if a file can't be opened.  Otherwise, this looks good to me.


- Russell


On 2010-09-07 14:00:09, Brett Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/910/
> -----------------------------------------------------------
> 
> (Updated 2010-09-07 14:00:09)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> From the bug reported by kshumard
> ---------------------------------------------------------------------------------
> I found that MOH using default files misbehaved in 1.8.0-beta1 because of a dot in the filenames of the LICENSE, CHANGES, and CREDITS files. I see that this has been fixed in 1.8.0-beta2 by renaming those files so they don't include a dot. But this uncovered something else that it wouldn't hurt to address.
> 
> When res_musiconhold can't play a file in one of its classes, it stops musiconhold and just waits indefinitely.
> 
> I suggest that a cleaner way to handle this would be to skip the file it can't play (and perhaps remove it from the musicclass) and continue on to try to play the next file in the class. As it is, MOH stops and there's just dead air on the channel until it's hungup or taken off hold or whatever. Logging a warning and playing another file is IMO a saner/cleaner way to fail.
> 
> 
> This addresses bug 17807.
>     https://issues.asterisk.org/view.php?id=17807
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/res/res_musiconhold.c 285196 
> 
> Diff: https://reviewboard.asterisk.org/r/910/diff
> 
> 
> Testing
> -------
> 
> Tested on 1.8 branch.
> 
> 
> Thanks,
> 
> Brett
> 
>