[Home]

Summary:ASTERISK-04645: [patch] for "Dropping voice to exceptionally long queue" issues
Reporter:outtolunc (outtolunc)Labels:
Date Opened:2005-07-21 12:18:18Date Closed:2005-08-03 13:08:23
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) channel.c.diff.txt
( 1) channel.c.diff.txt2
Description:i was getting a few of these and took a look at the code.  
qlen was counting all frames and not determining how many were voice and how many were other.  now it does.

Comments:By: Russell Bryant (russell) 2005-07-25 16:19:36

hey, cool!  I noticed that same thing earlier today.  :)

One small comment - I guess there isn't any good reason to keep the qlen variable, now that you split it up.
By: Kevin P. Fleming (kpfleming) 2005-07-25 17:58:12

I think the logic here is not quite right. We should only drop frames if:

1) The frame is "any type", and the queue already contains 128 frames.

- or -

2) The frame is "voice", and the queue already contains 96 voice frames.

As this patch stands, a queue containing 120 non-voice frames will accept up to 96 additional frames if they are all voice.


By: outtolunc (outtolunc) 2005-07-26 11:47:36

the whole point of this was that the old code and this statement were not happening.   /* Allow up to 96 voice frames outstanding, and up to 128 total frames */

and since we are counting all frames with
+        if (cur->frametype == AST_FRAME_VOICE) {
+                 voicelen++;
+         } else {
+ otherlen++;
+ }

and separately looking to qualify the voice, and other with

+        if ((fin->frametype == AST_FRAME_VOICE) && (voicelen > 96)) {
+ ast_log(LOG_DEBUG, "Dropping voice to exceptionally long queue on %s\n", chan->name);
+ ast_frfree(f);
+ ast_mutex_unlock(&chan->lock);
+ return 0;
+ }
+        if ((fin->frametype != AST_FRAME_VOICE) && ((voicelen + otherlen)  > 128)) {
+ ast_log(LOG_WARNING, "Exceptionally long queue length queuing to %s\n", chan->name);
+ CRASH;

note the second one is doing voicelen + otherlen so, no you couldn't have 96 + 120 (your example)


but as always, feel free to butcher it as you feel nessesary <G>
By: Kevin P. Fleming (kpfleming) 2005-07-26 12:15:23

No, that's incorrect. The second if statement only checks the queue length for when processing a non-voice frame, if the frame is voice and there are not more than 96 voice frames already queued it will get queued, regardless of how many non-voice frames are already queued.
By: outtolunc (outtolunc) 2005-07-26 12:20:45

ok, so remove the frametype checks on both?

that way if voicelen > 96 free, and if both greater than 128 puke
By: outtolunc (outtolunc) 2005-07-26 12:38:56

channel.c.diff.txt2 has the frametype checks and qlen removed.
By: Mark Spencer (markster) 2005-08-03 00:49:53

The current logic says "If there are < 96 frames or there are < 128 frames and this is not a voice frame then allow it to be queued."  What's wrong with that logic?
By: outtolunc (outtolunc) 2005-08-03 11:22:37

actually nothing, i misread it when i posted.  

the problem that's causing the error i'm sure is elsewhere.

(watches his karma fly away) <G>
By: Michael Jerris (mikej) 2005-08-03 13:07:56

This appears to be a non issue then.