[Home]

Summary:ASTERISK-07767: [branch][post 1.4] introduce jittertargetextra option to jitter buffer via iax.conf
Reporter:jeff oconnell (jeff_oconnell)Labels:
Date Opened:2006-09-18 16:24:34Date Closed:2006-12-20 18:30:49.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_iax2
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 7978tests.txt
( 1) jittertargetextra.patch
Description:i've put together some patches which introduce a option to iax.conf
that allows a manual override of the hard-coded  JB_TARGET_EXTRA
value in the jitter buffer.

the goal of this option is to help users that have networks with normally
low jitter, but occasionally spikes.

increasing the value introduces delay, but only as a trade-off with
susceptibility to jitter.

comparable changes have already been made to the jitter buffer
code in the iaxclient project ( http://iaxclient.sourceforge.net/ ).
Comments:By: Serge Vecher (serge-v) 2006-09-19 08:40:28

Jeff: thank you for you contribution. Please get a disclaimer on file (see bottom of http://bugs.digium.com/main_page.php) and confirm with a note here when done; so we can proceed with reviewing this. Thanks.

Edit: also, if you will, please consolidate all patches into one.


By: jeff oconnell (jeff_oconnell) 2006-09-19 10:45:45

i've faxed in the disclamer and attached a unified patch file: jittertargetextra.patch
By: Russell Bryant (russell) 2006-09-28 11:06:16

This looks good and I'm just about ready to merge it.  There is one little tweak that needs to be made, though, I think.

In the trunk, the adaptive jitterbuffer is used both directly in chan_iax2, as well as in the core for the generic jitterbuffer implementation.  chan_iax2 will initialize target_extra to a default value of JB_TARGET_EXTRA.  However, for the generic implementation, target_extra should be initialized in the jitterbuffer itself.  Otherwise, it will just always be 0.

I think it's as simple as changing:

jb->info.current = jb->info.target = JB_TARGET_EXTRA;

to

jb->info.current = jb->info.target =jb->info.target_extra =  JB_TARGET_EXTRA;

Also, due to some rearranging of the source tree, the files are in different places so the current patch will have to be tweaked to reflect the new location of the jitterbuffer implementation.


Thanks!
By: pj (pj) 2006-10-27 02:29:27

Hello, can somebody adjust this patch to be this usefull feature will soon commited to trunk/branch?
By: jeff oconnell (jeff_oconnell) 2006-10-27 08:39:26

i'll make the changes russell asked for next week and then resubmit the patches.
By: Steve Murphy (murf) 2006-11-16 17:52:36.000-0600

jeff_oconnell--

how goes the battle? Have you been able to work on this?
By: Steve Murphy (murf) 2006-11-17 12:25:52.000-0600

OK, I've prepared a branch with the patch adapted and applied.
Russell's suggested change is in there, too.

Please, test this and tell me if it's OK. I'd hate to toss this work, because
people were too busy to test it.

Branch = http://svn.digium.com/svn/asterisk/team/murf/bug7978
By: Steve Murphy (murf) 2006-11-17 12:28:38.000-0600

If anyone wants this enough: I'll merge it, if you test it and say it works.

By: jeff oconnell (jeff_oconnell) 2006-11-18 08:39:37.000-0600

murf,

we've been using our patched asterisk v1.2.x version
in production since the end of september without any
reported problems.

we're going to be moving to asterisk v1.4.x in the next month or so.  

when we do so, we'll be sure to test your adapted changes
( if no one else does so before hand ).

my initial patch was targetted to the v1.2.x branch.
if i make russell's change, can that go into the next v1.2?
is that branch still taking patches?
By: Russell Bryant (russell) 2006-11-18 11:17:39.000-0600

Since this is a new feature, it is not appropriate for 1.2, or even 1.4 at this point.  It can only be applied to the trunk.

Also, the problem that I brought up is only relevant to 1.4 or trunk.  In 1.2, chan_iax2 is the only place that uses this jitterbuffer code.  The problem that I brought up lies in the case that the generic jitterbuffer is using this code.

Thanks,
By: Harmen van der Wal (harmen) 2006-12-18 10:29:18.000-0600

Jeff asked me to test the patches in http://svn.digium.com/svn/asterisk/team/murf/bug7978. I did and they work as expected.
See the attached 7978tests.txt.

Note maxjitterbuffer needs to be at least as big as jittertargerextra.
By: pj (pj) 2006-12-18 10:38:38.000-0600

what about to rename this feature to minjitterbuffer?
maybe more clear that jittertargetextra...
By: Steve Murphy (murf) 2006-12-19 12:17:44.000-0600

What does everyone think about renaming the option? On the one hand, the
esoteric option names are a hand-me-down from the developers, to whom these names
have a natural and meaningful name, but they are the only people on earth, nearly,
to whom these names have any meaning.
On the other hand, even the minjitterbuffer option name is meaningless.
My opinion is that, as long as the option is explained well, its purpose, its consequences, its pros and cons, etc. in the config file, it doesn't matter if it's called 'zippety-do-dah-day'.
By: Leif Madsen (lmadsen) 2006-12-19 12:27:23.000-0600

I vote for zippety-do-dah-day as the name.

I agree with murf as long as they are explained well, then it doesn't really matter what it is called. However, I feel that jittertargetextra is going to be an easy option to mistype and could be difficult to see for some people, thus causing additional support issues in #asterisk. Notice harmen mistyped it in his comment on the 18th.

I'm not entirely sure if that is really something that needs to be heavily considered though. I guess minjitterbuffer is easier to type and see errors in.
By: jeff oconnell (jeff_oconnell) 2006-12-19 22:59:39.000-0600

the problem with calling the setting "min" is that there's already a concept of "min" in the jb code.  ( see history_get( jitterbuf *jb ) in  main/jitterbuf.c. )

this "extra" value is actually tacked on to the jitter on top of the minimum:
jb->info.target = jb->info.jitter + jb->info.min + jb->info.conf.target_extra ;

granted from a user's perspective this won't matter much, but since there is potential confusion in the mapping of the setting name to the code's variable names - and "minjitterbuffer" is somewhat as vague - i say we leave it as is and explain the setting very clearly.
By: pj (pj) 2006-12-20 12:19:41.000-0600

I have wireless connection (cdma), with max. jitter about 800ms, I set jitterextra to this value and this was reflected in iax show netstats (jitterbuffer delay never shrinks below this value). But sound has still gaps, when connection RTT significantly changes eg. from RTT 100 to 350ms or more. Any jitterextra value has no subjective efect for me.
But what I discovered is, that ilbc codec have much worse (!) result than gsm. I'm attaching iax2 debug jb output, you can see, that letters LLLllll etc., if any group of these letters appears gaps in speech occur, with gsm result is significatly better...
Any idea about codecs or any idea, how to confirm, that jitterextra realy working?


iLBC:
vvvvvvvvvvsvvvvvvvvvvvvvvvvSLLLSL
vvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvv
vvsvLLSLLLSvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvv
vvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvv
svvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvv
vvvvvvvvsvvvvvvvvvSLLLSLvvvvvvvvvvvvvvvvsvvvvvvvvv
vvvvvvvvsLLLSLLLvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsv
vvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvv
vvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvv
vvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvv
vvvvvvvvvvsvvvvvvvSLLLSLLLSvvvvvvvvvvvvvvvvvlslvlv
lvlvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvv
vvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvv
vvvvSLLLSLLLvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvv
vvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvSLLLSLLvvvvvvvv
vvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvv
vvvvvvvvSLLLSLLvvvvvvvvvvvSLLLSLLvvvvvvvvvvvvvvvsv
vvvvvvlvlvlvlvlvvvvvlvlslvlvlvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvv
vvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvSvvvvvvv
vvvvvvvvvvsvLLLLLLvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvvvvvvvvvsvvvvvvvvv
vvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvv
vvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsv
vvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvv
vvsvvvvvvvvvvvvvvvvvSLLLSLvvvvvvvvvvvvvvvvsvvvvvvv
vvvvvvvvvvsvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvLLLLLLLvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvLLLLLLvsvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvsvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvLL
LLLLvvvvvvvvvvsvvvvvvvvvvvvvvvvvsvvvvvvvvvvvlvlvlv
lvlvvvvvvvvvvvvvvvvLLLLLLLvvvvvvvvvvvvvvvvvvvvvvvv




GSM:
Vvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvsvvvvvvvvvvvvvvvvvvvvvvvvvvsvvvvvvvvvvv
vvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvvvvvvvvvvsvvvvvvvv
vvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvvvvvvvvvvsvvvv
vvvvvvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvvvvvvvvvvs
vvvvvvvvvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvvvvvvvv
vvsvvvvvvvvvvvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvsvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvsvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvLvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvv
By: Steve Murphy (murf) 2006-12-20 18:29:30.000-0600

pj--

I don't want to seem like I'm ignoring your input; but this report has sat long enough. I've committed these changes to trunk via r. 48663. If you indeed find some problems, feel free to reopen this issue.

It might be interesting to see how various settings of the jittertargetextra setting affect your situation. I'd start out with default values, and increase them by small steps, until it is pointless to continue. This option may not
have any positive benefit for your situation.
By: Steve Murphy (murf) 2006-12-20 18:30:48.000-0600

Closing this, but if there's problems, feel free to re-open it.