[asterisk-dev] [Code Review] 3414: internal_timing: Remove the option and always make it enabled if a timing module is loaded.

[rmudgett]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3414/
-----------------------------------------------------------

Review request for Asterisk Developers.


Bugs: ASTERISK-22846
    https://issues.asterisk.org/jira/browse/ASTERISK-22846


Repository: Asterisk


Description
-------

The masquerade supertest frequently fails because either the local channel chain doesn't completely optimize out or the DTMF handshake doesn't completely get accross.  Local channel optimization requires frames flowing to trigger when optimization can happen.  When optimization happens the media frame that triggered the optimization is dropped.  Sending DTMF requires frames to flow in the other direction for timing purposes while sending nothing.  If internal timing is not enabled when MOH is playing, Asterisk switches to received timing when an audio frame is received.  With optimization dropping media frames and MOH not sending frames unless it receives frames, occasionaly there are no more frames being passed and the test fails.

* The asterisk command line -I option and the asterisk.conf internal_timing option are removed.  Asterisk now always uses internal timing when needed if any timing module is loaded.  The issue ASTERISK-14861 did this quite awhile ago in v1.4 but effectively got broken when other internal timing modules besides DAHDI came along.  The ast_read_generator_actions() now only uses received timing if it has no choice for frame generators like MOH, silence, and playback streaming.

* Cleaned up some code dealing with frame generators in ast_deactivate_generator(), generator_write_format_change(), ast_activate_generator(), and ast_channel_stop_silence_generator().

* Removed ast_internal_timing_enabled(), AST_OPT_FLAG_INTERNAL_TIMING, and ast_opt_internal_timing.  The v1.8 and v11 versions (and possibly v12) will not have these changes as they change the API.

Question:  Should v12 get the API change above or just trunk?


Diffs
-----

  /branches/12/utils/extconf.c 411684 
  /branches/12/main/channel.c 411684 
  /branches/12/main/asterisk.c 411684 
  /branches/12/include/asterisk/options.h 411684 
  /branches/12/include/asterisk/channel.h 411684 
  /branches/12/configs/asterisk.conf.sample 411684 
  /branches/12/channels/chan_sip.c 411684 
  /branches/12/UPGRADE.txt 411684 

Diff: https://reviewboard.asterisk.org/r/3414/diff/


Testing
-------

With the v1.8 and v12 versions of the patch, Asterisk no longer fails the masquerade supertest.  The v11 version should be similar to v1.8.

v12 takes 15-16 seconds to run on the 64 bit build agent.
v1.8 takes 21-22 seconds to run on the 64 bit build agent.

As a side note, the counting instrumentation code I added to v12 to look for this problem showed there was almost no lock contention when grabbing the 7 locks needed for optimization.


Thanks,

rmudgett

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140403/b5ea5f41/attachment.html>