Summary:	ASTERISK-06015: misplaced ast_moh_stop in svn7865 ?
Reporter:	Luigi Rizzo (rizzo)	Labels:
Date Opened:	2006-01-08 03:51:10.000-0600	Date Closed:	2011-06-07 14:10:31
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Resources/res_features
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	the commit svn7865 to res_features.c, from bug ASTERISK-5534, places the pair ast_moh_stop(peer)/ast_indicate(...) in two different places in the if block. If parkedplay==0, the function might exit on error (hanging up the peer) without calling moh_stop and ast_indicate, whereas in the "else" case the two things are done first (i assume this is because you need to stream on the 'peer' channel). I cannot understand why they are not in the same place for both cases, e.g. at the beginning of the block, which would actually permit to put the two functions outside the 'if' statement, and make the code easier to follow. ****** ADDITIONAL INFORMATION ****** i would have reopened ASTERISK-5534 but don't see a way from the web page, maybe only the original submitter can do that ?
Comments:	By: BJ Weschke (bweschke) 2006-01-08 06:34:18.000-0600 if the function does exit on error, we're hanging up on the same channel peer so that will by default take them off of music hold. In the first scenario (!parkedplay), we're only playing to the opposing channel so we actually want moh to continue to play to peer so they don't hear silence while the other one is hearing the tone. By: Luigi Rizzo (rizzo) 2006-01-08 06:45:59.000-0600 bweschke: thanks for the explaination, i imagined this was the reason. i was just suggesting that instead of optimizing for the error case, we could try to simplify the code avoiding the replication of the common block, since it is correct in both ways. never mind - fwiw, this report can be closed. By: BJ Weschke (bweschke) 2006-01-08 07:30:04.000-0600 closing per poster's request. rizzo - I'm more than open to suggestions as to how this might be optimized while still keeping peer on moh when chan is the only one received the parkedtone. This patch was put in a while back as a "quick and dirty" type of thing, so there probably is an opportunity to make it better.