[asterisk-dev] [Code Review] 3433: bridge_unreal: An alternative implementation for optimizing Unreal/Local channels.

[rmudgett]

> On April 10, 2014, 6:20 p.m., rmudgett wrote:
> > I'm not seeing any protection from loss of frames when the channels optimize out.  Losing media frames isn't nice but is tollerable.  Losing control frames is unacceptable.
> 
> Joshua Colp wrote:
>     1. Can you explain the scenario and how I would lose frames.
>     
>     2. Since I'm using standard APIs we now provide if I have to do special stuff shouldn't every user?

1) frame loss
The unreal_bridge_optimize_task() is effectively a third party outside the chain.  It cannot know for certain what is happening within the chain when the channels actually get moved.  Thus it cannot prevent the potential loss of frames in the local channels being optimized without some cooperation of the bridges and local channels involved.  See the note in bridge_channel_write_frame() about a deferred write queue that could be useful to do this.  The deferred write queue would need to go with the surviving channel of the optimization that is being moved to the new bridge.

You can only optimize if the local channels involved are processing media frames at the time.

The current optimization code checks:
1) is local channel optimization enabled.
2) are the read and write queues of the local channels empty.
3) is DTMF not in progress.
4) are there any active monitors, audiohooks, or framehooks.
5) are the two local channels in bridges.
6) are the local channel threads in a safe position for the optimization.  Holding the locks blocks the local channel threads at certain points so the bridge_channel->activity and bridge_channel->state can be checked to find out what the local channel threads are up to.
7) do the bridges allow optimization and which way.

One thing I did see when trying to fix the masquerade supertest was that there is almost no contention for the seven locks needed.  The main preventer of optimization was the queues were not empty.

2) bridge API calls
The answer depends on which calls and why they are being made.  Optimization has a unique requirement when restructuring the bridges and channels.  Optimization should not be noticeable to the parties involved so losing frames is not good.  The other API users are moving channels to/from parking, to/from the agent holding bridge, or transferring calls.  The frame stream is expected to be interrupted and the frame source is expected to be changed.


- rmudgett


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


On April 9, 2014, 2:49 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3433/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 2:49 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan, Mark Michelson, and rmudgett.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change removes Unreal/Local channel optimization from the core and isolates it in a bridging technology implementation. The implementation attempts to optimize when channels join a bridge using it, instead of attempting to optimize constantly as frames pass. This review is not just for the code itself but the approach in general. Is this something we'd like? Is it viable?
> 
> Now the con...
> 
> Since this is implemented as a bridging technology and not as part of the bridging core it can not optimize completely when multi-party bridges are involved. If you have a chain in the middle that'll go anyway but no merging of bridges will occur.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/core_unreal.c 411867 
>   /branches/12/main/core_local.c 411867 
>   /branches/12/main/bridge.c 411867 
>   /branches/12/include/asterisk/core_unreal.h 411867 
>   /branches/12/bridges/bridge_unreal.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3433/diff/
> 
> 
> Testing
> -------
> 
> Since this has been an idea and side project I've only done some tests myself with large numbers of Local channels in chains (100, 200, 300) and confirmed that stuff works as expected.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

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