[Home]

Summary:ASTERISK-09810: MusicOnHold() drops the channel if the specified class does not exist
Reporter:Leif Madsen (lmadsen)Labels:
Date Opened:2007-07-05 14:37:00Date Closed:2007-07-09 15:44:06
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Resources/res_musiconhold
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) mohfallback-1.2.diff
( 1) mohfallthrough1.4.patch
( 2) musiconhold-fallback.diff
( 3) musiconhold-fallback-2.diff
Description:Subject pretty much says it all. Easiest fix is probably just to fall back to the 'default' MoH class. This would also assume 'default' existed, although if it didn't, then we can just drop the channel... or alternatively, continue on in the dialplan (with the former being preferred I guess since it won't change any behaviour)
Comments:By: Leif Madsen (lmadsen) 2007-07-05 14:41:57

Ugh, I'm dumb. If I actually had a 'default' class this would have already worked for me since the code already does that. Thanks Juggie!
By: Leif Madsen (lmadsen) 2007-07-05 15:05:01

Reopening because upon actually testing it didn't do what I thought at first (it isn't checking for the actual class, just that something is passed to MusicOnHold().

Juggie said he'd write up a patch tonight or tomorrow for this.
By: Donny Kavanagh (donnyk) 2007-07-05 15:11:04

What happening is ast_moh_start does its whole fallback procedure where it checks for a number of different levels of the music on hold class setting, however when it does this it doesn't check to see if the class actually exists.  Now granted this is *MOSTLY* user error.  But since blitzrage apparently has a use for it i'll post this patch tomorrow morning, essentially rather then just taking the first setting we find (or default if we get that far) we'll actually check to ensure the moh class exists and keep falling through if it indeed does not.


By: Donny Kavanagh (donnyk) 2007-07-06 13:52:41

This patch is for 1.4 as requested by blitzrage it will not apply cleanly on trunk as there are some changes.

If there is interest in adding this functionality to trunk let me know and i can create a patch for trunk.
By: Leif Madsen (lmadsen) 2007-07-06 14:01:59

This patch works as expected. When I give an illegal MoH class, but the default class exists, I end up with this:

   -- Executing [s@sub-callparking:17] MusicOnHold("Local/s@sub-callparking-e4b3,2", "illegal") in new stack
[Jul  6 15:11:18] WARNING[25635]: res_musiconhold.c:933 moh_classexist: No Class: illegal
   -- Started music on hold, class 'default', on Local/s@sub-callparking-e4b3,2


When neither exists, it falls through just as it used to prior to the patch (i.e. no crashes or anything :))

   -- Executing [s@sub-callparking:17] MusicOnHold("Local/s@sub-callparking-bfc8,2", "illegal") in new stack
[Jul  6 15:16:27] WARNING[25673]: res_musiconhold.c:933 moh_classexist: No Class: illegal
[Jul  6 15:16:27] WARNING[25673]: res_musiconhold.c:933 moh_classexist: No Class: default
[Jul  6 15:16:27] WARNING[25673]: res_musiconhold.c:968 local_ast_moh_start: Could not find acceptable Music On Hold Class, ending Music On Hold[Jul  6 15:16:27] WARNING[25673]: res_musiconhold.c:576 moh0_exec: Unable to start music on hold (class 'illegal') on channel Local/s@sub-callparking-bfc8,2


This patch is working based on my testing. Thanks!
By: Jason Parker (jparker) 2007-07-06 14:10:46

This patch was pretty much what I had in mind.  You don't really need to implement those two new functions though.  Give me a minute, and I'll put something together.
By: Jason Parker (jparker) 2007-07-06 14:57:04

Okay, after talking to Juggie on IRC, we came up with a new patch.
By: Donny Kavanagh (donnyk) 2007-07-06 15:51:10

Ok qwell, when ready port this to trunk and commit it :)
By: Donny Kavanagh (donnyk) 2007-07-09 11:08:32

Waiting on blitzrage to report his testing of the latest patch.
By: Leif Madsen (lmadsen) 2007-07-09 12:22:51

