Summary:	ASTERISK-11110: [patch] multiple issues with autoservice
Reporter:	Dmitry Andrianov (dimas)	Labels:
Date Opened:	2007-12-26 14:30:52.000-0600	Date Closed:	2011-06-07 14:08:20
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/Channels
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 11635.patch ( 1) v2-11635.patch
Description:	Based on issue 11628 and IRC conversation with russellb. This issue is mostly a reminder for Russell because he asked for reminder. The patch attached is how I see the solution. WARNING: I had no chance to test this patch because i'm having problems on production machine I do not have access for a few next days. Sorry about that. ****** ADDITIONAL INFORMATION ****** The idea is to mark all frames which were read by autoservice so when autoservice queues them back to the channel (when stopped) and someone calls ast_read, these frames won't go thru all that processing again. This should solve subtle issues where frames produced by DTMF emulation code of ast_read become input of that emulation code later. Frame's rarely used has_timing_info field is converted to a bit mask. I believe ABI is not broken because field size has not changed, the same value (1) old code used to put into has_timing_info is now mapped to AST_FRFLAG_HAS_TIMING_INFO and AST_FRFLAG_READ flag is not be seen by any code except ast_read. Hovewer, since patch is not tested :( review is necessary.
Comments:	By: Digium Subversion (svnbot) 2007-12-27 13:22:18.000-0600 Repository: asterisk Revision: 94973 A   team/russell/autoservice-1.4/ ------------------------------------------------------------------------ r94973 | russell | 2007-12-27 13:22:16 -0600 (Thu, 27 Dec 2007) | 4 lines create a branch to work on issue ASTERISK-11110.  dimas keeps coming up with situations which I have not accounted for in frame handling while a channel is in autoservice, so I'm going to take a different approach ... ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=94973 By: Dmitry Andrianov (dimas) 2008-01-08 17:15:25.000-0600 Russell, there is another issue which can be fixed with the patch (alhough the initial patch I submitted does not fix it). The problem: 1. DSP with DTMF detection is initialized for SIP channel. The native format for SIP channel is ulaw. 2. Application which terminates SIP call (fax in my case) sets read format to SLINEAR. 3. When DSP detects DTMF frequency, it zeroes out frame audio data and queue frame back to the channel. 4. ast_read gets voice frame from readq and immediately drops it with following warning: NOTICE[11156] channel.c: Dropping incompatible voice frame on SIP/10.0.0.10 of format slin since our native format has changed to ulaw This warning be easily avoided by marking modified voice frame with FRAMEOPT_READ when DSP queues modified frame back. This was ast_read just returns the frame immediately without any urther processin which is right because it was already translated to the target read format before. By: Dmitry Andrianov (dimas) 2008-02-08 08:22:17.000-0600 since commit 98943 already introduced flags for the frame, the patch does not apply anymore. Fixed. This also made the patch tiny (because biggest part of it was introduction of flags field) However.... After your explanation on IRC regarding how DTMF post-processing in ast_read works, I start feeling a bit pessimistic about the approach. Will discuss on IRC... By: Tilghman Lesher (tilghman) 2008-02-27 18:41:23.000-0600 dimas: any update here? By: Dmitry Andrianov (dimas) 2008-02-28 08:04:24.000-0600 Well... The main problem this patch was intended to solve is the issues with VDTMF emulation when channel is periodically put to autoservice and back. Now I think that this particular problem needs to be solved in completely different way - by moving the VDTMF normalization code from _ast_read into ast_write. In this sense this patch is not needed anymore... By: Dmitry Andrianov (dimas) 2008-05-20 10:11:51 This specific issue is now outdated - I filled new one (12658) with the patch which is superset of changes here. By: Tilghman Lesher (tilghman) 2008-05-20 11:23:10 Closed on request of reporter.