[svn-commits] mjordan: branch 1.8 r370252 - /branches/1.8/res/res_rtp_asterisk.c
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Thu Jul 19 15:15:12 CDT 2012
Author: mjordan
Date: Thu Jul 19 15:15:04 2012
New Revision: 370252
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=370252
Log:
Handle extremely out of order RFC 2833 DTMF
The current implementation of RFC 2833 DTMF handling in res_rtp_asterisk will,
if a packet arrives out of order, drop the packet. This is to prevent
duplicate ton generation in the Asterisk core. Since the RTP layer does not
buffer data itself, this is the only option the RTP layer currently has for
handling packets that arrive out of order.
For the most part, this doesn't matter. For a particular digit, so long as a
BEGIN packet arrives before the first END packet, the digit will be produced.
If subsequent BEGIN packets arrive interleaved with the ENDs, they will be
dropped; likewise, if the BEGIN or END packets themselves are out of order,
those packets are dropped but sufficient information is conveyed to the
Asterisk core to produce the appropriate digit.
For certain sequences of DTMF packets - most notably when, for a particular
digit, an END packet arrives before any BEGIN packet for that digit - this
is a real problem. When an END arrives before any BEGINs, the END packet is
dropped - but at the same time, it causes subsequent BEGIN packets for that
digit to be ignored. When the next in order END packet arrives, it too is
dropped - Asterisk believes that there was no initial BEGIN.
The solution this patch provides is to trust the END packet to convey the
information needed for the Asterisk core to produce the DTMF digit. If we
receive an END packet, and it:
* Has a timestamp greater then the last timestamp received from an END
packet
* Does not have the same sequence number as the last received sequence
number (and is thus not an END packet retransmission)
Then we send the END frame up to the Asterisk core. It contains enough
DTMF information for Asterisk to produce the digit.
On the other hand, if we receive a BEGIN or continuation packet that occurs
with a timestamp equal to or less then the last END timestamp, then we've
received something out of order - but we already have received enough
information to produce the digit. These packets are dropped.
Much thanks goes to Olle Johansson (oej) for providing the idea for this
solution.
Review: https://reviewboard.asterisk.org/r/2033/
(issue ASTERISK-18404)
Reporter: Stephane Chazelas
Tested by: Matt Jordan
Modified:
branches/1.8/res/res_rtp_asterisk.c
Modified: branches/1.8/res/res_rtp_asterisk.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/res/res_rtp_asterisk.c?view=diff&rev=370252&r1=370251&r2=370252
==============================================================================
--- branches/1.8/res/res_rtp_asterisk.c (original)
+++ branches/1.8/res/res_rtp_asterisk.c Thu Jul 19 15:15:04 2012
@@ -147,12 +147,13 @@
int rtpkeepalive; /*!< Send RTP comfort noice packets for keepalive */
/* DTMF Reception Variables */
- char resp;
- unsigned int lastevent;
- unsigned int dtmf_duration; /*!< Total duration in samples since the digit start event */
- unsigned int dtmf_timeout; /*!< When this timestamp is reached we consider END frame lost and forcibly abort digit */
+ char resp; /*!< The current digit being processed */
+ unsigned int last_seqno; /*!< The last known sequence number for any DTMF packet */
+ unsigned int last_end_timestamp; /*!< The last known timestamp received from an END packet */
+ unsigned int dtmf_duration; /*!< Total duration in samples since the digit start event */
+ unsigned int dtmf_timeout; /*!< When this timestamp is reached we consider END frame lost and forcibly abort digit */
unsigned int dtmfsamples;
- enum ast_rtp_dtmf_mode dtmfmode;/*!< The current DTMF mode of the RTP stream */
+ enum ast_rtp_dtmf_mode dtmfmode; /*!< The current DTMF mode of the RTP stream */
/* DTMF Transmission Variables */
unsigned int lastdigitts;
char sending_digit; /*!< boolean - are we sending digits */
@@ -1494,8 +1495,10 @@
rtp->dtmfsamples = 0;
return &ast_null_frame;
}
- ast_debug(1, "Sending dtmf: %d (%c), at %s\n", rtp->resp, rtp->resp,
- ast_sockaddr_stringify(&remote_address));
+ ast_debug(1, "Creating %s DTMF Frame: %d (%c), at %s\n",
+ type == AST_FRAME_DTMF_END ? "END" : "BEGIN",
+ rtp->resp, rtp->resp,
+ ast_sockaddr_stringify(&remote_address));
if (rtp->resp == 'X') {
rtp->f.frametype = AST_FRAME_CONTROL;
rtp->f.subclass.integer = AST_CONTROL_FLASH;
@@ -1559,12 +1562,12 @@
}
if (ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_DTMF_COMPENSATE)) {
- if ((rtp->lastevent != timestamp) || (rtp->resp && rtp->resp != resp)) {
+ if ((rtp->last_end_timestamp != timestamp) || (rtp->resp && rtp->resp != resp)) {
rtp->resp = resp;
rtp->dtmf_timeout = 0;
f = ast_frdup(create_dtmf_frame(instance, AST_FRAME_DTMF_END, ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_DTMF_COMPENSATE)));
f->len = 0;
- rtp->lastevent = timestamp;
+ rtp->last_end_timestamp = timestamp;
AST_LIST_INSERT_TAIL(frames, f, frame_list);
}
} else {
@@ -1581,30 +1584,40 @@
}
new_duration = (new_duration & ~0xFFFF) | samples;
- /* The second portion of this check is to not mistakenly
- * stop accepting DTMF if the seqno rolls over beyond
- * 65535.
- */
- if (rtp->lastevent > seqno && rtp->lastevent - seqno < 50) {
- /* Out of order frame. Processing this can cause us to
- * improperly duplicate incoming DTMF, so just drop
- * this.
- */
- return;
- }
-
if (event_end & 0x80) {
/* End event */
- if ((rtp->lastevent != seqno) && rtp->resp) {
+ if ((rtp->last_seqno != seqno) && (timestamp > rtp->last_end_timestamp)) {
+ rtp->last_end_timestamp = timestamp;
rtp->dtmf_duration = new_duration;
+ rtp->resp = resp;
f = ast_frdup(create_dtmf_frame(instance, AST_FRAME_DTMF_END, 0));
f->len = ast_tvdiff_ms(ast_samp2tv(rtp->dtmf_duration, rtp_get_rate(f->subclass.codec)), ast_tv(0, 0));
rtp->resp = 0;
rtp->dtmf_duration = rtp->dtmf_timeout = 0;
AST_LIST_INSERT_TAIL(frames, f, frame_list);
+ } else if (rtpdebug) {
+ ast_debug(1, "Dropping duplicate or out of order DTMF END frame (seqno: %d, ts %d, digit %c)\n",
+ seqno, timestamp, resp);
}
} else {
/* Begin/continuation */
+
+ /* The second portion of the seqno check is to not mistakenly
+ * stop accepting DTMF if the seqno rolls over beyond
+ * 65535.
+ */
+ if ((rtp->last_seqno > seqno && rtp->last_seqno - seqno < 50)
+ || timestamp <= rtp->last_end_timestamp) {
+ /* Out of order frame. Processing this can cause us to
+ * improperly duplicate incoming DTMF, so just drop
+ * this.
+ */
+ if (rtpdebug) {
+ ast_debug(1, "Dropping out of order DTMF frame (seqno %d, ts %d, digit %c)\n",
+ seqno, timestamp, resp);
+ }
+ return;
+ }
if (rtp->resp && rtp->resp != resp) {
/* Another digit already began. End it */
@@ -1629,7 +1642,7 @@
rtp->dtmf_timeout = timestamp + rtp->dtmf_duration + dtmftimeout;
}
- rtp->lastevent = seqno;
+ rtp->last_seqno = seqno;
}
rtp->dtmfsamples = samples;
More information about the svn-commits
mailing list