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

[Kevin Fleming]

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

(Updated 2009-02-26 18:16:33.510785)


Review request for Asterisk Developers.


Changes
-------

Final set of changes for this round of stuff:

1) Update app_chanspy, app_mixmonitor, app_meetme, slinfactory and file streams to be able to handle multiple-frame results from translation paths.

2) Remove broken and incomplete TRACE_FRAMES support that conflicted with any other use of the 'frame_list' field in 'struct ast_frame'.

3) Improve ast_frame_free()/ast_frfree() to be able to accept a chained list of frames and free them all.

4) Some minor optimizations in ast_frisolate(), and fix two bugs where it would copy pointers to malloc'd data from the source frame but not clear those pointers in the source frame (resulting in a likely crash if the source frame was ever freed). In essence, the output frame was *not* fully isolated from the source frame. There were only two users in the tree and neither of them could cause this bug, though.

5) Use ast_frisolate() in the autoservice run thread, to avoid actually duplicating a frame if it is not necessary.


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/apps/app_chanspy.c 178984 
  /branches/1.4/apps/app_meetme.c 178984 
  /branches/1.4/apps/app_mixmonitor.c 178984 
  /branches/1.4/build_tools/cflags-devmode.xml 178984 
  /branches/1.4/include/asterisk/frame.h 178984 
  /branches/1.4/include/asterisk/linkedlists.h 178984 
  /branches/1.4/main/autoservice.c 178984 
  /branches/1.4/main/channel.c 178984 
  /branches/1.4/main/file.c 178984 
  /branches/1.4/main/frame.c 178984 
  /branches/1.4/main/slinfactory.c 178984 

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


Testing
-------

Compile testing only, so far.


Thanks,

Kevin