[asterisk-bugs] [JIRA] (ASTERISK-28686) chan_sip strictrtp=yes fails when media source is changed: no audio
Walter Doekes (JIRA)
noreply at issues.asterisk.org
Tue Jan 14 06:30:25 CST 2020
[ https://issues.asterisk.org/jira/browse/ASTERISK-28686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=249350#comment-249350 ]
Walter Doekes commented on ASTERISK-28686:
------------------------------------------
{quote}
Not sure if chan_sip is worth that much effort on my part anymore since I don't really use it.
{quote}
I understand that meddling in the old driver is suboptimal. From a quick search through the tracker, it appears that PJSIP does not even have an option to allow media switching. (I do hope it doesn't behave as quirkily as chan_sip does though.)
However, we have to deal with the real world. And in particular, in my case where the 183 has a different media source than the actual 200, ignoring the 200 _sometimes_ is just wrong(tm).
{quote}
This just change just doesn't feel right [...]
{quote}
The only pragmatic thing to do here, is accept the fix, as the (mis)feature as it stands now cannot work reliably for anyone. (Because it _only_ takes the version into account and not the rest of the o= line.)
If people are interested, I might be persuaded into adding a denymediachange=yes option, to restore old behaviour (but then taking the entire o= line into account). And then, you might consider differentiating between a new media session after a provisional 1xx, vs. a new media session after the first final 200. But that feels to me like "a new feature", for which the effort might be better spent on chan_pjsip instead.
{quote}
You are quoting from the SIP RFC 3261 which talks about sessions and modifying sessions. What I am referring to is the SDP RFC 3264 which is very specific about modifying the SDP characteristics in an active session.
{quote}
Yes, but we're implementing both. There is no chan_sip driver without SIP or without SDP. And I'm arguing that the former forces you to implement multiple sessions (not going to do that). Or, if that's too much to ask (which it is), implement something that's as close to it as possible, for the greatest interoperability (which is, accepting the new media source).
{quote}
My point was that if we want to go down the path of history, there are several issues on this issue tracker where we have already had discussions around endpoints trying to change the media source on an active session and they do so by changing the "o=" line. In those discussions the conclusion was that trying to change the "o=" line more than just incrementing the version number with an active session is not proper behavior. Those conclusions are based on RFC3264 . We have directed people to use "ignoresdpversion=yes" if they so desire.
{quote}
I can see how it feels like defeat, if you're changing your stance about something you have been defending forever. But now that you realise the (undesired) randomness of the current behaviour, there are three sane positions you can take on this matter:
- fix it, by making it stricter
- fix it, by making it looser (my patch)
- don't care about chan_sip (also, my patch)
> 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)
More information about the asterisk-bugs
mailing list