> 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 > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
