[asterisk-bugs] [JIRA] (ASTERISK-28686) chan_sip strictrtp=yes fails when media source is changed: no audio

Tue Jan 14 08:20:25 CST 2020

    [ https://issues.asterisk.org/jira/browse/ASTERISK-28686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=249352#comment-249352 ] 

Michael L. Young edited comment on ASTERISK-28686 at 1/14/20 8:18 AM:
----------------------------------------------------------------------

Hey Walter.  Not feeling defeat about anything.  I didn't think I was in a battle here.  I thought we were all trying to work together here.  I always learn from you and was just trying to understand things.  I freely admit my errors and learn from them.

The intention was to add context to this issue and bring forward information to help decide what is best for the community.  As mentioned in those issues linked and you mention on Gerrit, it has been acceptable all these years to use ignoresdpversion=yes.

In your scenario, you are referring to having a 183 then getting the 200.  I think this is a new scenario, that, from what I can recall, I haven't personally seen tackled.  Yes, the 200 has the final say on the media source.

The proposed change does not take into account a 183 and then getting a 200, right?  It is just going to accept any SDP change, if I understand the patch correctly.  Maybe that is acceptable.  The stance has always been that you can't have a 200 and then later on something up and changes the SDP on you without some form of checking or without you expressly saying that is okay with you by ignoring the sdp versioning (although I know that doesn't seem right either because we are not checking the IP part).  I am just pushing back to make sure it is thought all the way through, which I am sure you already have done because I know you do not make willy nilly changes.

Thanks.  I am just trying to work together by supplying context and information.

Edit: Just saw comments about pjsip.  That is great to know.  Thanks for adding that information. 
 Just more reason to be using pjsip over chan_sip.  My comments have all been centered around chan_sip.

was (Author: elguero):
Hey Walter.  Not feeling defeat about anything.  I didn't think I was in a battle here.  I thought we were all trying to work together here.  I always learn from you and was just trying to understand things.  I freely admit my errors and learn from them.

The intention was to add context to this issue and bring forward information to help decide what is best for the community.  As mentioned in those issues linked and you mention on Gerrit, it has been acceptable all these years to use ignoresdpversion=yes.

In your scenario, you are referring to having a 183 then getting the 200.  I think this is a new scenario, that, from what I can recall, I haven't personally seen tackled.  Yes, the 200 has the final say on the media source.

The proposed change does not take into account a 183 and then getting a 200, right?  It is just going to accept any SDP change, if I understand the patch correctly.  Maybe that is acceptable.  The stance has always been that you can't have a 200 and then later on something up and changes the SDP on you without some form of checking or without you expressly saying that is okay with you by ignoring the sdp versioning (although I know that doesn't seem right either because we are not checking the IP part).  I am just pushing back to make sure it is thought all the way through, which I am sure you already have done because I know you do not make willy nilly changes.

Thanks.  I am just trying to work together by supplying context and information.

> chan_sip strictrtp=yes fails when media source is changed: no audio
> -------------------------------------------------------------------
>
>                 Key: ASTERISK-28686
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-28686
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Channels/chan_sip/Interoperability
>    Affects Versions: 13.30.0
>            Reporter: Walter Doekes
>            Assignee: Walter Doekes
>
> Hi!
> When the SDP origin changes, the SDP session version from the new media is not necessarily higher than the existing session version.
> chan_sip contains this bit:
> {code}
>         if (ast_test_flag(&p->flags[1], SIP_PAGE2_IGNORESDPVERSION) ||
>             (p->sessionversion_remote < 0) ||
>             (p->sessionversion_remote != rua_version)) {
>                 p->sessionversion_remote = rua_version;
>         } else {
> ...
>                         ast_debug(2, "Call %s responded to our reinvite without changing SDP version; ignoring SDP.\n", p->callid);
>                         return FALSE;
> {code}
> That means that the updated SDP will be ignored for a change like this:
> - Initial o= line (for example in 183):
> {noformat}
> o=root 1845921041 1845921041 IN IP4 193.x.x.215
> {noformat}
> - Updated o= line (for example in 200):
> {noformat}
> o=- 1126144851 1126144853 IN IP4 87.x.x.235
> {noformat}
> In this example, the new SDP sess_version 1126144851 is lower than 1845921041, causing the SDP to be ignored. And because the SDP is not processed, {{ast_rtp_instance_set_remote_address}} is never called, and the new RTP source is discarded.
> The fix: do not only check sess_version, but also check the "unique parts" from the SDP. And if they change, then also update the SDP.
> Basically this:
> {noformat}
>         if (ast_test_flag(&p->flags[1], SIP_PAGE2_IGNORESDPVERSION) ||
> -           (p->sessionversion_remote < 0) ||
> -           (p->sessionversion_remote < rua_version)) {
> -               p->sessionversion_remote = rua_version;
> +                       sess_version > p->sessionversion_remote ||
> +                       strcmp(unique, S_OR(p->sessionunique_remote, ""))) {
> +               p->sessionversion_remote = sess_version;
> +               ast_string_field_set(p, sessionunique_remote, unique);
>         } else {
> {noformat}
> Along with a marginally bigger patch (to be put on gerrit), we then get these:
> {noformat}
> [2020-01-13 10:31:20] VERBOSE[10720][C-00000001] chan_sip.c:
>   Got SDP version 203027438 and unique parts [- 203027438 IN IP4 10.x.x.161]
> [2020-01-13 10:31:20] VERBOSE[10720][C-00000001] chan_sip.c:
>   Got SDP version 1845921041 and unique parts [root 1845921041 IN IP4 193.x.x.215]
> [2020-01-13 10:31:31] VERBOSE[10720][C-00000001] chan_sip.c: 
>   Comparing SDP version 1845921041 -> 1126144853 and unique parts [root 1845921041 IN IP4 193.x.x.215] -> [- 1126144851 IN IP4 87.x.x.235]
> {noformat}
> And then everything works as intended.

--
This message was sent by Atlassian JIRA
(v6.2#6252)