Summary: | ASTERISK-15559: [patch] DSP progress detection unable to detect SIT | ||
Reporter: | dant (dant) | Labels: | |
Date Opened: | 2010-02-02 00:34:37.000-0600 | Date Closed: | 2010-05-19 01:41:58 |
Priority: | Minor | Regression? | No |
Status: | Closed/Complete | Components: | Core/General |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ( 0) dsp.c-bug16749-1.patch | |
Description: | The DSP should be able to detect the SIT. The existing code checks for the first tone followed by the second tone, followed by the third tone in sequence and on detection of the third tone allows firing a CONGESTION control frame. This unfortunately doesn't work due to the fact that entire SIT is about 1 second long, the DSP is working with smaller samples and the code will not stay in a second or third tone state beyond one pass. This issue is being raised against trunk, but, the problem appears to be present even in 1.2. ****** ADDITIONAL INFORMATION ****** They key bit of code is... <snip> } else if (hz[HZ_950] > TONE_MIN_THRESH * TONE_THRESH) { newstate = DSP_TONE_STATE_SPECIAL1; } else if (hz[HZ_1400] > TONE_MIN_THRESH * TONE_THRESH) { if (dsp->tstate == DSP_TONE_STATE_SPECIAL1) newstate = DSP_TONE_STATE_SPECIAL2; } else if (hz[HZ_1800] > TONE_MIN_THRESH * TONE_THRESH) { if (dsp->tstate == DSP_TONE_STATE_SPECIAL2) { newstate = DSP_TONE_STATE_SPECIAL3; } <snip> } else { newstate = DSP_TONE_STATE_SILENCE; } <snip> When a SIT is present, the DSP processes each frame, it detects the 950Hz tone for 16 cycles and stays in DSP_TONE_STATE_SPECIAL1 state for that time, then the 1400Hz tone starts and it switches DSP_TONE_STATE_SPECIAL2 due to the 1400Hz tone being present and the previous state being DSP_TONE_STATE_SPECIAL1, however, on the next cycle the 1400Hz tone is still present and the current state is DSP_TONE_STATE_SPECIAL2, so, the default state of DSP_TONE_STATE_SILENCE is set and the sequence can never complete. | ||
Comments: | By: dant (dant) 2010-02-02 00:37:21.000-0600 Relevant debug output from reception of two SITs a couple of seconds apart: [Feb 2 13:25:42] DEBUG[10497] dsp.c: Stop state 0 with duration 0 [Feb 2 13:25:42] DEBUG[10497] dsp.c: Start state 5 [Feb 2 13:25:42] DEBUG[10497] dsp.c: Stop state 5 with duration 15 [Feb 2 13:25:42] DEBUG[10497] dsp.c: Start state 6 [Feb 2 13:25:42] DEBUG[10497] dsp.c: Stop state 6 with duration 1 [Feb 2 13:25:42] DEBUG[10497] dsp.c: Start state 0 [Feb 2 13:25:45] DEBUG[10497] dsp.c: Stop state 0 with duration 121 [Feb 2 13:25:45] DEBUG[10497] dsp.c: Start state 5 [Feb 2 13:25:45] DEBUG[10497] dsp.c: Stop state 5 with duration 16 [Feb 2 13:25:45] DEBUG[10497] dsp.c: Start state 6 [Feb 2 13:25:45] DEBUG[10497] dsp.c: Stop state 6 with duration 1 [Feb 2 13:25:45] DEBUG[10497] dsp.c: Start state 0 By: dant (dant) 2010-02-02 00:43:32.000-0600 Patch attached that allows the second and third tones to be present across multiple dsp calls allowing the sequence to complete... New debug output: [Feb 2 14:10:20] DEBUG[11515] dsp.c: Stop state 0 with duration 0 [Feb 2 14:10:20] DEBUG[11515] dsp.c: Start state 5 [Feb 2 14:10:21] DEBUG[11515] dsp.c: Stop state 5 with duration 15 [Feb 2 14:10:21] DEBUG[11515] dsp.c: Start state 6 [Feb 2 14:10:21] DEBUG[11515] dsp.c: Stop state 6 with duration 15 [Feb 2 14:10:21] DEBUG[11515] dsp.c: Start state 7 By: Tilghman Lesher (tilghman) 2010-05-18 14:42:34 The patch attached does not do what you indicate above. In fact, it doesn't do anything. I would ascribe any testing results to be purely coincidental or could be related to another change you've made. By: dant (dant) 2010-05-18 18:27:41 tilghman, Are you really sure that that is the case? This was the only change to dsp.c... Looking at the code again, this appears to be called for each frame, 20ms long... An SIT, as you know, is made up of approx 330ms of 950hz, followed by approx 330ms of 1400hz, followed by approx 300ms of 1800hz... So, for the first frame of SIT, 950hz would be detected and newstate would be set to 5 (DSP_TONE_STATE_SPECIAL1), for the subsequent 15 frames of 950hz newstate would continue to be set to 5, on line 1023 newstate would be compared to the current state (dsp->tstate) and if it was the same, dsp->tcount would be incremented, so, reaching 15... for the first frame that 1400hz is detected, the current state would be first checked to make sure it was 5, if it was, newstate would be set to 6 (DSP_TONE_STATE_SPECIAL2), for the second frame that 1400hz was detected the current state would be 6, so the check to make sure it was 5 would fail, and newstate would not be set. At line 1023 dsp->tstate would not equal newstate as newstate would still have it's initialised value of 0 (DSP_TONE_STATE_SILENCE) due to the fact that while 1400hz was found, the current state was not 5, as such, at line 1071 the debug messages above would be output and the current state would be set to newstate, in this case 0. There could never be a way for the switch statement on line 1028 to ever match on a current state of 7 even if the SIT was 3 frames long... The patch does change this behavior, on the second frame of 1400hz being detected the enclosed if statement would evaluate true as the current state of 6 does match 5 || 6... as a result, at line 1023 it would find that the state hadn't changed, it wouldn't reset the state to 0 as line 1071 would never by hit and for all subsequent instances of 1400hz newstate would continue to be set to 6 allowing for an eventual setting of newstate to 7 (DSP_TONE_STATE_SPECIAL3) when the first 1800hz frame was detected such that on the second frame the switch statement on line 1028 would match a current state of 7, and on the 4th frame of that the threshold would be reached and res would be set to AST_CONTROL_CONGESTION... To me, that does indeed seem to resolve the logic problem with the current code... Thanks... By: Tilghman Lesher (tilghman) 2010-05-19 01:17:51 Ah, I think I see. You're referring to a much lower if clause than I was looking at. By: Digium Subversion (svnbot) 2010-05-19 01:32:27 Repository: asterisk Revision: 263949 U branches/1.4/main/dsp.c ------------------------------------------------------------------------ r263949 | tilghman | 2010-05-19 01:32:27 -0500 (Wed, 19 May 2010) | 8 lines Because progress is called multiple times, across several frames, we must persist states when detecting multitone sequences. (closes issue ASTERISK-15559) Reported by: dant Patches: dsp.c-bug16749-1.patch uploaded by dant (license 670) Tested by: dant ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=263949 By: Digium Subversion (svnbot) 2010-05-19 01:41:04 Repository: asterisk Revision: 263950 _U trunk/ U trunk/main/dsp.c ------------------------------------------------------------------------ r263950 | tilghman | 2010-05-19 01:41:04 -0500 (Wed, 19 May 2010) | 15 lines Merged revisions 263949 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r263949 | tilghman | 2010-05-19 01:32:27 -0500 (Wed, 19 May 2010) | 8 lines Because progress is called multiple times, across several frames, we must persist states when detecting multitone sequences. (closes issue ASTERISK-15559) Reported by: dant Patches: dsp.c-bug16749-1.patch uploaded by dant (license 670) Tested by: dant ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=263950 By: Digium Subversion (svnbot) 2010-05-19 01:41:58 Repository: asterisk Revision: 263951 _U branches/1.6.2/ U branches/1.6.2/main/dsp.c ------------------------------------------------------------------------ r263951 | tilghman | 2010-05-19 01:41:57 -0500 (Wed, 19 May 2010) | 22 lines Merged revisions 263950 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r263950 | tilghman | 2010-05-19 01:41:04 -0500 (Wed, 19 May 2010) | 15 lines Merged revisions 263949 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r263949 | tilghman | 2010-05-19 01:32:27 -0500 (Wed, 19 May 2010) | 8 lines Because progress is called multiple times, across several frames, we must persist states when detecting multitone sequences. (closes issue ASTERISK-15559) Reported by: dant Patches: dsp.c-bug16749-1.patch uploaded by dant (license 670) Tested by: dant ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=263951 |