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

[rmudgett]

> On April 3, 2014, 1:10 p.m., Mark Michelson wrote:
> > /branches/12/main/channel.c, line 3664
> > <https://reviewboard.asterisk.org/r/3414/diff/1/?file=57005#file57005line3664>
> >
> >     In chan_sip.c the replacement for ast_internal_timing_enabled() was to check if chan->timingfd was valid. Here, you check for the existence of ast_channel_timingfunc(). What's the reason for the difference?

The chan_sip.c test checks to see if internal timing is available.  The timingfunc test checks to see if frames are being generated by the timerfd at this time.


> On April 3, 2014, 1:10 p.m., Mark Michelson wrote:
> > /branches/12/main/channel.c, lines 3668-3671
> > <https://reviewboard.asterisk.org/r/3414/diff/1/?file=57005#file57005line3668>
> >
> >     Any reason you killed the debug message that used to announce this?

It is a comment now rather than a debug message because we no longer switch between internal timing and loop timing modes.  We are always one or the other when we are generating frames.


- rmudgett


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


On April 3, 2014, 11:52 a.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3414/
> -----------------------------------------------------------
> 
> (Updated April 3, 2014, 11:52 a.m.)
> 
> 
> 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/a5acb3ee/attachment.html>