[asterisk-bugs] [Asterisk 0012955]: suspected typo in main/rtp.c bridge_p2p_rtp_write() payload type check (can cause RFC2833 DTMF detection issues)

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=12955 
====================================================================== 
Reported By:                tonyredstone
Assigned To:                
====================================================================== 
Project:                    Asterisk
Issue ID:                   12955
Category:                   Core/RTP
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     new
Asterisk Version:           1.4.21 
SVN Branch (only for SVN checkouts, not tarball releases): N/A 
SVN Revision (number only!):  
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             06-30-2008 10:44 CDT
Last Modified:              07-04-2008 07:20 CDT
====================================================================== 
Summary:                    suspected typo in main/rtp.c bridge_p2p_rtp_write()
payload type check (can cause RFC2833 DTMF detection issues)
Description: 
Hi,

Near the top of bridge_p2p_rtp_write(), there is code that reads as
follows:
      /* If the payload coming in is not one of the negotiated ones
then send it to the core, this will cause formats to change and the
bridge to break */
       if (!bridged->current_RTP_PT[payload].code)
               return -1;

The value of payload is from the inbound leg, however, (struct ast_rtp *)
bridged is the RTP struct for the *outbound* leg.  I believe this code
should read:
       /* If the payload coming in is not one of the negotiated ones
then send it to the core, this will cause formats to change and the
bridge to break */
       if (!rtp->current_RTP_PT[payload].code)
               return -1;

since (struct ast_rtp *)rtp is the structure for the inbound leg so now
the code matches the comment (and it makes sense).

With the code as it currently stands, packet2packet briding sometimes
punts packets to the core unnecessarily and can cause DTMF detection
breakage under certain conditions (see below).

Regards,
-Tony.

patch:
--- main/rtp.c.orig	2008-05-14 22:32:00.000000000 +0100
+++ main/rtp.c	2008-06-20 15:48:44.000000000 +0100
@@ -1063,7 +1063,7 @@
 	rtpPT = ast_rtp_lookup_pt(rtp, payload);

 	/* If the payload coming in is not one of the negotiated ones then
send it to the core, this will cause formats to change and the bridge
to break */
-	if (!bridged->current_RTP_PT[payload].code)
+	if (!rtp->current_RTP_PT[payload].code)
 		return -1;

 	/* If the payload is DTMF, and we are listening for DTMF - then feed
it into the core */
====================================================================== 

---------------------------------------------------------------------- 
 tsearle - 07-04-08 07:20  
---------------------------------------------------------------------- 
Some clarification about the code

It isn't a typo.  The method sends RTP packets to "bridged" thus it is
checking if "bridged" is actually supports the codec that is trying to be
sent.  If it does not, then clearly transcoding needs to occur.  (In your
case, the payload type needs to be changed from 97 to 101 before
forwarding)

Your change prevents this logic from occurring, which can lead to one way
audio situations. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
07-04-08 07:20  tsearle        Note Added: 0089737                          
======================================================================