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

2015-03-10 Thread rmudgett

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

Review request for Asterisk Developers.


Bugs: ASTERISK-24781
https://issues.asterisk.org/jira/browse/ASTERISK-24781


Repository: Asterisk


Description
---

This patch builds on https://reviewboard.asterisk.org/r/4472/

When that patch is committed this patch reduces to simply adding the
rpid_immediate option to skip generating INVITE response messages as a
result of connected line updates.


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_caller_id.c 432761 
  /branches/13/res/res_pjsip/pjsip_configuration.c 432761 
  /branches/13/res/res_pjsip.c 432761 
  /branches/13/include/asterisk/res_pjsip.h 432761 
  /branches/13/configs/samples/pjsip.conf.sample 432761 
  /branches/13/channels/chan_pjsip.c 432761 

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

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

2015-03-12 Thread Matt Jordan

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


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.


/branches/13/res/res_pjsip.c


You'll need to update the alembic scripts for the new configuration option.


- Matt Jordan


On March 10, 2015, 6:48 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4473/
> ---
> 
> (Updated March 10, 2015, 6:48 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24781
> https://issues.asterisk.org/jira/browse/ASTERISK-24781
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch builds on https://reviewboard.asterisk.org/r/4472/
> 
> When that patch is committed this patch reduces to simply adding the
> rpid_immediate option to skip generating INVITE response messages as a
> result of connected line updates.
> 
> 
> 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_caller_id.c 432761 
>   /branches/13/res/res_pjsip/pjsip_configuration.c 432761 
>   /branches/13/res/res_pjsip.c 432761 
>   /branches/13/include/asterisk/res_pjsip.h 432761 
>   /branches/13/configs/samples/pjsip.conf.sample 432761 
>   /branches/13/channels/chan_pjsip.c 432761 
> 
> 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

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

2015-03-12 Thread rmudgett


> On March 12, 2015, 3: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.

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.


- rmudgett


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


On March 10, 2015, 6:48 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4473/
> ---
> 
> (Updated March 10, 2015, 6:48 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24781
> https://issues.asterisk.org/jira/browse/ASTERISK-24781
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch builds on https://reviewboard.asterisk.org/r/4472/
> 
> When that patch is committed this patch reduces to simply adding the
> rpid_immediate option to skip generating INVITE response messages as a
> result of connected line updates.
> 
> 
> 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_caller_id.c 432761 
>   /branches/13/res/res_pjsip/pjsip_configuration.c 432761 
>   /branches/13/res/res_pjsip.c 432761 
>   /branches/13/include/asterisk/res_pjsip.h 432761 
>   /branches/13/configs/samples/pjsip.conf.sample 432761 
>   /branches/13/channels/chan_pjsip.c 432761 
> 
> 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

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

2015-03-12 Thread 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.

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?


- gareth


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


On March 10, 2015, 11:48 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4473/
> ---
> 
> (Updated March 10, 2015, 11:48 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24781
> https://issues.asterisk.org/jira/browse/ASTERISK-24781
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch builds on https://reviewboard.asterisk.org/r/4472/
> 
> When that patch is committed this patch reduces to simply adding the
> rpid_immediate option to skip generating INVITE response messages as a
> result of connected line updates.
> 
> 
> 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_caller_id.c 432761 
>   /branches/13/res/res_pjsip/pjsip_configuration.c 432761 
>   /branches/13/res/res_pjsip.c 432761 
>   /branches/13/include/asterisk/res_pjsip.h 432761 
>   /branches/13/configs/samples/pjsip.conf.sample 432761 
>   /branches/13/channels/chan_pjsip.c 432761 
> 
> 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

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

2015-03-13 Thread rmudgett

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

(Updated March 13, 2015, 12:13 p.m.)


Review request for Asterisk Developers.


Changes
---

Updated patch with https://reviewboard.asterisk.org/r/4472/ committed.


Bugs: ASTERISK-24781
https://issues.asterisk.org/jira/browse/ASTERISK-24781


Repository: Asterisk


Description (updated)
---

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

  /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

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

2015-03-13 Thread rmudgett


> On March 12, 2015, 3: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?

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.


- rmudgett


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


On March 13, 2015, 12: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, 12: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

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

2015-03-13 Thread Matt Jordan


> On March 12, 2015, 3: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.

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.


- Matt


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


On March 13, 2015, 12: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, 12: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

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

2015-03-13 Thread rmudgett


> On March 12, 2015, 3: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.

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.


- rmudgett


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


On March 13, 2015, 12: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, 12: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 a

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

2015-03-13 Thread gareth

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

Ship it!


Ship It!

- gareth


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

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

2015-03-13 Thread 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.
> 

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

2015-03-16 Thread Matt Jordan


> On March 13, 2015, 5:19 p.m., gareth wrote:
> > Ship It!

Well, if we're okay with the patch, then my initial comments still stand :-)

{quote}
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.
{quote}

{quote}
You'll need to update the alembic scripts for the new configuration option.
{quote}


- Matt


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


On March 13, 2015, 12: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, 12: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

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

2015-03-19 Thread rmudgett

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

(Updated March 19, 2015, 10:59 a.m.)


Review request for Asterisk Developers.


Changes
---

Addressed feedback: Added note in CHANGES and added alembic script.
Still need to make tests for the testsuite to cover the new rpid_immediate 
option.


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

  /branches/13/res/res_pjsip/pjsip_configuration.c 433152 
  /branches/13/res/res_pjsip.c 433152 
  /branches/13/include/asterisk/res_pjsip.h 433152 
  
/branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py
 PRE-CREATION 
  /branches/13/configs/samples/pjsip.conf.sample 433152 
  /branches/13/channels/chan_pjsip.c 433152 
  /branches/13/CHANGES 433152 

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

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

2015-03-20 Thread rmudgett

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

(Updated March 20, 2015, 12:35 p.m.)


Review request for Asterisk Developers.


Changes
---

Update the rpid_immediate option description.


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

  /branches/13/res/res_pjsip/pjsip_configuration.c 433198 
  /branches/13/res/res_pjsip.c 433198 
  /branches/13/include/asterisk/res_pjsip.h 433198 
  
/branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py
 PRE-CREATION 
  /branches/13/configs/samples/pjsip.conf.sample 433198 
  /branches/13/channels/chan_pjsip.c 433198 
  /branches/13/CHANGES 433198 

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

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

2015-03-20 Thread rmudgett

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

(Updated March 20, 2015, 3:50 p.m.)


Review request for Asterisk Developers.


Changes
---

Add link to corresponding testsuite test review.


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 433198 
  /branches/13/res/res_pjsip.c 433198 
  /branches/13/include/asterisk/res_pjsip.h 433198 
  
/branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py
 PRE-CREATION 
  /branches/13/configs/samples/pjsip.conf.sample 433198 
  /branches/13/channels/chan_pjsip.c 433198 
  /branches/13/CHANGES 433198 

Diff: https://reviewboard.asterisk.org/r/4473/diff/


Testing (updated)
---

* 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.

* https://reviewboard.asterisk.org/r/4518/ testsuite test passes.


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

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

2015-03-20 Thread Corey Farrell

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



/branches/13/include/asterisk/res_pjsip.h


Is this an ABI issue?  Maybe this member should be last in the structure 
for v13 to avoid changing the offset of send_diversion and refresh_method.  
Seems like it's in the right place for trunk (keeping unsigned int's together). 
 Not sure if it even matters for res_pjsip to provide stable ABI.


- Corey Farrell


On March 20, 2015, 4:50 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4473/
> ---
> 
> (Updated March 20, 2015, 4:50 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 433198 
>   /branches/13/res/res_pjsip.c 433198 
>   /branches/13/include/asterisk/res_pjsip.h 433198 
>   
> /branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py
>  PRE-CREATION 
>   /branches/13/configs/samples/pjsip.conf.sample 433198 
>   /branches/13/channels/chan_pjsip.c 433198 
>   /branches/13/CHANGES 433198 
> 
> 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.
> 
> * https://reviewboard.asterisk.org/r/4518/ testsuite test passes.
> 
> 
> 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

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

2015-03-23 Thread rmudgett


> On March 20, 2015, 11:50 p.m., Corey Farrell wrote:
> > /branches/13/include/asterisk/res_pjsip.h, lines 418-419
> > 
> >
> > Is this an ABI issue?  Maybe this member should be last in the 
> > structure for v13 to avoid changing the offset of send_diversion and 
> > refresh_method.  Seems like it's in the right place for trunk (keeping 
> > unsigned int's together).  Not sure if it even matters for res_pjsip to 
> > provide stable ABI.

Ugh.  Yes it is an ABI change.  Even worse, the new value cannot go at the end 
of the struct because the struct itself is contained within a key struct 
(ast_sip_endpoint).  This is going to put a damper on new options in res_pjsip 
for v13.


- rmudgett


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


On March 20, 2015, 3:50 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4473/
> ---
> 
> (Updated March 20, 2015, 3:50 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 433198 
>   /branches/13/res/res_pjsip.c 433198 
>   /branches/13/include/asterisk/res_pjsip.h 433198 
>   
> /branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py
>  PRE-CREATION 
>   /branches/13/configs/samples/pjsip.conf.sample 433198 
>   /branches/13/channels/chan_pjsip.c 433198 
>   /branches/13/CHANGES 433198 
> 
> 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.
> 
> * https://reviewboard.asterisk.org/r/4518/ testsuite test passes.
> 
> 
> 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

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

2015-03-23 Thread Matt Jordan


> On March 20, 2015, 11:50 p.m., Corey Farrell wrote:
> > /branches/13/include/asterisk/res_pjsip.h, lines 418-419
> > 
> >
> > Is this an ABI issue?  Maybe this member should be last in the 
> > structure for v13 to avoid changing the offset of send_diversion and 
> > refresh_method.  Seems like it's in the right place for trunk (keeping 
> > unsigned int's together).  Not sure if it even matters for res_pjsip to 
> > provide stable ABI.
> 
> rmudgett wrote:
> Ugh.  Yes it is an ABI change.  Even worse, the new value cannot go at 
> the end of the struct because the struct itself is contained within a key 
> struct (ast_sip_endpoint).  This is going to put a damper on new options in 
> res_pjsip for v13.

Well, it will put a damper on new options in res_pjsip that modify this struct 
at least :-)

Could you stick it on the bottom of the ast_sip_endpoint struct, and then put 
it into the correct embedded struct in trunk?


- Matt


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


On March 20, 2015, 3:50 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4473/
> ---
> 
> (Updated March 20, 2015, 3:50 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 433198 
>   /branches/13/res/res_pjsip.c 433198 
>   /branches/13/include/asterisk/res_pjsip.h 433198 
>   
> /branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py
>  PRE-CREATION 
>   /branches/13/configs/samples/pjsip.conf.sample 433198 
>   /branches/13/channels/chan_pjsip.c 433198 
>   /branches/13/CHANGES 433198 
> 
> 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.
> 
> * https://reviewboard.asterisk.org/r/4518/ testsuite test passes.
> 
> 
> 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

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

2015-03-23 Thread rmudgett


> On March 20, 2015, 11:50 p.m., Corey Farrell wrote:
> > /branches/13/include/asterisk/res_pjsip.h, lines 418-419
> > 
> >
> > Is this an ABI issue?  Maybe this member should be last in the 
> > structure for v13 to avoid changing the offset of send_diversion and 
> > refresh_method.  Seems like it's in the right place for trunk (keeping 
> > unsigned int's together).  Not sure if it even matters for res_pjsip to 
> > provide stable ABI.
> 
> rmudgett wrote:
> Ugh.  Yes it is an ABI change.  Even worse, the new value cannot go at 
> the end of the struct because the struct itself is contained within a key 
> struct (ast_sip_endpoint).  This is going to put a damper on new options in 
> res_pjsip for v13.
> 
> Matt Jordan wrote:
> Well, it will put a damper on new options in res_pjsip that modify this 
> struct at least :-)
> 
> Could you stick it on the bottom of the ast_sip_endpoint struct, and then 
> put it into the correct embedded struct in trunk?

Heh.  That's exactly what I was working on and planing to do when I merged to 
trunk. :)


- rmudgett


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


On March 20, 2015, 3:50 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4473/
> ---
> 
> (Updated March 20, 2015, 3:50 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 433198 
>   /branches/13/res/res_pjsip.c 433198 
>   /branches/13/include/asterisk/res_pjsip.h 433198 
>   
> /branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py
>  PRE-CREATION 
>   /branches/13/configs/samples/pjsip.conf.sample 433198 
>   /branches/13/channels/chan_pjsip.c 433198 
>   /branches/13/CHANGES 433198 
> 
> 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.
> 
> * https://reviewboard.asterisk.org/r/4518/ testsuite test passes.
> 
> 
> 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

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

2015-03-23 Thread rmudgett

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

(Updated March 23, 2015, 4:52 p.m.)


Review request for Asterisk Developers.


Changes
---

Addressed ABI compatibility issue.


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

  /branches/13/res/res_pjsip/pjsip_configuration.c 433316 
  /branches/13/res/res_pjsip.c 433316 
  /branches/13/include/asterisk/res_pjsip.h 433316 
  
/branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py
 PRE-CREATION 
  /branches/13/configs/samples/pjsip.conf.sample 433316 
  /branches/13/channels/chan_pjsip.c 433316 
  /branches/13/CHANGES 433316 

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.

* https://reviewboard.asterisk.org/r/4518/ testsuite test passes.


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

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

2015-03-24 Thread Matt Jordan

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

Ship it!


Ship It!

- Matt Jordan


On March 23, 2015, 4:52 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4473/
> ---
> 
> (Updated March 23, 2015, 4:52 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 433316 
>   /branches/13/res/res_pjsip.c 433316 
>   /branches/13/include/asterisk/res_pjsip.h 433316 
>   
> /branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py
>  PRE-CREATION 
>   /branches/13/configs/samples/pjsip.conf.sample 433316 
>   /branches/13/channels/chan_pjsip.c 433316 
>   /branches/13/CHANGES 433316 
> 
> 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.
> 
> * https://reviewboard.asterisk.org/r/4518/ testsuite test passes.
> 
> 
> 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

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

2015-03-24 Thread rmudgett

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

(Updated March 24, 2015, 2:26 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 48


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 433316 
  /branches/13/res/res_pjsip.c 433316 
  /branches/13/include/asterisk/res_pjsip.h 433316 
  
/branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py
 PRE-CREATION 
  /branches/13/configs/samples/pjsip.conf.sample 433316 
  /branches/13/channels/chan_pjsip.c 433316 
  /branches/13/CHANGES 433316 

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.

* https://reviewboard.asterisk.org/r/4518/ testsuite test passes.


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