[Asterisk-code-review] channels/chan sip: 180 Ringing not sent after 183 Session Pr... (asterisk[13])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: channels/chan_sip: 180 Ringing not sent after 183 Session Progress
......................................................................


Patch Set 1:

> Many SIP devices open their media paths to allow the caller to hear
 > the early media when they receive a 183.  If a 180 follows then the
 > device does not generate its own ringback tone because it is
 > passing early media assuming that there is something to actually
 > listen to.
 > 
 > Maybe what should happen is Asterisk needs to generate its own
 > inband ringback tone in this situation so a ringback can actually
 > be heard.

It appears as if that is what would happen already, unilaterally. Really, the change only makes it so that if an upstream device sends a 180, and the peer does not have progressinband=yes, then we'll send a 180 downstream, and hope that things don't care that there are already RTP ports opened.

I'm not sure about this change, if for no other reason than we now have a new behaviour being introduced for a default setting. That makes me worried: it would not surprise me if this caused something 'interesting' to occur for people's existing Asterisk installations. Given that this is being introduced to work around something occurring in an upstream piece of equipment that is already kind of strange - sending a 180 after already sending a 183 simply because you're translating an ISUP code and haven't kept track of your own state is a bit odd - I'm not sure we're going to be making the world a better place with this patch.

I'm not against having this go into master with an UPGRADE note indicating the change, but having it go into an LTS where it may break an existing Asterisk installation feels wrong.

Morten: I'm sure that doesn't sound great. If you can think of a way to mitigate risk for existing Asterisk users, I'd be happy to have this be rethought of for Asterisk 11 and 13. If you're okay with this going into 'master' only, then we can move down that path.

-- 
To view, visit https://gerrit.asterisk.org/1696
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I498fed853128831b80536f8818cd7b60e641f39c
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Morten Tryfoss <morten at tryfoss.no>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Morten Tryfoss <morten at tryfoss.no>
Gerrit-Reviewer: Olle Johansson <oej at edvina.net>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: No