<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>