[Home]

Summary:ASTERISK-12526: [patch] Set(SIP_CODEC=xxxx) only applies to first inbound leg of call
Reporter:Sam Deller (samdell3)Labels:
Date Opened:2008-08-05 18:32:33Date Closed:2009-04-06 11:19:59
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/CodecHandling
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 13243.diff
( 1) 20080827__bug13243.diff.txt
( 2) ast1.6.0.5-chan_sip_SIP_CODEC.patch
( 3) Asterisk-1-4-14-SIP_CODEC-fix.patch
Description:We have had a long standing requirement to be able to force the use of g711 codec based on dialled number, eg known modem destinations etc.
We still need to use g729 by default for voice calls.

The obvious choice is to Set(SIP_CODEC=alaw) prior to Dial()

However, SIP_CODEC only ever forced the inbound (first) leg of the call to use alaw. If the outbound leg codec priority was 1st G729 2nd alaw, then g729 was always used.

Attached is a very simple patch against 1.4.14 that solves our problem. It works for both reinvited and non reinvited media.
Due to the patch only being 2 lines of additional code, it would be easy to apply to later versions of Asterisk

It's now running in a production environment, but I would really like some feedback from other users.



****** ADDITIONAL INFORMATION ******

Before patch, and canreinvite = yes, call fails with 488 due to no codec being presented in the reinvite.
Actaully, a bug in the chan_sip.c logic brings up a T.38 error as a direct result of the 488

   -- Executing [0800000001@airnetvpbx:1] Set("SIP/66500531-0826e420", "SIP_CODEC=alaw") in new stack
   -- Executing [0800000001@airnetvpbx:2] Dial("SIP/66500531-0826e420", "SIP/55530800000000#@imgnap1|60|L(14400000)") in new stack
   -- Setting call duration limit to 14400 seconds.
   -- Called 55530800000000#@imgnap1
[Aug  6 11:13:50] NOTICE[31170]: chan_sip.c:12414 handle_response_peerpoke: Peer '66500531' is now Reachable. (22ms / 2000ms)
   -- SIP/imgnap1-08284e98 is making progress passing it to SIP/66500531-0826e420
[Aug  6 11:13:50] NOTICE[31186]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
   -- SIP/imgnap1-08284e98 is ringing
   -- SIP/imgnap1-08284e98 answered SIP/66500531-0826e420
[Aug  6 11:13:51] NOTICE[31186]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
[Aug  6 11:13:51] NOTICE[31186]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
   -- Native bridging SIP/66500531-0826e420 and SIP/imgnap1-08284e98
[Aug  6 11:13:51] ERROR[31170]: chan_sip.c:12148 handle_response_invite: Got error on T.38 re-invite. Bad configuration. Peer needs to have T.38 disabled.
 == Spawn extension (airnetvpbx, 0800000001, 2) exited non-zero on 'SIP/66500531-0826e420'
   -- Executing [h@airnetvpbx:1] Hangup("SIP/66500531-0826e420", "") in new stack
 == Spawn extension (airnetvpbx, h, 1) exited non-zero on 'SIP/66500531-0826e420'
voip-rot1*CLI>



Before patch, and canreinvite = no, call completes but only first leg is alaw. outbound (second leg) is g729

   -- Executing [0800000001@airnetvpbx:1] Set("SIP/66500531-0827f998", "SIP_CODEC=alaw") in new stack
   -- Executing [0800000001@airnetvpbx:2] Dial("SIP/66500531-0827f998", "SIP/55530800000000#@imgnap1|60|L(14400000)") in new stack
   -- Setting call duration limit to 14400 seconds.
   -- Called 55530800000000#@imgnap1
[Aug  6 11:16:37] NOTICE[31170]: chan_sip.c:12414 handle_response_peerpoke: Peer '66500531' is now Reachable. (30ms / 2000ms)
   -- SIP/imgnap1-082f5b20 is making progress passing it to SIP/66500531-0827f998
[Aug  6 11:16:37] NOTICE[31220]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
   -- SIP/imgnap1-082f5b20 is ringing
   -- SIP/imgnap1-082f5b20 answered SIP/66500531-0827f998
[Aug  6 11:16:38] NOTICE[31220]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
[Aug  6 11:16:38] NOTICE[31220]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
voip-rot1*CLI> sip show channels
Peer             User/ANR    Call ID      Seq (Tx/Rx)  Form  Hold     Last Message
x.x.x.83   5553080000  0609d194602  00102/00000  g729  No       Tx: ACK
10.33.12.106     66500531    31c5b2ed-ba  00101/00102  alaw  No       Rx: ACK
2 active SIP channels


