[Home]

Summary:ASTERISK-01274: [patch] voicemail calculates duration incorrectly and does not delete txtfile if duration<minduration
Reporter:sbingner (sbingner)Labels:
Date Opened:2004-03-23 03:08:46.000-0600Date Closed:2008-01-15 14:53:02.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) vm_durationfix.diff.txt
Description:Voicemail includes the length of the OGM and the beep in the length of messages recorded.  Also did not delete the msgxxx.txt file when the mesage was <minduration long.  This patch fixes both problems

****** STEPS TO REPRODUCE ******

1. Set an outgoing message for a voicemailbox
2. Leave a voicemail, and time how long your message is
3. Look at the length of the message in the message envelope
Comments:By: twisted (twisted) 2004-03-23 04:41:25.000-0600

This seems similar to ASTERISK-588 ...  I've made a note in there to check this patch for possible resolution.  Hopefully, we can kill 2 birds with one stone here.
By: twisted (twisted) 2004-04-07 03:51:13

Also, can anyone report if this patch works?  If so, I'd like to see this make it to cvs.
By: Tilghman Lesher (tilghman) 2004-04-11 12:09:41

I've added a patch that I think makes ast_filedelete() do the right thing when called with NULL in the fmt position.  I think that's a better way to go.
By: sbingner (sbingner) 2004-04-11 16:22:09

Your patch doesn't correct the calculation of the duration of the VM message itself...  it only makes ast_deletefile remove the txtfile IF it thinks it was under the minduration.  That will fix the problem of getting the gsm file deleted but the txt file left behind when the length is under the minduration, but the minduration is still majorly skew'd by incorrect data on the length of the message itself.  My patch addresses both problems, and also simplifies the length calculations while correcting the result thereof.

I think it might be good to apply your patch, and remove the unlink code for the txtfile from app_voicemail.c... however that has been in there since the beginning in other areas, so it would need to be cleaned out when your patch was applied.   In short, if your patch makes sense... both should probably be applied.
By: Tilghman Lesher (tilghman) 2004-04-11 16:53:12

Could you describe a bit more about why the duration is being calculated incorrectly, and by how much?  I don't see how your patch makes a difference in the calculated length (more than a single second).  In other words, can you give me a scenario where the duration would be incorrect, and how your patch would fix that?
By: sbingner (sbingner) 2004-04-11 16:56:18

Voicemail currently calcs the duration of the message as the length of the message + the length of the OGM ;)

It also calculates it in multiple places, so my way you have a single instance of the timer and it ignores the OGM
By: sbingner (sbingner) 2004-04-11 17:07:53

I found small bug, where it wouldn't have calc'd time if maxtime was undefined... this fixes that
By: sbingner (sbingner) 2004-04-11 17:30:00

The only change in v.3 is that it also changes play_and_prepend (fixing a typo in play_and_prepend, and applying length calculations) so that the two functions are standard, as well as making sure length is always initialized to 0

play_and_prepend doesn't seem to be used anywhere that it needs to know length, but it seems the two should be standardized
By: Tilghman Lesher (tilghman) 2004-04-23 16:08:52

Mark has objected to my method of fixing this bug, so I've rolled the changes in this patchset into ASTERISK-1352.
By: twisted (twisted) 2004-04-27 16:29:05

Patches for actual bugs should not be closed simply because they are rolled into patches to add features.   Actual fixes should have the ability to be reviewed without adding unnecessary functionality.
By: sbingner (sbingner) 2004-04-27 16:32:19

Added patch against bug 1483 to this, so it can be used in conjunction also.  That version will be backported to current bugfix tonight.
By: Tilghman Lesher (tilghman) 2004-04-27 16:32:45

As I stated above, the fix was reviewed and rejected by Mark.
By: Tilghman Lesher (tilghman) 2004-04-27 16:37:30

In your latest patch, it's invalid code.  You cannot put code before declarations.
By: sbingner (sbingner) 2004-04-27 17:08:32

You said your fix was rejected, was my version rejected?   If so did he say why?

My gcc is strange that way, lets me do it. fixed it....
By: sbingner (sbingner) 2004-04-27 22:05:33

Added version compatible with advanced10 patch, and updated old patch to work the same way
By: sbingner (sbingner) 2004-04-28 15:37:55

advanced10 got integrated into cvs, removed old patch.  advanced10 patch works with CVS
By: sbingner (sbingner) 2004-04-29 06:23:08

Updated to cvs diff, standardized duration to be an integer everywhere (I blame corydon if it needed to be long :b), and fixed a typo in a comment
By: twisted (twisted) 2004-04-29 23:08:40

testers?  anyone?
By: sbingner (sbingner) 2004-05-01 17:58:56

My barf code wasn't barfing, fixed that... I offer my firstborn monkey to anybody who tests this and leaves feedback! :b
By: Mark Spencer (markster) 2004-05-01 18:05:01

Added to CVS, thanks *very much*, this is a nicely done patch.
By: sbingner (sbingner) 2004-05-01 18:06:02

Fixed in CVS
By: Digium Subversion (svnbot) 2008-01-15 14:53:02.000-0600

Repository: asterisk
Revision: 2847

U   trunk/apps/app_voicemail.c

------------------------------------------------------------------------
r2847 | markster | 2008-01-15 14:53:01 -0600 (Tue, 15 Jan 2008) | 2 lines

Merge sbinger's voicemailf ixes for duration and short files (bug ASTERISK-1274) thanks!

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

http://svn.digium.com/view/asterisk?view=rev&revision=2847