[Home]

Summary:ASTERISK-06944: Asterisk do not validate SDP Session Name "Inacive"
Reporter:Joran Vinzens (vinzens)Labels:
Date Opened:2006-05-10 10:52:04Date Closed:2006-05-18 14:17:29
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/CodecHandling
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_sip.c
Description:I'm tester for Siemens SIP phones and we were testing also on Asterisk. We found a point were our Phones are not working wright with the asterisk. The Point is MOH. Our phones are sending in the SPD message an Incactice. The asterisk does not know this in this area.

We made some changes:

before:
in Line 3552 chan_sip.c

sdpLineNum_iterator_init(&iterator);

               while ((a = get_sdp_iterate(&iterator, req, "a"))[0] != '\0') {

                               char* mimeSubtype = ast_strdupa(a); /* ensures we have enough space */

                               if (!strcasecmp(a, "sendonly")) {

                                               sendonly=1;

                                               continue;

                               }

                               if (!strcasecmp(a, "sendrecv")) {

                                               sendonly=0;

                               }

                               if (sscanf(a, "rtpmap: %u %[^/]/", &codec, mimeSubtype) != 2) continue;

                               if (debug)

                                               ast_verbose("Found description format %s\n", mimeSubtype);

                               /* Note: should really look at the 'freq' and '#chans' params too */

                               ast_rtp_set_rtpmap_type(p->rtp, codec, "audio", mimeSubtype);

                               if (p->vrtp)

                                               ast_rtp_set_rtpmap_type(p->vrtp, codec, "video", mimeSubtype);

               }





After changes:



sdpLineNum_iterator_init(&iterator);

               while ((a = get_sdp_iterate(&iterator, req, "a"))[0] != '\0') {

                               char* mimeSubtype = ast_strdupa(a); /* ensures we have enough space */

                               if (!strcasecmp(a, "sendonly")) {

                                               sendonly=1;

                                               continue;

                               }

                               /* condition added by VGarroy to test MOH */

                               if (!strcasecmp(a, "inactive")) {

                                               sendonly=1;

                                               continue;

                               }              

                               if (!strcasecmp(a, "sendrecv")) {

                                               sendonly=0;

                               }

                               if (sscanf(a, "rtpmap: %u %[^/]/", &codec, mimeSubtype) != 2) continue;

                               if (debug)

                                               ast_verbose("Found description format %s\n", mimeSubtype);

                               /* Note: should really look at the 'freq' and '#chans' params too */

                               ast_rtp_set_rtpmap_type(p->rtp, codec, "audio", mimeSubtype);

                               if (p->vrtp)

                                               ast_rtp_set_rtpmap_type(p->vrtp, codec, "video", mimeSubtype);

               }



With this the asterisk starts sending MOH to the phone.

maybe you can add this to the souce in next binds

Comments:By: Serge Vecher (serge-v) 2006-05-10 11:06:25

vinzens:
1.can you please upload your changes as a patch file?
2.get a disclaimer on file?
3.see bug-guidelines http://www.digium.com/bugguidelines.html ?

thank you for contributing.

By: Olle Johansson (oej) 2006-05-10 11:11:37

I am actually working on this right now, so it's coming soon to trunk in another patch.

By: Joran Vinzens (vinzens) 2006-05-11 02:22:40

Changed file is uploaded.

By: Serge Vecher (serge-v) 2006-05-11 08:21:40

Please upload the changes as a patch file http://www.asterisk.org/developers/Patch_Howto as requested.

what about the disclaimer?

By: Joran Vinzens (vinzens) 2006-05-11 09:08:30

I'm sorry but i cannot make the patch file. I tryed like in HowTo, but without good result. I' not a linux user i only use it for testing the asterisk.
I hope this could be implemented anyway.

Disclaimer:
We from Siemens COM ESY do not claim any rights for this chanes in the source we did to enable MOH on Session Name "INACTIVE". Everything we did could be implemented by digium and could be taken for with costs and free software.

By: Olle Johansson (oej) 2006-05-16 15:36:14

Well, if you're phone sends INACTIVE, we're not allowed to send anything, right. We should propably answer INACTIVE too then, but with your patch we do not do that. Which means that the patch is not something we are going to apply to solve this issue and the disclaimer is not needed.

Can you tell me a bit more on why the phone sends a=inactive and not a=sendonly for hold state?

By: Joran Vinzens (vinzens) 2006-05-17 03:36:52

I don't know why we use the inacive. Our coder for the Siemens Phones say, this is a part of the RFC. There are 3 entrys for the session name. (inacive, sendrecv and sendonly) we had a lot of problems with this inactive, but they will not change it, so we still have the problem.

I think the inactive state is to signale the phone playing his own MOH.

it's a pity that the asterisk will not implement the changes for our Phones.

By: Olle Johansson (oej) 2006-05-17 03:56:53

I have not said we are not going to fix it, just that your patch is not correct. I was kind of curious on why this was used. Will look into this in a while. If you can, please look into it too. I just changed chan_sip to answer to sendonly, so we should be able to fix inactive too and do the right thing.

By: Joran Vinzens (vinzens) 2006-05-17 04:09:40

If you cange the chan_sip in this way that when our phones sends INACTIVE your server answers INACTIVE too, not all phones will understand this, also a trunk. If a snom gets INACTIVE it will do nothing. We had a big discussion for this at our server side (hipath 8000).

Your right to say that out changes are not allowed but this is an very easy way.

I will have a look into changes in Chan_sip

By: Joran Vinzens (vinzens) 2006-05-17 05:38:45

I had a look into Broadsoft and Sylantro. This servers work at the same way. The handle INACTIVE as Sendonly.

By: Kevin P. Fleming (kpfleming) 2006-05-18 14:17:28

I have put the 'simple fix' into branch 1.2 in revision 28335; the proper fix (involving us replying back also 'inactive') will have to wait until the 1.4 release goes out.