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

[Walter Doekes (JIRA)]

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

Walter Doekes commented on ASTERISK-28686:
------------------------------------------

{quote}
The RFC provides clear guidance about modifying an existing session.
{quote}
Yes. But here there are different sessions going on.
{quote}
RFC3261: 15. Terminating a Session
   \[...\] The state of the session and the state of the
   dialog are very closely related.  When a session is initiated with an
   INVITE, each 1xx or 2xx response from a distinct UAS creates a
   dialog, and if that response completes the offer/answer exchange, it
   also creates a session.  As a result, each session is "associated"
   with a single dialog - the one which resulted in its creation.
{quote}
So, when I get a 183 with a Tariff Notice, chan_sip should create a dialog (+ media session). And then, when I get a 200 (from a different entity), it should create a new dialog (+ media session).

But, since chan_sip only copes with a single dialog and a single audio stream for a call, I'm perfectly fine with it overwriting the old (now obsolete) dialog (+media session).

*In all cases, discarding the SDP from the first 200 makes no sense.*

Now, if we look at this:
{quote}
RFC3261: 13.1 Overview
   A 2xx response to an INVITE establishes a session, and it also
   creates a dialog between the UA that issued the INVITE and the UA
   that generated the 2xx response.  Therefore, when multiple 2xx
   responses are received from different remote UAs (because the INVITE
   forked), each 2xx establishes a different dialog.  All these dialogs
   are part of the same call.
{quote}
Don't you read that as:
- multiple dialogs can be established;
- thus, multiple (media) sessions can be established;
- thus, we don't want to discard any SDP from different origins.

Because chan_sip can only handle a single stream, do you want it to only use the oldest one? Or use the latest and greatest one? (Which, in the real world, holds the actual audio source.)

I've tried to come up with a real-world problem that ignoring the updated SDP would fix, but I cannot. But maybe I'm overlooking something here...?

> 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)