<p>Friendly Automation <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15700">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved
Friendly Automation: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_rtp_asterisk: Don't count 0 as a minimum lost packets<br><br>The calculated minimum lost packets represents the lowest number of<br>lost packets missed during an RTCP report interval. Zero of course<br>is the lowest, but the idea is that this value contain the lowest<br>number of lost packets once some have been missed.<br><br>This patch checks to make sure the number of lost packets over an<br>interval is not zero before checking and setting the minimum value.<br><br>Also, this patch updates the rtp lost packet test to check for<br>packet loss over several reports vs one.<br><br>Change-Id: I07d6e21cec61e289c2326138d6bcbcb3c3d5e008<br>---<br>M res/res_rtp_asterisk.c<br>M tests/test_res_rtp.c<br>2 files changed, 38 insertions(+), 4 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c</span><br><span>index 16c3be0..4fd803b 100644</span><br><span>--- a/res/res_rtp_asterisk.c</span><br><span>+++ b/res/res_rtp_asterisk.c</span><br><span>@@ -4472,7 +4472,7 @@</span><br><span> if (rtp->rtcp->rxlost_count == 0) {</span><br><span> rtp->rtcp->minrxlost = rtp->rtcp->rxlost;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- if (lost_interval < rtp->rtcp->minrxlost) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lost_interval && lost_interval < rtp->rtcp->minrxlost) {</span><br><span> rtp->rtcp->minrxlost = rtp->rtcp->rxlost;</span><br><span> }</span><br><span> if (lost_interval > rtp->rtcp->maxrxlost) {</span><br><span>diff --git a/tests/test_res_rtp.c b/tests/test_res_rtp.c</span><br><span>index 08c9971..1d36116 100644</span><br><span>--- a/tests/test_res_rtp.c</span><br><span>+++ b/tests/test_res_rtp.c</span><br><span>@@ -269,7 +269,7 @@</span><br><span> RAII_VAR(struct ast_rtp_instance *, instance2, NULL, ast_rtp_instance_destroy);</span><br><span> RAII_VAR(struct ast_sched_context *, test_sched, NULL, ast_sched_context_destroy_wrapper);</span><br><span> struct ast_rtp_instance_stats stats = { 0, };</span><br><span style="color: hsl(0, 100%, 40%);">- enum ast_rtp_instance_stat stat = AST_RTP_INSTANCE_STAT_RXPLOSS;</span><br><span style="color: hsl(120, 100%, 40%);">+ enum ast_rtp_instance_stat stat = AST_RTP_INSTANCE_STAT_ALL;</span><br><span> </span><br><span> switch (cmd) {</span><br><span> case TEST_INIT:</span><br><span>@@ -303,8 +303,42 @@</span><br><span> </span><br><span> /* Check RTCP stats to see if we got the expected packet loss count */</span><br><span> ast_rtp_instance_get_stats(instance2, &stats, stat);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_test_validate(test, stats.rxploss == 5,</span><br><span style="color: hsl(0, 100%, 40%);">- "Condition of 5 lost packets was not met");</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_test_validate(test, stats.rxploss == 5 && stats.local_minrxploss == 5 &&</span><br><span style="color: hsl(120, 100%, 40%);">+ stats.local_maxrxploss == 5, "Condition of 5 lost packets was not met");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Drop 3 before writing 5 more */</span><br><span style="color: hsl(120, 100%, 40%);">+ test_write_and_read_frames(instance1, instance2, 1023, 5);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_rtp_instance_queue_report(instance1);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_write_frames(instance2, 1001, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_rtp_instance_get_stats(instance2, &stats, stat);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Should now be missing 8 total packets with a change in min */</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_test_validate(test, stats.rxploss == 8 && stats.local_minrxploss == 3 &&</span><br><span style="color: hsl(120, 100%, 40%);">+ stats.local_maxrxploss == 5);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Write 5 more with no gaps */</span><br><span style="color: hsl(120, 100%, 40%);">+ test_write_and_read_frames(instance1, instance2, 1028, 5);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_rtp_instance_queue_report(instance1);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_write_frames(instance2, 1002, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_rtp_instance_get_stats(instance2, &stats, stat);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Should still only be missing 8 total packets */</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_test_validate(test, stats.rxploss == 8 && stats.local_minrxploss == 3 &&</span><br><span style="color: hsl(120, 100%, 40%);">+ stats.local_maxrxploss == 5);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Now drop 1, write another 5, drop 8, and then write 5 */</span><br><span style="color: hsl(120, 100%, 40%);">+ test_write_and_read_frames(instance1, instance2, 1034, 5);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_write_and_read_frames(instance1, instance2, 1047, 5);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_rtp_instance_queue_report(instance1);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_write_frames(instance2, 1003, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_rtp_instance_get_stats(instance2, &stats, stat);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Now it should be missing 17 total packets, with a change in max */</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_test_validate(test, stats.rxploss == 17 && stats.local_minrxploss == 3 &&</span><br><span style="color: hsl(120, 100%, 40%);">+ stats.local_maxrxploss == 9);</span><br><span> </span><br><span> return AST_TEST_PASS;</span><br><span> }</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15700">change 15700</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/15700"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I07d6e21cec61e289c2326138d6bcbcb3c3d5e008 </div>
<div style="display:none"> Gerrit-Change-Number: 15700 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>