Review Board 1.7.16


While handling RFC 2833 DTMF, accommodate endpoints that increment the RTP timestamp in End of Event re-transmits

Review Request #2124 - Created Sept. 19, 2012 and submitted

Matt Jordan
1.8
ASTERISK-20424
Reviewers
asterisk-dev
Asterisk
See the long e-mail chain on the -users list for a description of the problem:

http://lists.digium.com/pipermail/asterisk-users/2012-September/274646.html

While endpoints should not be changing the source timestamp between DTMF event packets, the fact is there exists - somewhere - some endpoint that does.  If there's one, there's probably more, and I don't like breaking compatability with such devices in release branches.

To get around this, we absorb timestamps within the expected re-transmit period.  Note that this period would only affect End of Event packets, so it shouldn't prevent the detection of new DTMF digits that happen to arrive right on top of each other.
Ran it through the Test Suite's RFC 2833 DTMF tests.  Passed.

Basic mashing of DTMF keys locally.

Diff revision 1 (Latest)

  1. /branches/1.8/res/res_rtp_asterisk.c: Loading...
/branches/1.8/res/res_rtp_asterisk.c
Revision 373060 New Change
[20] 1584 lines
[+20] [+] static void process_dtmf_rfc2833(struct ast_rtp_instance *instance, unsigned char *data, int len, unsigned int seqno, unsigned int timestamp, struct ast_sockaddr *addr, int payloadtype, int mark, struct frame_list *frames)
1585
			new_duration += 0xFFFF + 1;
1585
			new_duration += 0xFFFF + 1;
1586
		}
1586
		}
1587
		new_duration = (new_duration & ~0xFFFF) | samples;
1587
		new_duration = (new_duration & ~0xFFFF) | samples;
1588

    
   
1588

   
1589
		if (event_end & 0x80) {
1589
		if (event_end & 0x80) {
1590
			/* End event */
1590
			/* End event.  Absorb re-transmits, and account for some endpoints
1591
			if ((rtp->last_seqno != seqno) && (timestamp > rtp->last_end_timestamp)) {
1591
			 * that erroneously increment the timestamp during re-transmissions */

    
   
1592
			if ((seqno != rtp->last_seqno) && (timestamp > rtp->last_end_timestamp + 320)) {
1592
				rtp->last_end_timestamp = timestamp;
1593
				rtp->last_end_timestamp = timestamp;
1593
				rtp->dtmf_duration = new_duration;
1594
				rtp->dtmf_duration = new_duration;
1594
				rtp->resp = resp;
1595
				rtp->resp = resp;
1595
				f = ast_frdup(create_dtmf_frame(instance, AST_FRAME_DTMF_END, 0));
1596
				f = ast_frdup(create_dtmf_frame(instance, AST_FRAME_DTMF_END, 0));
1596
				f->len = ast_tvdiff_ms(ast_samp2tv(rtp->dtmf_duration, rtp_get_rate(f->subclass.codec)), ast_tv(0, 0));
1597
				f->len = ast_tvdiff_ms(ast_samp2tv(rtp->dtmf_duration, rtp_get_rate(f->subclass.codec)), ast_tv(0, 0));
1597
				rtp->resp = 0;
1598
				rtp->resp = 0;
1598
				rtp->dtmf_duration = rtp->dtmf_timeout = 0;
1599
				rtp->dtmf_duration = rtp->dtmf_timeout = 0;
1599
				AST_LIST_INSERT_TAIL(frames, f, frame_list);
1600
				AST_LIST_INSERT_TAIL(frames, f, frame_list);
1600
			} else if (rtpdebug) {
1601
			} else if (rtpdebug) {
1601
				ast_debug(1, "Dropping duplicate or out of order DTMF END frame (seqno: %d, ts %d, digit %c)\n",
1602
				ast_debug(1, "Dropping re-transmitted, duplicate, or out of order DTMF END frame (seqno: %d, ts %d, digit %c)\n",
1602
					seqno, timestamp, resp);
1603
					seqno, timestamp, resp);
1603
			}
1604
			}
1604
		} else {
1605
		} else {
1605
			/* Begin/continuation */
1606
			/* Begin/continuation */
1606

    
   
1607

   
[+20] [20] 1468 lines
  1. /branches/1.8/res/res_rtp_asterisk.c: Loading...

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.