[asterisk-dev] [Code Review] Bridging API for Conference Bridge purposes

[Joshua Colp]

> On 2009-03-03 05:23:28, vadim wrote:
> > /trunk/bridges/bridge_multiplexed.c, line 50
> > <http://reviewboard.digium.com/r/93/diff/5/?file=2910#file2910line50>
> >
> >     This should be static var too IMO.

See above.


> On 2009-03-03 05:23:28, vadim wrote:
> > /trunk/bridges/bridge_multiplexed.c, line 47
> > <http://reviewboard.digium.com/r/93/diff/5/?file=2910#file2910line47>
> >
> >     I wonder if this should not be a static variable to allow for run-time config.
> >     Imo this value will heavily depend on actual hardware the PBX will run

I agree that this should be configurable, but I'm going to push that to phase 2 since that is where it will actually make the most difference. We can then do tests to figure out the best solution for determining what the values should be (whether we can do it automatically) or making it configurable.


> On 2009-03-03 05:23:28, vadim wrote:
> > /trunk/bridges/bridge_multiplexed.c, line 194
> > <http://reviewboard.digium.com/r/93/diff/5/?file=2910#file2910line194>
> >
> >     What is the interest to awake the thread BEFORE modifying  the channell array?
> >     
> >

This is to ensure that the multiplexed thread will be immediately servicing the channel upon return of the API call/that the thread will not be servicing the channel upon return of the API call.


> On 2009-03-03 05:23:28, vadim wrote:
> > /trunk/bridges/bridge_multiplexed.c, line 111
> > <http://reviewboard.digium.com/r/93/diff/5/?file=2910#file2910line111>
> >
> >     This is a dumb question maybe, but don't you think a pipe would be cheaper nudging mechanism than a signal?

Good point, I will change it to use a pipe instead of a signal.


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/93/#review503
-----------------------------------------------------------


On 2009-03-02 10:49:59, Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/93/
> -----------------------------------------------------------
> 
> (Updated 2009-03-02 10:49:59)
> 
> 
> Review request for Asterisk Developers and Russell Bryant.
> 
> 
> Summary
> -------
> 
> This patch implements the new bridging API and brings with it a module for conference bridges. It does *not* replace existing internal bridging or features yet and will not cause any regressions when put in. It will essentially be introduced as a first test phase to work out any unforeseen critical issues. The bridging core itself is fully implemented besides the following: jitterbuffer support, native bridging, and interval hooks (hooks that are time based versus action based). If you would like an explanation of what the bridging API is made up of and how it works that can be found in the bridging.h header file.
> 
> 
> Diffs
> -----
> 
>   /trunk/Makefile 179359 
>   /trunk/apps/app_confbridge.c PRE-CREATION 
>   /trunk/bridges/Makefile PRE-CREATION 
>   /trunk/bridges/bridge_builtin_features.c PRE-CREATION 
>   /trunk/bridges/bridge_multiplexed.c PRE-CREATION 
>   /trunk/bridges/bridge_simple.c PRE-CREATION 
>   /trunk/bridges/bridge_softmix.c PRE-CREATION 
>   /trunk/channels/chan_bridge.c PRE-CREATION 
>   /trunk/include/asterisk/bridging.h PRE-CREATION 
>   /trunk/include/asterisk/bridging_features.h PRE-CREATION 
>   /trunk/include/asterisk/bridging_technology.h PRE-CREATION 
>   /trunk/include/asterisk/channel.h 179359 
>   /trunk/main/Makefile 179359 
>   /trunk/main/bridging.c PRE-CREATION 
> 
> Diff: http://reviewboard.digium.com/r/93/diff
> 
> 
> Testing
> -------
> 
> Conference bridge testing using app_confbridge with features. Joining two channels with simple frame exchange and joining three channels to move it to a true conference bridge. IVR capability of app_confbridge was also tested.
> 
> 
> Thanks,
> 
> Joshua
> 
>