[asterisk-dev] [Code Review] 2459: bridge_construction: apply Jitterbuffers at bridge join via the JITTERBUFFER function.

[jrose]

> On April 18, 2013, 11:04 a.m., Joshua Colp wrote:
> > /team/group/bridge_construction/main/abstract_jb.c, lines 614-637
> > <https://reviewboard.asterisk.org/r/2459/diff/1/?file=36164#file36164line614>
> >
> >     According to my reading of func_jitterbuffer you can have empty arguments - it'll just use the default value. That means you don't need to do this last_arg stuff.

The reason for the last arg stuff was less about setting defaults and more about making certain I only added the needed number of commas, but it turns out that's probably completely unnecessary anyway.  This is all dropping anyway with Jason's comments.


> On April 18, 2013, 11:04 a.m., Joshua Colp wrote:
> > /team/group/bridge_construction/main/abstract_jb.c, lines 590-597
> > <https://reviewboard.asterisk.org/r/2459/diff/1/?file=36164#file36164line590>
> >
> >     You can drop this.

And so I did.


> On April 18, 2013, 11:04 a.m., Joshua Colp wrote:
> > /team/group/bridge_construction/main/abstract_jb.c, line 581
> > <https://reviewboard.asterisk.org/r/2459/diff/1/?file=36164#file36164line581>
> >
> >     Pedantically it's not loaded.
> 
> Matt Jordan wrote:
>     We probably don't want a warning either - this will get spamarific (tm)
>     
>     If you haven't loaded func_jitterbuffer, a one-time warning message is good. This should be a debug log statement only.
> 
> Jason Parker wrote:
>     If we're going to be using this from the core, why not just move the function there as well?

> If we're going to be using this from the core, why not just move the function there as well?

functionality have been made core and the function has been altered to rely on rely on abstract_jb API. I'm no longer relying on the outside function.


- jrose


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


On April 17, 2013, 11:01 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2459/
> -----------------------------------------------------------
> 
> (Updated April 17, 2013, 11:01 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-21333
>     https://issues.asterisk.org/jira/browse/ASTERISK-21333
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch adds support for applying jitterbuffers based on channel settings for jitterbuffers when bridging channels with simple bridges and softmixed bridges by use of func_jitterbuffer.
> 
> 
> Diffs
> -----
> 
>   /team/group/bridge_construction/funcs/func_jitterbuffer.c 385983 
>   /team/group/bridge_construction/configs/confbridge.conf.sample 385983 
>   /team/group/bridge_construction/bridges/bridge_softmix.c 385983 
>   /team/group/bridge_construction/bridges/bridge_simple.c 385983 
>   /team/group/bridge_construction/CHANGES 385983 
>   /team/group/bridge_construction/include/asterisk/abstract_jb.h 385983 
>   /team/group/bridge_construction/main/abstract_jb.c 385983 
> 
> Diff: https://reviewboard.asterisk.org/r/2459/diff/
> 
> 
> Testing
> -------
> 
> Tested simple two way calls with SIP with the SIP option jbenable on and off.  While on the jitterbuffer is applied as expected and while off it is not applied. Note that this is different from behavior in trunk. In trunk if a SIP channel calls another SIP channel with jbenabled=yes, there generally will not be a jitterbuffer created unless jbforced is set. This is because trunk checks both channels in a two way bridge for the following condition:
> 
> if (((!c0_wants_jitter && c1_creates_jitter) || (c0_force_jb && c1_creates_jitter)) && c0_jb_enabled) {
>    /* jitter buffer creation flag is raised for c0 */
>    ...
> }
> 
> and since SIP has the wants_jitter flag enabled, the jitter buffer is not applied.
> 
> We are no longer reaching across the bridge to look at both channels when doing this, so the jitter buffer will be applied unconditionally on bridging. This means the jbforced option is no longer respected.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130418/ddf2efe9/attachment-0001.htm>