[asterisk-dev] [Code Review] 3700: sip.conf progressinband=never should mean 'never'

[Matt Jordan]

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

Ship it!


Ship It!

- Matt Jordan


On July 2, 2014, 10:45 a.m., one47 wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3700/
> -----------------------------------------------------------
> 
> (Updated July 2, 2014, 10:45 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23972
>     https://issues.asterisk.org/jira/browse/ASTERISK-23972
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> progressinband=never in sip.conf is easily defeated if an onward trunk sends a progress indication of its own. This is almost certain to happen if the onward trunk is ISDN or IAX as these technologies send a progress indication even if early media is not required. This progress message is passed to the caller, and causes the "never" option to be rather badly named.
> 
> Proposed solution, applied in this patch:
> 
> 1) In sip_write(), do not pass the media unless we have either progressed beyond INV_EARLY_MEDIA, or we are in INV_EARLY_MEDIA state, and early media is both set-up and wanted. This helps resolve double-ringing on some buggy handsets.
> 
> 2) In sip_indicate(), if we see AST_CONTROL_PROGRESS, but SIP_PROG_INBAND_NEVER is set, send a 180 Ringing instead to avoid implicitly enabling early media. Avoid sending double ring indications.
> 
> NOTE: the meaning of the SIP_PROGRESS_SENT flag changes slightly in this patch to also encapsulate the fact that a channel has *sent or received* a 183 Progress indication. This makes the updated code in sip_write() much more simple.
> 
> NOTE2: Not sure this change is safe for Asterisk 11 as it may cause an unexpected change of behaviour for some users.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 417704 
> 
> Diff: https://reviewboard.asterisk.org/r/3700/diff/
> 
> 
> Testing
> -------
> 
> The change to sip_write() and SIP_PROGRESS_SENT has been tested on a couple of hundred live servers running 1.6.2, 1.8.2x and 11.10 - No obvious issues, and correctly resolves some double ringing and no-ringing issues with Polycom and Yealink handsets.
> 
> The change to sip_indicate() tested with progressinband=never and progressinband=no with both SIP and Websockets endpoints. Intended behaviour observed.
> 
> 
> Thanks,
> 
> one47
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140708/becadf3d/attachment.html>