Summary:	ASTERISK-06079: [patch][post 1.4] adjust volume of voicemail sent by email
Reporter:	Robert A. Thompson (ucs_rat)	Labels:
Date Opened:	2006-01-14 09:10:16.000-0600	Date Closed:	2006-08-08 10:38:30
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Applications/app_voicemail
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 20060704__bug6237.diff.txt ( 1) voicemail-email-volgain-fdleak-fix.diff ( 2) volgain_6237_u1.patch
Description:	When receiving email from asterisk with the voicemail attached the volume of the attached file is extraordinarily low.  This has caused problems on systems where the user is already playing one sound and when going back and forth they forget to adjust the volume of their system (most notably after turning the volume up to listen to a voicemail and then returning to their old sounds (beeps/music/etc)). ****** ADDITIONAL INFORMATION ****** I have attached a patch that will allow the user to increase the volume the of the email attachment only (as standard phone volume is fine).   It adds two new volume settings to the voicemail.conf file "dovolgain" and "volgain".  dovolgain is a boolean that will turn on the feature and volgain is a volume increase level.  At the moment 10 seems to be working fine.   Sox is required for this feature to work.   I have patches against 1.2.1 and trunk.  I would be willing to upload or modify the patch to fit any spec/format etc if given feedback on desired methods.
Comments:	By: Jason Parker (jparker) 2006-01-14 10:11:07.000-0600 Is this the same thing that was discussed on the asterisk-dev mailing list? By: Tilghman Lesher (tilghman) 2006-01-14 10:47:02.000-0600 Instead of two parameters, why not default volgain=0, and only do the volume increase if volgain != 0 ? By: Robert A. Thompson (ucs_rat) 2006-01-14 11:21:24.000-0600 yes it is the same as the one being tracked on the devel list.  I posted a note back to the devel list but hadn't seen it go through.  I'll check my mailserver to see if it got hung in the queues. By: Matt Riddell (zx81) 2006-01-14 21:49:59.000-0600 Also, it shouldn't really be called normalized, because to normalize an audio file you have to first find the peak volume, and then increase the gain by the amount the peak was below 0. By: Robert A. Thompson (ucs_rat) 2006-01-15 09:11:32.000-0600 uploaded two new patch files (one against 1.2.1 and one against trunk) that takes out all references to the word normalize and removes the "dovolgain" variable in place of just performing the operation when volgain > 0. By: Russell Bryant (russell) 2006-01-16 10:55:13.000-0600 Just so you know, there is no need to keep up a patch against the 1.2 branch.  The 1.2 branch is not accepting any new features. By: Anthony Minessale (anthm) 2006-01-23 17:39:47.000-0600 Is it really a feature to be able to hear your voicemail messages?  I have this same bug and it makes me pretty annoyed.  Is anyone going to find out what causes it for real and deal with it? By: Tilghman Lesher (tilghman) 2006-01-26 12:15:15.000-0600 Are you using wav49 for your email messages?  We've discovered a possible issue with volume from that format alone. By: Aaron Daniel (adaniel) 2006-01-26 13:39:12.000-0600 we've switched the formats a few times to see which would work best.  doesn't matter which one, they're all pretty low, so we're using this to increase the volume of the messages. By: kb1_kanobe2 (kb1_kanobe2) 2006-02-01 20:17:02.000-0600 At the risk of repeating something already discussed elsewhere, this issue came up as a feature request way back bug ASTERISK-1996. Specifically, people who had implemented reasonable network loss planning on their zaptel interfaces as part of an echo control strategy tended to have excessively quiet voicemails, however this could be worked around by specifically using the 'wav' format. No one could explain why there was this inconsistency between codecs at the time. I notice this evening in comments to bug ASTERISK-5672 (Monitor() recordings have 12dB too much gain) that there is indeed an artifical gain built into transcoding into wav, thereby explaining away the 'wav' format amplitude workaround. I politely suggest that whatever work comes forward from this patch be documented and deployed in conjunction with whatever comes from ASTERISK-5672, thereby preserving usefully functioning voicemail for those who are still in 'workaround' mode. Just my 2c :-) k. By: Serge Vecher (serge-v) 2006-05-01 14:11:52 ucs_rat: can you please provide an updated patch for current trunk? By: Aaron Daniel (adaniel) 2006-05-01 17:20:25 I've uploaded an updated patch for SVN.  I've cleaned it up a little, and made a few modifications.  We're not checking that sox is installed, and there's a couple configuration options to keep in mind. In voicemail.conf, you can set archive=<absolute dir> so it uses that, otherwise it defaults to temp.  Set volgain=(+/-)# (i.e. we have ours at +7) if you want to turn it on, or 0 (or just don't put it in the file) to turn it off. We've been using this patch since it was posted on here pretty successfully with almost no complaints, other than initial testing. By: BJ Weschke (bweschke) 2006-05-01 17:31:53 ucs_rat / amdtech: do either of you have a disclaimer on file? we should try to get this one out of the way so if we get to a point where code can be committed, this isn't still outstanding. Thanks. By: Aaron Daniel (adaniel) 2006-05-01 17:40:46 My disclaimer should be sitting on the fax machine now :) By: Serge Vecher (serge-v) 2006-05-02 09:34:27 Amdtech: 1) Thanks for updating the patch quickly. Since your patch looks to be an update of ucs_rat's work, we still need his/her disclaimer, hence the note in title. 2) I believe the coding guidelines specify not using curly brackets for single lines in if ... else ... contruct. Please update again. Thanks a lot! By: Robert A. Thompson (ucs_rat) 2006-05-02 09:44:44 I just faxed over my disclaimer (or had amdtech fax it for me).  I'll try to get back to looking at this and helping Aaron clean things up and fix the odd and things that we are working on with our roll out. By: Aaron Daniel (adaniel) 2006-05-02 09:55:10 I've removed the extraneous {} braces from this patch version, and fixed a whitespace change I hadn't intended. By: Serge Vecher (serge-v) 2006-05-02 10:02:48 looks good. Removing old patches. This should be ready for review I guess. By: Dan R. Greening (greening) 2006-07-03 15:47:26 Any chance you could review this soon?  I think it affects a ton of people. By: Tilghman Lesher (tilghman) 2006-07-04 02:05:40 The patch I've uploaded represents what I'm willing to commit.  Basically what I've changed is to make the parameter per-mailbox, as well as global, as well as to make the parameter a float, as is allowed by sox. There are other miscellaneous cleanups to the patch as well, but those are the changes to the functionality. Please test this patch and ensure that it performs well, then post feedback in a bugnote. By: Douglas Garstang (dgarstang) 2006-07-12 11:14:59 I tried to apply this patch against Asterisk 1.2.9.1, and it failed. Maybe because I'd already applied a patch (this one: http://bugs.digium.com/view.php?id=7410) to voicemail.c hestia:(pbx1)asterisk-1.2.9.1 # patch -p0 < volgain_6237_u1.patch patching file apps/app_voicemail.c Hunk #1 succeeded at 386 with fuzz 2 (offset -13 lines). Hunk #2 FAILED at 1680. Hunk #3 succeeded at 1781 (offset -16 lines). Hunk #4 succeeded at 1800 (offset -16 lines). Hunk ASTERISK-1 succeeded at 5898 (offset -76 lines). Hunk ASTERISK-2 succeeded at 5936 (offset -70 lines). 1 out of 6 hunks FAILED -- saving rejects to file apps/app_voicemail.c.rej Is there any way I can apply this patch to 1.2.9.1? By: Tilghman Lesher (tilghman) 2006-07-12 11:28:30 No; as stated before, this patch will never be allowed into 1.2, as it is a new feature. By: Douglas Garstang (dgarstang) 2006-07-12 11:31:03 Just for my own edification, can you tell me where that's stated? In the additional information section of this bug, it states 'I have patches against 1.2.1 and trunk', so I'm a little confused. By: Robert A. Thompson (ucs_rat) 2006-07-12 11:37:02 I created patches for what we run and figured it made sense to do it against trunk too.  I understand their point of not adding features to a stable version (hence that patch has probably died off over time).   I can probably get it back ported into a version if you would like. By: Tilghman Lesher (tilghman) 2006-07-12 11:43:03 Russell's bugnote 0039647 states this very clearly:  this is a new feature and new features are not permitted in 1.2. By: Serge Vecher (serge-v) 2006-07-13 09:57:52 dgarstang: without digging too far into archives, this page http://www.voip-info.org/wiki/index.php?page=Asterisk+status gives a good overview of the current Asterisk development process. Under "Month 6" heading, we have the following statement: "Once the release is made, a branch will be created. This branch will then receive maintenance for bug fixes only." Although the paragraph talks specifically about 1.4 release, other releases (1.2.x, 1.0.x) are also grandfathered into this policy. Additionally, as Corydon76 noted, you have a reference to Russell's note. As an official maintainer of release branches, he has the final say over what qualifies as release-branch-applicable material :) By: Nic Bellamy (nic_bellamy) 2006-08-07 17:58:46 I've just uploaded voicemail-email-volgain-fdleak-fix.diff that fixes the leaking of the file descriptor created by mkstemp() in the volgain patch. It needs to be applied after 20060704__bug6237.diff.txt. It would leak one descriptor per voicemail message emailed, which on a busy system would cause problems pretty quickly. As for a disclaimer from me: working on getting a company-wide one done, will add a note when complete. By: Tilghman Lesher (tilghman) 2006-08-08 10:38:30 Committed in 39332.