After patch, canreinvite yes or no both work fine. Both legs of call use correct codec

   -- Executing [0800000001@airnetvpbx:1] Set("SIP/66500531-08281d18", "SIP_CODEC=alaw") in new stack
   -- Executing [0800000001@airnetvpbx:2] Dial("SIP/66500531-08281d18", "SIP/55530800000000#@imgnap1|60|L(14400000)") in new stack
   -- Setting call duration limit to 14400 seconds.
[Aug  6 11:12:25] NOTICE[30686]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
   -- Called 55530800000000#@imgnap1
[Aug  6 11:12:25] NOTICE[30097]: chan_sip.c:12416 handle_response_peerpoke: Peer '66500531' is now Reachable. (24ms / 2000ms)
   -- SIP/imgnap1-08277680 is making progress passing it to SIP/66500531-08281d18
[Aug  6 11:12:25] NOTICE[30686]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
   -- SIP/imgnap1-08277680 is ringing
   -- SIP/imgnap1-08277680 answered SIP/66500531-08281d18
[Aug  6 11:12:26] NOTICE[30686]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
[Aug  6 11:12:26] NOTICE[30686]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
   -- Native bridging SIP/66500531-08281d18 and SIP/imgnap1-08277680
[Aug  6 11:12:26] NOTICE[30686]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
[Aug  6 11:12:26] NOTICE[30097]: chan_sip.c:3587 try_suggested_sip_codec: Changing codec to 'alaw' for this call because of ${SIP_CODEC} variable
voip-rot1*CLI> sip show channels
Peer             User/ANR    Call ID      Seq (Tx/Rx)  Form  Hold     Last Message
x.x.x.83   5553080000  2069c2db21a  00103/00000  alaw  No       Tx: ACK
10.33.12.106     66500531    d1ea294d-70  00102/00102  alaw  No       Tx: ACK
2 active SIP channels
voip-rot1*CLI>
Comments:By: Sam Deller (samdell3) 2008-08-05 18:54:34

Removed for clarity above, however SIP_CODEC MUST be used with a proceeding _ AND the patch to properly work.

eg

Set(_SIP_CODEC=alaw)

By: Ross Beer (rossbeer) 2008-08-26 16:08:11

I too would like to dynamically set the SIP_CODEC for each call. For example when calling an external extension <num>@<domain> will this patch do this and will the patch also be in the next versions of Asterisk?

By: Leif Madsen (lmadsen) 2008-08-27 10:34:42

There is a patch attached for this issue, and the license is OK. Can a developer please review and determine if this should go into Asterisk? Thanks!

By: Tilghman Lesher (tilghman) 2008-08-27 14:59:30

Patch updated to current SVN.  Looks okay to me.

By: Sam Deller (samdell3) 2008-08-27 15:47:46

Rossbeer: SIP_CODEC was already a function of asterisk... but in my opinion it never actually worked correctly.

Croydon76: This patch has been in production for several weeks on our system, doing approx 50k minutes per day. 1k of those daily minutes depend on _SIP_CODEC being set prior to Dial().
Therefore I can pretty much confirm that there are no problems. In fact it now works like a charm...

By: Tilghman Lesher (tilghman) 2008-08-27 17:06:30

I'm still a little uncomfortable about committing changes like this to the SIP channel driver, so I think I'd want someone else to look at this before I commit it.

By: Sam Deller (samdell3) 2008-08-27 17:20:04

try_suggested_sip_codec() basically does nothing if SIP_CODEC variable is not set in the dialplan.

EG
if(!codec)
   return;

The patch adds try_suggested_sip_codec() in 2 places.

Therefore, the theory is that this cant break anything if SIP_CODEC is not set in the dialplan.

Given that SIP_CODEC never worked properly to begin with, then surely this patch cannot cause further consequences....

By: Sam Deller (samdell3) 2009-01-21 18:50:39.000-0600

Just noticed some activity on this so here's an update. Has been running for many months in production now with no problems whatsoever...

By: Leif Madsen (lmadsen) 2009-01-22 14:54:01.000-0600

Assigned to file for review! If you really need me to test, let me know, and I'll get to it tomorrow.

By: Joshua C. Colp (jcolp) 2009-02-10 08:36:35.000-0600

This looks fine to me. It shouldn't introduce any regressions since it'll only come into play if you actually use the SIP_CODEC variable and while it may be possible to shoot yourself in the foot with it... you can do that with a lot of other things. I'd propose this only go into trunk though since it would change behavior in previous versions.

By: Joshua C. Colp (jcolp) 2009-02-10 08:37:46.000-0600

