[asterisk-dev] [Code Review] Improve behavior of ast_answer() to not lose incoming frames

[Kevin Fleming]

> On 2009-03-16 16:09:08, Russell Bryant wrote:
> > /trunk/main/channel.c, line 1779
> > <http://reviewboard.digium.com/r/196/diff/1/?file=3068#file3068line1779>
> >
> >     This should actually be INSERT_HEAD().  Otherwise, the frames do not end up in the proper order when the get put back on the channel.

I believe they are in the proper order. They are placed into the 'frames' list in such a way that that list represents them in the actual order received. Then later, they are pulled off that list from the tail end first, and queued onto the channel's queue using ast_queue_frame_head(). This should result in them appearing the channel's queue in the order received.

When my multiframe patch makes its way in, there will be an ast_queue_frame_list() API call that makes this simpler.


> On 2009-03-16 16:09:08, Russell Bryant wrote:
> > /trunk/main/channel.c, lines 1781-1791
> > <http://reviewboard.digium.com/r/196/diff/1/?file=3068#file3068line1781>
> >
> >     I would rather use a switch statement here (with no default case).  That way, as new frame types are added, we will get a compiler warning telling us that this piece of code should be addressed.

Fixed.


> On 2009-03-16 16:09:08, Russell Bryant wrote:
> > /trunk/main/features.c, lines 2483-2493
> > <http://reviewboard.digium.com/r/196/diff/1/?file=3069#file3069line2483>
> >
> >     Hm, I thought we discussed just asking ast_answer() to not do the waiting at all in the case of a bridge?

Yep, Josh brought this on up Jabber as well. New patch will do this properly.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/196/#review547
-----------------------------------------------------------


On 2009-03-16 15:59:55, Kevin Fleming wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/196/
> -----------------------------------------------------------
> 
> (Updated 2009-03-16 15:59:55)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> ast_answer(), when supplied a delay before returning to the caller, use ast_safe_sleep() to implement the delay. Unfortunately during this time any incoming frames are discarded, which is problematic for T.38 re-INVITES and other sorts of channel operations.
> 
> When a delay is not passed to ast_answer(), it still delays for up to 500 milliseconds, waiting for media to arrive. Again, though, it discards any control frames, or non-voice media frames.
> 
> This patch rectifies this situation, by storing all incoming frames during the delay period on a list, and then requeuing them onto the channel before returning to the caller.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/channel.c 182359 
>   /trunk/main/features.c 182359 
> 
> Diff: http://reviewboard.digium.com/r/196/diff
> 
> 
> Testing
> -------
> 
> Compile testing only.
> 
> 
> Thanks,
> 
> Kevin
> 
>