[asterisk-dev] [Code Review] 4473: chan_pjsip: Add "rpid_immediate" option to prevent unnecessary "180 Ringing" messages.

[gareth]

> On March 12, 2015, 8:41 p.m., Matt Jordan wrote:
> > 1. For this to go into Asterisk 13, tests will need to be provided that cover the new parameter. (Really, those tests should be written regardless)
> > 2. The CHANGES file will need to get updated with the new option.
> 
> rmudgett wrote:
>     Actually I'd prefer that the rpid_immediate option not exist at all and the code it controls to just be removed.  Sending connected line updates back to the caller _before_ getting connected doesn't really make sense.  This is what the REDIRECTING information is supposed to be doing.
> 
> gareth wrote:
>     I disagree, providing connected line updates pre-answer makes perfect sense. Why would I want to know the connected-line name only after the call has been answered?
>     
>     That assumes the call even is answered, sending the connected-line information immediately allows the phone to include the name in it's call history.
>     
>     If no call-forwarding and/or re-addressing has taken place why would REDIRECTING be used?
> 
> rmudgett wrote:
>     OK, that makes sense but the code that rpid_immediate controls doesn't.  As described in the review description, that code immediately sends redundant "180 Ringing" or "183 Progress" messages at best and at worst lies that the other end is ringing with inaccurate connected line information.
> 
> Matt Jordan wrote:
>     I'm not sure what this setting is attempting to do then. The description of the new setting makes it sound like it controls sending connected line information prior to Answer, which fits gareth's interpretation. Whether or not it sends multiple 180/183 provisional responses does not seem to be implied in the description of the setting.
>     
>     If setting 'rpid_immediate' to 'yes' still results in extraneous 180/183 provisional responses, then I'm not sure this patch fixes the underlying issue.
> 
> rmudgett wrote:
>     The option does exactly the same thing as chan_sip and will be disabled by default just like chan_sip.
>     
>     When disabled it does not send the extraneous 180/183 messages.  The connected line information simply waits until there is a real reason to send the 180/183 messages.  e.g., Such as when the outgoing channel receives a 180/183 message.  When the outgoing channel receives a 180/183 message, that message may also contain updated/correct connected line information from the peer.
>     
>     When enabled it sends the connected line update information *immediately* on a 180/183 message regardless of if the far end has received the call yet or the connected line information came from the far end.  The core may do some filtering to eliminate an update event if the connected line information does not change.  However, when the peer receives a 180/183 message the caller will also get the 180/183 message generated because of the ast_indicate(AST_CONTROL_RINGING/AST_CONTROL_PROGRESS).
>     
>     The key word is immediate.  I suppose I should sprinkle immediate a few more times in the option description.  I doubt anyone even enables the option in chan_sip because it is undocumented, disabled by default, and will cause this odd behaviour when enabled.
>     
>     These are the reasons why I think the code the option controls should just be removed and the option not even exist.  The code doesn't really help and the extraneous 180/183 messages cause problems when enabled.

I have rpid_immediate enabled. I know about the option because I searched through chan_sip.c to find out why RPID updates were not working as expected.

Until the SIP protocol gains a "here is the new RPID info" response there is always going to be the problem of piggy-backing that information on a response which already had a meaning.

Patch looks fine.


- gareth


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4473/#review14669
-----------------------------------------------------------


On March 13, 2015, 5:13 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4473/
> -----------------------------------------------------------
> 
> (Updated March 13, 2015, 5:13 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24781
>     https://issues.asterisk.org/jira/browse/ASTERISK-24781
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Incoming PJSIP call legs that have not been answered yet send unnecessary
> "180 Ringing" or "183 Progress" messages every time a connected line
> update happens.  If the outgoing channel is also PJSIP then the incoming
> channel will always send a "180 Ringing" or "183 Progress" message when
> the outgoing channel sends the INVITE.
> 
> Consequences of these unnecessary messages:
> 
> * The caller can start hearing ringback before the far end even gets the
> call.
> 
> * Many phones tend to grab the first connected line information and refuse
> to update the display if it changes.  The first information is not likely
> to be correct if the call goes to an endpoint not under the control of the
> first Asterisk box.
> 
> When connected line first went into Asterisk in v1.8, chan_sip received an
> undocumented option "rpid_immediate" that defaults to disabled.  When
> enabled, the option immediately passes connected line update information
> to the caller in "180 Ringing" or "183 Progress" messages as described
> above.
> 
> * Added "rpid_immediate" option to prevent unnecessary "180 Ringing" or
> "183 Progress" messages.  The default is "no" to disable sending the
> unnecessary messages.
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/res_pjsip/pjsip_configuration.c 432895 
>   /branches/13/res/res_pjsip.c 432895 
>   /branches/13/include/asterisk/res_pjsip.h 432895 
>   /branches/13/configs/samples/pjsip.conf.sample 432895 
>   /branches/13/channels/chan_pjsip.c 432895 
> 
> Diff: https://reviewboard.asterisk.org/r/4473/diff/
> 
> 
> Testing
> -------
> 
> * Ran the tests/channels/pjsip testsuite tests.  They still pass.
> 
> * Made a call chain as follows: 100 -> * -> * -> * -> 200.  With the patch
> there are no unnecessary messages.  Without the patch there were several
> "180 Ringing" messages sent back to the caller.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150313/0ce07910/attachment-0001.html>