<p>Friendly Automation <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15842">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve
Kevin Harwell: 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: More robust timestamp checking<br><br>We assume that a timestamp value of 0 represents an 'uninitialized'<br>timestamp, but 0 is a valid value. Add a simple wrapper to be able to<br>differentiate between whether the value is set or not.<br><br>This also removes the fix for ASTERISK~28812 which should not be<br>needed if we are checking the last timestamp appropriately.<br><br>ASTERISK-29030 #close<br><br>Change-Id: Ie70d657d580d9a1f2877e25a6ef161c5ad761cf7<br>---<br>M res/res_rtp_asterisk.c<br>1 file changed, 19 insertions(+), 9 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 607fd91..20504cb 100644</span><br><span>--- a/res/res_rtp_asterisk.c</span><br><span>+++ b/res/res_rtp_asterisk.c</span><br><span>@@ -356,6 +356,11 @@</span><br><span> int schedid;</span><br><span> };</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+typedef struct {</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int ts;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned char is_set;</span><br><span style="color: hsl(120, 100%, 40%);">+} optional_ts;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! \brief RTP session description */</span><br><span> struct ast_rtp {</span><br><span> int s;</span><br><span>@@ -392,7 +397,7 @@</span><br><span> /* DTMF Reception Variables */</span><br><span> char resp; /*!< The current digit being processed */</span><br><span> unsigned int last_seqno; /*!< The last known sequence number for any DTMF packet */</span><br><span style="color: hsl(0, 100%, 40%);">- unsigned int last_end_timestamp; /*!< The last known timestamp received from an END packet */</span><br><span style="color: hsl(120, 100%, 40%);">+ optional_ts last_end_timestamp; /*!< The last known timestamp received from an END packet */</span><br><span> unsigned int dtmf_duration; /*!< Total duration in samples since the digit start event */</span><br><span> unsigned int dtmf_timeout; /*!< When this timestamp is reached we consider END frame lost and forcibly abort digit */</span><br><span> unsigned int dtmfsamples;</span><br><span>@@ -5536,12 +5541,13 @@</span><br><span> }</span><br><span> </span><br><span> if (ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_DTMF_COMPENSATE)) {</span><br><span style="color: hsl(0, 100%, 40%);">- if ((rtp->last_end_timestamp != timestamp) || (rtp->resp && rtp->resp != resp)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!rtp->last_end_timestamp.is_set || rtp->last_end_timestamp.ts != timestamp || (rtp->resp && rtp->resp != resp)) {</span><br><span> rtp->resp = resp;</span><br><span> rtp->dtmf_timeout = 0;</span><br><span> f = ast_frdup(create_dtmf_frame(instance, AST_FRAME_DTMF_END, ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_DTMF_COMPENSATE)));</span><br><span> f->len = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->last_end_timestamp = timestamp;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->last_end_timestamp.ts = timestamp;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->last_end_timestamp.is_set = 1;</span><br><span> AST_LIST_INSERT_TAIL(frames, f, frame_list);</span><br><span> }</span><br><span> } else {</span><br><span>@@ -5560,8 +5566,9 @@</span><br><span> </span><br><span> if (event_end & 0x80) {</span><br><span> /* End event */</span><br><span style="color: hsl(0, 100%, 40%);">- if ((rtp->last_seqno != seqno) && ((timestamp > rtp->last_end_timestamp) || ((timestamp == 0) && (rtp->last_end_timestamp == 0)))) {</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->last_end_timestamp = timestamp;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rtp->last_seqno != seqno && (!rtp->last_end_timestamp.is_set || timestamp > rtp->last_end_timestamp.ts)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->last_end_timestamp.ts = timestamp;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->last_end_timestamp.is_set = 1;</span><br><span> rtp->dtmf_duration = new_duration;</span><br><span> rtp->resp = resp;</span><br><span> f = ast_frdup(create_dtmf_frame(instance, AST_FRAME_DTMF_END, 0));</span><br><span>@@ -5581,7 +5588,8 @@</span><br><span> * 65535.</span><br><span> */</span><br><span> if ((rtp->last_seqno > seqno && rtp->last_seqno - seqno < 50)</span><br><span style="color: hsl(0, 100%, 40%);">- || timestamp <= rtp->last_end_timestamp) {</span><br><span style="color: hsl(120, 100%, 40%);">+ || (rtp->last_end_timestamp.is_set</span><br><span style="color: hsl(120, 100%, 40%);">+ && timestamp <= rtp->last_end_timestamp.ts)) {</span><br><span> /* Out of order frame. Processing this can cause us to</span><br><span> * improperly duplicate incoming DTMF, so just drop</span><br><span> * this.</span><br><span>@@ -6671,7 +6679,7 @@</span><br><span> * re-transmissions of the last dtmf end still. Feed those to the</span><br><span> * core so they can be filtered accordingly.</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- if (rtp->last_end_timestamp == timestamp) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rtp->last_end_timestamp.is_set && rtp->last_end_timestamp.ts == timestamp) {</span><br><span> ast_debug_rtp(1, "(%p, %p) RTP feeding packet with duplicate timestamp to core\n", instance, instance1);</span><br><span> return -1;</span><br><span> }</span><br><span>@@ -7279,7 +7287,8 @@</span><br><span> rtp->cycles = 0;</span><br><span> prev_seqno = 0;</span><br><span> rtp->last_seqno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->last_end_timestamp = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->last_end_timestamp.ts = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->last_end_timestamp.is_set = 0;</span><br><span> if (rtp->rtcp) {</span><br><span> rtp->rtcp->expected_prior = 0;</span><br><span> rtp->rtcp->received_prior = 0;</span><br><span>@@ -8518,7 +8527,8 @@</span><br><span> </span><br><span> /* Need to reset the DTMF last sequence number and the timestamp of the last END packet */</span><br><span> rtp->last_seqno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->last_end_timestamp = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->last_end_timestamp.ts = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->last_end_timestamp.is_set = 0;</span><br><span> </span><br><span> if (strictrtp && rtp->strict_rtp_state != STRICT_RTP_OPEN</span><br><span> && !ast_sockaddr_isnull(addr) && ast_sockaddr_cmp(addr, &rtp->strict_rtp_address)) {</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15842">change 15842</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/+/15842"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 18 </div>
<div style="display:none"> Gerrit-Change-Id: Ie70d657d580d9a1f2877e25a6ef161c5ad761cf7 </div>
<div style="display:none"> Gerrit-Change-Number: 15842 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean@seanbright.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-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>