[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-09-2008 05:32 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-09-08 05:32  
---------------------------------------------------------------------- 
There are cases where the P2P bridge can be invoked and needs to be shut
down.

the P2P bridge is started so long as there is at least one codec in common
between the two end points

Example

A: (GSM, G7.11)  calls B: (G711 only)

Asterisk will see that both can do G711 and start the bridge
if A sends GSM (which was still allowed in the 200 OK), it will be
forwarded without transcoding and be ignored by B (one way audio)


The line of code will see that B can't do GSM so it will set up the
transcoding path

I agree that dynamic payloads were not taken into account in the cited
logic.  I'm attaching a patch that should take the dynamic payloads into
account.  Could you test to see if this resolves your issue? 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
07-09-08 05:32  tsearle        Note Added: 0089934                          
======================================================================