Tested and working on 1.4.6!
By: Donny Kavanagh (donnyk) 2007-07-09 12:40:13

Great, not sure if this is a 'bug fix' or a 'feature' for 1.4, russ?  But trunk should be no issue.

Thanks
By: Jason Parker (jparker) 2007-07-09 12:40:53

Attached a similar patch against 1.2 - feel like trying it out?
By: Leif Madsen (lmadsen) 2007-07-09 12:53:16

Sorry, I don't have any 1.2 boxes :(
By: Donny Kavanagh (donnyk) 2007-07-09 12:55:48

Me either.

I can set one up but before i do that, if anyone else is watching and can test against 1.2 it would be much appreciated.
By: Jason Parker (jparker) 2007-07-09 13:06:28

The 1.2 version "Works on my Box", for what it's worth.

   -- Executing SetMusicOnHold("SIP/5555-007a1a70", "blah2") in new stack
   -- Executing StartMusicOnHold("SIP/5555-007a1a70", "blah3") in new stack
Jul  9 13:22:12 WARNING[19467]: res_musiconhold.c:906 moh_getclass: No Class: blah3
Jul  9 13:22:12 WARNING[19467]: res_musiconhold.c:906 moh_getclass: No Class: blah2
Jul  9 13:22:12 WARNING[19467]: res_musiconhold.c:906 moh_getclass: No Class: default
Jul  9 13:22:12 WARNING[19467]: res_musiconhold.c:935 local_ast_moh_start: Using first Music on Hold class in list: blah.
   -- Started music on hold, class 'blah', on SIP/5555-007a1a70
By: Digium Subversion (svnbot) 2007-07-09 15:37:05

Repository: asterisk
Revision: 74162

------------------------------------------------------------------------
r74162 | russell | 2007-07-09 15:37:04 -0500 (Mon, 09 Jul 2007) | 9 lines

(closes issue ASTERISK-9810)
Reported by: blitzrage
Patches submitted by: juggie, qwell, me
Tested by: blitzrage

When trying to find a music on hold class to use, try all of the options,
instead of only the first one that is set.  Also, change the MusicOnHold
applications to not hang up on the channel when a class can not be found.

------------------------------------------------------------------------
By: Digium Subversion (svnbot) 2007-07-09 15:37:53

Repository: asterisk
Revision: 74163

------------------------------------------------------------------------
r74163 | russell | 2007-07-09 15:37:53 -0500 (Mon, 09 Jul 2007) | 17 lines

Merged revisions 74162 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r74162 | russell | 2007-07-09 15:53:46 -0500 (Mon, 09 Jul 2007) | 9 lines

(closes issue ASTERISK-9810)
Reported by: blitzrage
Patches submitted by: juggie, qwell, me
Tested by: blitzrage

When trying to find a music on hold class to use, try all of the options,
instead of only the first one that is set.  Also, change the MusicOnHold
applications to not hang up on the channel when a class can not be found.

........

------------------------------------------------------------------------
By: Digium Subversion (svnbot) 2007-07-09 15:43:38

Repository: asterisk
Revision: 74165

------------------------------------------------------------------------
r74165 | russell | 2007-07-09 15:43:36 -0500 (Mon, 09 Jul 2007) | 4 lines

When the specified class isn't found, properly fall back to the channel's music
class or the default.
(issue ASTERISK-9810, reported by blitzrage, patches from juggie, qwell, and me)

------------------------------------------------------------------------
By: Digium Subversion (svnbot) 2007-07-09 15:44:06

Repository: asterisk
Revision: 74166

------------------------------------------------------------------------
r74166 | russell | 2007-07-09 15:44:05 -0500 (Mon, 09 Jul 2007) | 11 lines

Blocked revisions 74165 via svnmerge

........
r74165 | russell | 2007-07-09 16:00:17 -0500 (Mon, 09 Jul 2007) | 4 lines

When the specified class isn't found, properly fall back to the channel's music
class or the default.
(issue ASTERISK-9810, reported by blitzrage, patches from juggie, qwell, and me)

........

------------------------------------------------------------------------