[Home]

Summary:ASTERISK-17659: [patch] "sip show settings" shows wrong values for jitter buffer settings
Reporter:Rob Gagnon (rgagnon)Labels:
Date Opened:2011-04-07 15:10:47Date Closed:2011-05-05 17:55:11
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:1.8.3 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) issue_19083-r313094.diff
( 1) issue-19083-trunk-r313139.diff
Description:Default settings indicated for jitter buffer within sip.conf.sample are not used or indicated in chan_sip when issuing the 'sip show settings' command.

Attached patch corrects this, as well as adds display of "jbtargetextra" value

****** ADDITIONAL INFORMATION ******

sip.conf.sample shows that the default settings for various jitter buffer settings are as follows:

jbenable = no
jbforce = no
jbmaxsize = 200
jbresyncthreshold = 1000
jbimpl = fixed
jbtargetextra = 40
jblog = no

Internally, none of these default settings are actually stored in the "global_jbconf" struct unless they are present in sip.conf.

Patch attached fixes this, as well as removes output of most jitter buffer settings if "jbenable" is set to "no".  The value of "jbtargetextra" is also now shown conditionally depending on the value of "jbimpl".

Existing behavior of "sip show settings":
=========================================

No entries in sip.conf for "jb*" settings:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Jitterbuffer enabled:   No
 Jitterbuffer forced:    No
 Jitterbuffer max size:  -1
 Jitterbuffer resync:    -1
 Jitterbuffer impl:      
 Jitterbuffer log:       No

Only "jbenable=yes" in sip.conf:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Jitterbuffer enabled:   Yes
 Jitterbuffer forced:    No
 Jitterbuffer max size:  -1
 Jitterbuffer resync:    -1
 Jitterbuffer impl:      
 Jitterbuffer log:       No

Only "jbenable=yes" and "jbimpl=adaptive" in sip.conf:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Jitterbuffer enabled:   Yes
 Jitterbuffer forced:    No
 Jitterbuffer max size:  -1
 Jitterbuffer resync:    -1
 Jitterbuffer impl:      adaptive
 Jitterbuffer log:       No


Expected behavior, behavior created by attached patch:
======================================================

No entries in sip.conf for "jb*" settings:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Jitterbuffer enabled:   No

Only "jbenable=yes" in sip.conf:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Jitterbuffer enabled:   Yes
 Jitterbuffer forced:    No
 Jitterbuffer max size:  200
 Jitterbuffer resync:    1000
 Jitterbuffer impl:      fixed
 Jitterbuffer log:       No

Only "jbenable=yes" and "jbimpl=adaptive" in sip.conf:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Jitterbuffer enabled:   Yes
 Jitterbuffer forced:    No
 Jitterbuffer max size:  200
 Jitterbuffer resync:    1000
 Jitterbuffer impl:      adaptive
 Jitterbuffer tgt extra: 40
 Jitterbuffer log:       No
Comments:By: Michael L. Young (elguero) 2011-04-08 12:52:49

I just tested this patch and it appears to work as expected.  The default values are now being set as one would expect.

It looks like chan_dahdi.c could use the same patch as far as setting the default values to reflect what chan_dahdi.conf.sample shows.
By: Rob Gagnon (rgagnon) 2011-04-08 13:06:12

Good point.  I might as well be thorough.  I'll check all the others as well...

alsa, dahdi, console, h323, mgcp, misdn, oss, sip, skinny, unistm, and usbradio all show that they use the new "jb" settings.

I should have a new patch to cover all of those in a couple hours or so.
By: Rob Gagnon (rgagnon) 2011-04-08 14:02:27

New patch file "issue-19083-trunk-r313139.diff" applies the same changes to all channels with jitter buffer settings.

Only chan_sip and chan_skinny had a "show settings" option that allowed you to see the configuration values.

The output of "skinny show settings" has changed in the same way as for "sip show settings" noted in the original submission.
By: Michael L. Young (elguero) 2011-04-08 15:25:21

Good job.  Looks good from what I can see.  Applied the patch and did some basic testing and everything appears to be working.

This should help prevent any confusion since one would expect that the jitter buffer would have those default settings set by just enabling the jitter buffer.
By: Digium Subversion (svnbot) 2011-05-05 17:53:47

Repository: asterisk
Revision: 317478

U   branches/1.8/channels/chan_alsa.c
U   branches/1.8/channels/chan_console.c
U   branches/1.8/channels/chan_dahdi.c
U   branches/1.8/channels/chan_h323.c
U   branches/1.8/channels/chan_mgcp.c
U   branches/1.8/channels/chan_oss.c
U   branches/1.8/channels/chan_sip.c
U   branches/1.8/channels/chan_skinny.c
U   branches/1.8/channels/chan_unistim.c
U   branches/1.8/channels/chan_usbradio.c
U   branches/1.8/channels/misdn_config.c

------------------------------------------------------------------------
r317478 | russell | 2011-05-05 17:53:46 -0500 (Thu, 05 May 2011) | 12 lines

Fix some consistency issues with jitterbuffer config.

Store the defaults noted in the sample config files in the jitterbuffer config
data structure.  This makes the CLI commands that output these settings show
the right thing.  Also only show the settings that are relevant in the settings
CLI commands, based on which jitterbuffer is selected and whether it's enabled.

(closes issue ASTERISK-17659)
Reported by: rgagnon
Patches:
     issue-19083-trunk-r313139.diff uploaded by rgagnon (license 1202)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=317478
By: Digium Subversion (svnbot) 2011-05-05 17:55:11

Repository: asterisk
Revision: 317479

_U  trunk/
U   trunk/channels/chan_alsa.c
U   trunk/channels/chan_console.c
U   trunk/channels/chan_dahdi.c
U   trunk/channels/chan_h323.c
U   trunk/channels/chan_mgcp.c
U   trunk/channels/chan_oss.c
U   trunk/channels/chan_sip.c
U   trunk/channels/chan_skinny.c
U   trunk/channels/chan_unistim.c
U   trunk/channels/chan_usbradio.c
U   trunk/channels/misdn_config.c

------------------------------------------------------------------------
r317479 | russell | 2011-05-05 17:55:10 -0500 (Thu, 05 May 2011) | 19 lines

Merged revisions 317478 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

........
 r317478 | russell | 2011-05-05 17:53:45 -0500 (Thu, 05 May 2011) | 12 lines
 
 Fix some consistency issues with jitterbuffer config.
 
 Store the defaults noted in the sample config files in the jitterbuffer config
 data structure.  This makes the CLI commands that output these settings show
 the right thing.  Also only show the settings that are relevant in the settings
 CLI commands, based on which jitterbuffer is selected and whether it's enabled.
 
 (closes issue ASTERISK-17659)
 Reported by: rgagnon
 Patches:
       issue-19083-trunk-r313139.diff uploaded by rgagnon (license 1202)
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=317479