Actually to further that thought perhaps we should have a second variable for outbound calls that control it, instead of using the same variable. I'm just wondering if things could go sour if suddenly that variable also controlled outbound calls and caused things to break for those who upgrade.

By: Olle Johansson (oej) 2009-02-10 09:40:47.000-0600

This is a documented feature since a very long time. Wonder how it ended up not working... Well.

Here's the doc for 1.4:
${SIP_CODEC}            Set the SIP codec for a call

Doesn't say inbound or outbound. I always assumed outbound only. Could we use the CHANNEL function for this somehow?

By: Joshua C. Colp (jcolp) 2009-02-10 09:43:59.000-0600

Yes, it is rather vague. As for using the CHANNEL function it depends. We can certainly do that for inbound, but the dialplan variable already works with that. For outbound the channel is created in app_dial and we don't have any control in the dialplan of that channel via the CHANNEL function.

By: Olle Johansson (oej) 2009-02-10 09:48:18.000-0600

So _SIP_CODEC is for outbound and CHANNEL() may be used for inbound. Which means that we can fix SIP_CODEC for outbound only and clarify docs.

For inbound it's harder. You need a way to escape. If I have an inbound call that only sends ALAW and set SIP_CODEC to G.729, then the SIP channel will have to deny the call. We can't really set error codes in reply to variables. people will have to first read CHANNEL() to see inbound codecs and select one.
The important is that the SIP channel will fail the call in this case, not ignore the SIP_CODEC setting.
Which will confuse people.

By: Olivier Krief (okrief) 2009-02-12 12:58:03.000-0600

Could you elaborate a bit what is planned :
- will sambdell3's patch make it way into trunk ?
- or will current situation left unchanged beside doc clarification ?

By: Olle Johansson (oej) 2009-02-12 13:19:13.000-0600

Oh, we need to get it working. The question is what to do about inbound calls.

By: Joshua C. Colp (jcolp) 2009-02-12 13:28:38.000-0600

Right, while this patch may not be the solution we are discussing where we should go and what we should do to fix this in the proper manner. The issue I have with changing SIP_CODEC to be outbound is compatibility with all the dialplans that use it already. If we change the behavior and they don't change it... things will break.

By: Olivier Krief (okrief) 2009-02-13 01:02:01.000-0600

What about defining SIP_CODEC_INBOUND, SIP_CODEC_OUTBOUND ?

By: Joshua C. Colp (jcolp) 2009-02-13 13:54:49.000-0600

That would allow us to be backwards compatible with SIP_CODEC pretty easily, yeah.

By: Joshua C. Colp (jcolp) 2009-03-30 08:17:20

I've uploaded a patch which changes things based on the end result of this conversation. It will be backwards compatible with current usage, but also introduce some finger grain control using the SIP_CODEC_OUTBOUND and SIP_CODEC_INBOUND dialplan variables.

By: Digium Subversion (svnbot) 2009-04-06 11:15:31

Repository: asterisk
Revision: 186624

U   trunk/CHANGES
U   trunk/channels/chan_sip.c
U   trunk/doc/tex/channelvariables.tex

------------------------------------------------------------------------
r186624 | file | 2009-04-06 11:15:31 -0500 (Mon, 06 Apr 2009) | 13 lines

Add support for changing the outbound codec on a SIP call using
a dialplan variable.

This adds a dialplan variable (SIP_CODEC_OUTBOUND) which controls
the codec offered for an outgoing SIP call. This is much like the
SIP_CODEC dialplan variable and has the same restrictions. The codec
set must be one that is configured for the call.

(closes issue ASTERISK-12526)
Reported by: samdell3
Patches:
     13243.diff uploaded by file (license 11)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=186624

By: Digium Subversion (svnbot) 2009-04-06 11:19:57

Repository: asterisk
Revision: 186635

_U  branches/1.6.2/

------------------------------------------------------------------------
r186635 | file | 2009-04-06 11:19:57 -0500 (Mon, 06 Apr 2009) | 19 lines

Blocked revisions 186624 via svnmerge

........
 r186624 | file | 2009-04-06 13:15:30 -0300 (Mon, 06 Apr 2009) | 13 lines
 
 Add support for changing the outbound codec on a SIP call using
 a dialplan variable.
 
 This adds a dialplan variable (SIP_CODEC_OUTBOUND) which controls
 the codec offered for an outgoing SIP call. This is much like the
 SIP_CODEC dialplan variable and has the same restrictions. The codec
 set must be one that is configured for the call.
 
 (closes issue ASTERISK-12526)
 Reported by: samdell3
 Patches:
       13243.diff uploaded by file (license 11)
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=186635