[asterisk-dev] [Code Review] Add unit tests for jitterbuf.c

[Mark Michelson]

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


The test documentation should be altered to make sure to explicitly state that only the adaptive jitterbuffer is tested.

With the exception of the resynch test, the voice and control versions of each test are exactly the same, just with a different frame type specified for jb_put(). The common code can be factored out into functions that take the test name and type of frame to place in the jitterbuffer. This will greatly reduce the size of this file.


/trunk/tests/test_jitterbuffer.c
<https://reviewboard.asterisk.org/r/1815/#comment10640>

    May as well enclose all "expected" and "attribute" uses in parentheses.
    
    Same can apply to the other macros you have here, but this one stands out since the parameters are numeric and expressions are given as parameters to this macro.



/trunk/tests/test_jitterbuffer.c
<https://reviewboard.asterisk.org/r/1815/#comment10645>

    These comments should probably be "Add 6th frame" and "Add 5th frame" since the indexing for 'i' begins at 0.



/trunk/tests/test_jitterbuffer.c
<https://reviewboard.asterisk.org/r/1815/#comment10646>

    Same comment about the comments here.


- Mark


On March 13, 2012, 12:54 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1815/
> -----------------------------------------------------------
> 
> (Updated March 13, 2012, 12:54 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch adds unit tests to cover major functions in jitterbuf.c.  This include jb_put, jb_get, jb_next, as well as general information and statistic gathering.  The scenarios include:
> 
> 1) Nominal - adding voice/control frames and making sure that all added frames are removed and in the correct time slots
> 2) Out of Order - some voice/control frames arrive out of order with respect to the previous frame.  The tests check that when reading from the jitter buffer, the frames are reordered
> 3) Lost - some voice/control frames never arrive.  In the case of voice, the jitter buffer should interpolate the frame; in the case of control, the jitter buffer should report that no frame exists at that time slot
> 4) Late - the arrival time of some voice/control frames does not occur at the same rate as surrounding frames
> 5) Resync - test that voice/control frames that break the resync threshold trigger a resync of the jitter buffer
> 6) Overflow - test that adding voice/control frames that exceed the maximum time length of the buffer are dropped
> 
> This change was prompted by https://reviewboard.asterisk.org/r/1814 - not all tests will pass without that patch.  
> 
> 
> This addresses bug ASTERISK-18964.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18964
> 
> 
> Diffs
> -----
> 
>   /trunk/tests/test_jitterbuffer.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/1815/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt
> 
>

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