Summary:	ASTERISK-13594: [patch] DTMF blips at end of recordings in __ast_play_and_record()
Reporter:	Michael Smith (asbestoshead)	Labels:
Date Opened:	2009-02-17 10:39:44.000-0600	Date Closed:	2011-06-07 14:08:02
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Applications/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) patch-play-record-buttondown-160b9.txt
Description:	When using a channel with real in-band DTMF (e.g. Dahdi), voicemail greetings and other recordings have remnants of the DTMF tone left at the end. Usually it's about 5-15ms of DTMF before the Dahdi DTMF detector kicked in, and sometimes there's an additional 5ms at the end of the tone. The attached patch stops recording on button DOWN, not button UP; it also trims the last 400ms from the recording in the case where it was terminated by DTMF. I'm not 100% happy with this, but it seems to completely get rid of noisy blips at the end of recordings a little over 50% (*) of the time, and I haven't heard complaints about it truncating too much. I'm sort of looking for feedback more than anything. The patch is against 1.6.0-beta9, but it applies cleanly to 1.6.0.5. (*) 24 out of 43 users who changed their greetings since I applied the patch. Some of them might have retried if they heard the blip, but most probably wouldn't notice. I didn't notice, until it was pointed out to me -- and since then it's all I can hear :) ****** ADDITIONAL INFORMATION ****** asterisk-users thread: http://thread.gmane.org/gmane.comp.telephony.pbx.asterisk.user/224229/focus=224240
Comments:	By: Leif Madsen (lmadsen) 2009-07-13 10:42:43 Assigning this target for 1.6.0.12, but curious if this is still an issue on the latest 1.6.0 version? Thanks! By: Dmitry Andrianov (dimas) 2009-07-15 06:27:49 1. If you are checking for AST_FRAME_DTMF_BEGIN then the second check should be for AST_FRAME_DTMF_END and not just AST_FRAME_DTMF. These are the same values so nothing really changes. The idea is that old code uses AST_FRAME_DTMF which is the end while newer code knows both begin and end. 2. Your comment says 5-25 ms but AFAIK ast_stream_rewind takes milliseconds so you are trimming 400 ms. 3. Are you sure that long trim is really necessay? To my knowledge if you are catching DTMF_BEGIN, then only very short blip ( < 20 ms ) of DTMF signal will be in recorded audio prior to that event. 4. If you ae beaking by DTMF_BEGIN, you should be pepared that DTMF_END will be received by another piece of code which is not good really. My understanding of proper implementation is: you detect both BEGIN and END. You take actions on END only. However BEGIN gives you point to cut your recording to (-20ms). So you act when you got END: if you saw BEGIN before - you cut to it minus 20ms. If you had no BEGIN, you just cut 400 ms from the end. By: Michael Smith (asbestoshead) 2009-07-24 15:00:02 Hi, thanks for the feedback. lmadsen: I can't say for sure as I'm stuck on an earlier beta until CONNECTED_LINE support makes it into a release. Looking at the code, I think it would still be an issue. I see that __ast_play_and_record() has some changes for the case where the recording is NOT ended with DTMF, to fix *duration. The patch here might need to be updated to do the same, but either way, the new code probably has the same problem. dimas: Yes, it looks like the comment in the patch is wrong, I'm trimming 400 ms. I think there can be a long bounce period when the user mashes '#' before the software DTMF detector kicks in (on another Asterisk instance, with a Dahdi channel). There is also some click noise from the buttons on some phones before the DTMF is generated. I admit I wasn't very scientific about it, but 400 ms gave decent results. Your suggestion to record the time DTMF_BEGIN was received, then wait for DTMF_END, is probably the right way to do it, although I would still cut 400 ms. jpeeler: I probably won't have a chance to update the patch anytime soon, but if you would like me to test anything for you, I can. By: Dmitry Andrianov (dimas) 2009-07-25 14:44:17 cutting 400 ms always seems to me a wrong thing. This is because lots of phones produce very long beep as lonf as you hold the button. From the other hand, BEGIN frame always give you information where the DTMF actually started and where you should cut. To my knowledge the facts are: 1. When DTMFs are cocming from DAHDI channel, the BEGIN frame should be received no latter than 20 ms of the moment the DTMF actually started.  (you may assume 50 ms for weak signal and button click). 2. DAHDI driver itself suppresses the DTNF tone from recorded signal (except for the first 20 ms). So in the case of proper operations, you will get 0-20ms  DTMF audio, then DTMF BEGIN frame then silence for the duration on DTMF and then DTMF end frame. So again, the right thing is to cut at the moment you've got BEGIN - 20-50ms. 3. In the case of SIP and RFC2833 DTMF there is no DTMF audio already in thee stream - you will probably be recording silence between BEGIN and END frames so the approach above should work fine too. 4. In the case of SIP and INFO DTMF, you will be receiving DTMF END frame only (probably preceeded by silence). So ideal approach would be to just cut silence at the end. Just my thoughts. By: Michael Smith (asbestoshead) 2009-07-26 10:13:11 The current patch cuts 400ms before BEGIN, so however long the beep is, it'll be cut. It cuts 400ms before BEGIN as a compromise between wanting to cut the most common "bounce" periods I was seeing, and not wanting to cut speech. I was testing with Mitel Superset 4025 phones connected to a Mitel SX-200, connected to Asterisk 1.6.0.x by T1 E&M (Dahdi), to Asterisk 1.6.0b9 by SIP. The Mitel keys are bouncy, and the Dahdi DTMF detector is fairly conservative, sosometimes it takes 400ms or longer from the instant the '#' is hit until the tone kicks in solid enough for Dahdi to detect and suppress it. By: Jeff Peeler (jpeeler) 2009-07-28 18:21:42 I've been looking at this and haven't found a good solution yet. We can't just trim 400 ms of audio (or really any uncalculated amount). I don't see how timing the duration between DTMF_END and DTMF_BEGIN events is going to help because the dtmf frames are not written to the file. The file duration must be calculated as the comment reports using the file length and not some form of timing. The only thing I can think of so far is to mute more fragments previous to detection. By: Dmitry Andrianov (dimas) 2009-08-02 13:40:53 jpeeler, I'm not sure I really understand what you mean by "I don't see how timing the duration between DTMF_END and DTMF_BEGIN events is going to help because the dtmf frames are not written to the file". Are you saying that between BEGIN and END frames no audio at all goes to the file? (I mean that even silence is not recorded) ? By: Jeff Peeler (jpeeler) 2009-08-03 16:11:08 What I was saying is that the DTMF frames do not get written to the file (technically). Timing the duration between the begin and end events would only be useful to use against a full timing of all the audio written to the file as far as I can tell. The full length of the file is calculated based on the blips being written to the file as voice data, but excluding the actual DTMF. What this means is if the terminating DTMF key is pressed for a long duration far too much audio will be trimmed. asbestoshead: If you can update the patch to make the trimming optional the patch can be merged. Since you said it will be a while before you can update the patch I'm suspending this issue for now. By: Michael Smith (asbestoshead) 2009-08-03 19:43:26 Reopening so I can comment. jpeeler: I think what dimas is suggesting is: 1. On DTMF_BEGIN: remember current position as "dtmf_detected" 2. On DTMF_END: if no dtmf_detected (i.e. no DTMF_BEGIN), use the current position as dtmf_detected 3. When done (DTMF_END or recording stopped): if dtmf_detected, rewind the end of the file to be dtmf_detected MINUS n milliseconds. For me, n = 400. I could see this as a configurable option where the default value (0) cuts the recording as soon as DTMF_BEGIN is received, and a positive value cuts the recording at dtmf_detected minus n. Where would you like this to be configured? Should it be a voicemail setting passed into __ast_play_and_record() as another parameter, or is there a better way? By: Jeff Peeler (jpeeler) 2009-08-04 12:24:58 My previous comment was not clear in that I did not describe that while a key is pressed and held down using an analog phone null frames are returned. This was my main concern for confusing the timing. Putting the option in voicemail.conf would be fine and adding the setting as another parameter to __ast_play_and_record would be fine too. asbestoshead: are you going to work on the patch? By: Dmitry Andrianov (dimas) 2009-08-04 14:05:09 It is possible to handle null frames easily. We should just stop recording on BEGIN frame (dsp.c generates these even for analog lines). However, again, after we stopped recording because of BEGIN frame we should wait for its END counterpart.... By: Jeff Peeler (jpeeler) 2009-08-14 18:15:53 dimas: Can you submit a patch with what you had in mind? If you receive a DTMF_BEGIN event and then receive a bunch of NULL frames, were you just going to reset the timing for the begin event? By: Jeff Peeler (jpeeler) 2009-08-17 12:04:25 So again, I'm suspending this since this is really a feature request. Please only reopen for a patch submission.