Summary: | ASTERISK-15573: [patch] T.38 negotiation fails with Patton SN2400 | ||
Reporter: | Raivis Rengelis (raivisr) | Labels: | |
Date Opened: | 2010-02-03 16:00:00.000-0600 | Date Closed: | 2010-02-09 11:49:54.000-0600 |
Priority: | Major | Regression? | No |
Status: | Closed/Complete | Components: | Channels/chan_sip/T.38 |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ( 0) faulty_sdp_debug.txt ( 1) t38-sdp-newline-fix1.diff ( 2) t38-sdp-newline-fix2.diff | |
Description: | Patton SN2400 refuses to negotiate T.38 session because of extra newline in SDP. [Feb 3 22:31:46] DEBUG[31017] chan_sip.c: Body 5 [ 22]: m=image 4326 udptl t38 [Feb 3 22:31:46] DEBUG[31017] chan_sip.c: Body 6 [ 0]: [Feb 3 22:31:46] DEBUG[31017] chan_sip.c: Body 7 [ 17]: a=T38FaxVersion:0 proposed fix: --- asterisk.orig/channels/chan_sip.c 2010-02-03 23:52:03.000000000 +0200 +++ asterisk/channels/chan_sip.c 2010-02-03 23:58:41.000000000 +0200 @@ -9191,8 +9191,6 @@ static enum sip_result add_sdp(struct si ast_str_append(&m_video, 0, "\r\n"); if (needtext) ast_str_append(&m_text, 0, "\r\n"); - if (add_t38) - ast_str_append(&m_modem, 0, "\r\n"); len = strlen(version) + strlen(subject) + strlen(owner) + strlen(connection) + strlen(session_time); | ||
Comments: | By: Leif Madsen (lmadsen) 2010-02-08 10:29:49.000-0600 Information about where and why the change was added: lmadsen@asterisk-releaser:~/WORKING/asterisk/trunk/channels$ svn annotate -r100000:HEAD chan_sip.c | grep "m_modem" | more 184948 file struct ast_str *m_modem = ast_str_alloca(256); /* Media declaration line for modem */ 243860 russell ast_str_append(&m_modem, 0, "m=image %d udptl t38\r\n", ntohs(udptldest.sin_port)); 225089 file ast_str_append(&m_modem, 0, "c=IN IP4 %s\r\n", ast_inet_ntoa(udptldest.sin_addr)); 184948 file ast_str_append(&m_modem, 0, "\r\n"); 184948 file len += m_modem->used + a_modem->used; 184948 file add_line(resp, m_modem->str); lmadsen@asterisk-releaser:~/WORKING/asterisk/trunk/channels$ svn log -r184948 http://svn.asterisk.org/svn/asterisk/trunk ------------------------------------------------------------------------ r184948 | file | 2009-03-30 09:37:47 -0500 (Mon, 30 Mar 2009) | 21 lines Merged revisions 184947 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r184947 | file | 2009-03-30 11:35:47 -0300 (Mon, 30 Mar 2009) | 14 lines Improve our handling of T38 in the initial INVITE from a device. We now answer with matching media streams to what is requested. If an INVITE is received with both a T38 and RTP media stream this means we answer with both. For any outgoing calls created as a result of this inbound one no T38 is requested in the initial INVITE. Instead if we start receiving udptl packets we trigger a reinvite on the outbound side. (closes issue ASTERISK-11843) Reported by: marsosa Tested by: pinga-fogo, okrief, file, afu Review: http://reviewboard.digium.com/r/208/ ........ ------------------------------------------------------------------------ By: Leif Madsen (lmadsen) 2010-02-08 10:31:20.000-0600 The issue related here is what caused this regression. By: Leif Madsen (lmadsen) 2010-02-08 10:35:19.000-0600 After further discussion, the issue I reference is not likely to be the actual cause of this issue, as it would seem improbable to only have this issue reported nearly a year after it was originally committed. At this point will leave it to a capable developer to do some additional debugging and searching. By: Raivis Rengelis (raivisr) 2010-02-08 11:50:29.000-0600 looking at the source it seems that somebody just ran across piece of code: if (needaudio) ast_str_append(&m_audio, 0, "\r\n"); if (needvideo) ast_str_append(&m_video, 0, "\r\n"); if (needtext) ast_str_append(&m_text, 0, "\r\n"); and went on adding the same for if (add_t38) ignoring the fact that m_modem is built a little different and has trailing \r\n added already. that's it :) By: Matthew Nicholson (mnicholson) 2010-02-08 15:07:31.000-0600 Do you have an example of the faulty SDP packet? By: Matthew Nicholson (mnicholson) 2010-02-08 15:18:41.000-0600 I think I have found the problem, test with the patch I uploaded (it is identical to your suggested patch). By: Matthew Nicholson (mnicholson) 2010-02-08 15:22:54.000-0600 I have also uploaded an alternative patch that should fix the bug. Both patches should work equally well. By: Raivis Rengelis (raivisr) 2010-02-08 15:23:03.000-0600 i already tested before reporting the bug :) and yes, it fixes it. (just uploaded full sip/sdp debug log that was failing) By: Raivis Rengelis (raivisr) 2010-02-08 15:26:09.000-0600 yes, the second one should work as well. I just did not dwell into why original author went one way for audio/video/text and different way for t38. anyways both of those patches fix the problem with pattons and possibly other gateways. By: Raivis Rengelis (raivisr) 2010-02-08 15:36:17.000-0600 btw, redundant \r\n was introduced in revision 243860, if that matters. By: Digium Subversion (svnbot) 2010-02-09 11:47:51.000-0600 Repository: asterisk Revision: 245727 U trunk/channels/chan_sip.c ------------------------------------------------------------------------ r245727 | mnicholson | 2010-02-09 11:40:04 -0600 (Tue, 09 Feb 2010) | 8 lines This commit removes an extra newline in T.38 generated SDP packets. This bug was caused by the fix introduced in r243860. (closes issue ASTERISK-15573) Reported by: raivisr Patches: t38-sdp-newline-fix1.diff uploaded by mnicholson (license 96) Tested by: raivisr ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=245727 By: Digium Subversion (svnbot) 2010-02-09 11:49:54.000-0600 Repository: asterisk Revision: 245728 _U branches/1.6.2/ U branches/1.6.2/channels/chan_sip.c ------------------------------------------------------------------------ r245728 | mnicholson | 2010-02-09 11:43:41 -0600 (Tue, 09 Feb 2010) | 15 lines Merged revisions 245727 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r245727 | mnicholson | 2010-02-09 11:40:04 -0600 (Tue, 09 Feb 2010) | 2 lines This commit removes an extra newline in T.38 generated SDP packets. This bug was caused by the fix introduced in r243860. (closes issue ASTERISK-15573) Reported by: raivisr Patches: t38-sdp-newline-fix1.diff uploaded by mnicholson (license 96) Tested by: raivisr ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=245728 |