[asterisk-dev] [Code Review] Fix incorrect jitterbuffer overflow when timing source changes are indicated by control frames

[Matt Jordan]

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

Review request for Asterisk Developers.


Summary
-------

When a change in time occurs, such that the timestamps associated with frames being placed into an adaptive Jitter Buffer are significantly different then the previously inserted frames, the adaptive Jitter Buffer checks to see if it needs to be resynched to the new time frame (based on an initially configurable re-synchronization threshold).  If three consecutive packets break the threshold, the jitter buffer re-synchs itself to the new timestamps.  This currently only occurs when the history is calculated, and hence only on VOICE frames.

CONTROL frames, on the other hand, are never passed to the history calculations.  Because of this, if the jump in time is greater then the maximum allowed length of the Jitter Buffer, the CONTROL frames are dropped and no re-synchronization occurs.  Because the re-synch does not happen, subsequent VOICE frames are counted as overflow of the Jitter Buffer, and are also dropped.  In some scenarios, this can lead to 'stuttery' audio as some number of VOICE frames are dropped.

This patch allows CONTROL frames to re-synch the Jitter Buffer.  As CONTROL frames are very unlikely to occur in multiples, it performs the resynchornization on any CONTROL frame that breaks the re-synch threshold.


This addresses bug ASTERISK-18964.
    https://issues.asterisk.org/jira/browse/ASTERISK-18964


Diffs
-----

  /branches/1.8/main/jitterbuf.c 358724 

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


Testing
-------

Testing was done by the initial issue reporter.  I've also verified that the patch does not adversely effect nominal operation of IAX2 channels with a forced jitter buffer.

Unit tests were written to cover this functionality and to verify that other behavior did not change with this patch (see review https://reviewboard.asterisk.org/r/1815/).  Since the unit tests are a new module, I put them under their own review against trunk.  Without this patch, the unit test jitterbuffer_resynch_control will fail, as the jitter buffer will not be resynced and will instead overflow.  The rest of the unit tests have the same result with and without this patch.


Thanks,

Matt

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120313/714776ef/attachment.htm>