[asterisk-dev] [Code Review] Handle multi-frame lists returned from channel drivers or translators

Kevin Fleming kpfleming at digium.com
Wed Feb 25 18:07:44 CST 2009


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

(Updated 2009-02-25 18:07:44.358010)


Review request for Asterisk Developers.


Changes
-------

This version fixes a number of issues:

1) Frames that had been queued in the ast_read() path were not being freed after ast_queue_frame() duplicated them.

2) If the readtrans in ast_read() returned multiple frames, the excess frames were not always placed into the readq in the proper location.

3) Writing frames to audiohooks in ast_write() happened *before* the call to the writetrans, which made them the wrong format (this will be fixed separately).

4) If the writetrans() in ast_write() returned multiple frames, the excess frames were not passed to the audiohooks and monitor stream on the channel.

5) Some minor code cleanup in ast_write().


Summary
-------

While working on some translator issues, it became apparent that Asterisk has some flaws in handling translators that implement frame-based codecs and that cannot always return translated frames one-for-one for each source frame. That caused me to start looking at various code paths, and found that even the simple multi-frame support in __ast_read() that was added for UDPTL support is not properly implemented.

This patch does a few things:

1) Enhances ast_queue_frame() to be able to accept a list of frames, and places the entire list of frames in the desired location in the channel's readq. The internal-only __ast_queue_frame can also accept an 'after' parameter, so that the frame or list of frames being passed can be inserted into the readq after a specific frame already in the readq.

2) Corrects the multi-frame support existing in ast_read() that is used when a channel drivers's read() function returns a list of frames; now the frames are queued (as they were before), but the alertpipe or timingfd on the channel is also properly updated to reflect the number of frames that were queued, so that when ast_read() returns the thread reading from the channel will immediately wake up again to read the queued frames (one by one).

3) Adds support in __ast_read() for a translation path (on chan->readtrans) returning multiple frames, queuing the excess frames onto the channel's readq in the proper location.

4) Adds support in ast_write() for a translation path (on chan->writetrans) returning multiple frames, feeding each returned frame to the channel driver's write() function in order.


Diffs (updated)
-----

  /branches/1.4/include/asterisk/linkedlists.h 178702 
  /branches/1.4/main/channel.c 178702 

Diff: http://reviewboard.digium.com/r/175/diff


Testing
-------

Compile testing only, so far.


Thanks,

Kevin




More information about the asterisk-dev mailing list