<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3433/">https://reviewboard.asterisk.org/r/3433/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 10th, 2014, 6:20 p.m. CDT, <b>rmudgett</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On April 10th, 2014, 7:53 p.m. CDT, <b>Joshua Colp</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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?</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
<br />
<p>- rmudgett</p>
<br />
<p>On April 9th, 2014, 2:49 p.m. CDT, Joshua Colp wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Matt Jordan, Mark Michelson, and rmudgett.</div>
<div>By Joshua Colp.</div>
<p style="color: grey;"><i>Updated April 9, 2014, 2:49 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/12/main/core_unreal.c <span style="color: grey">(411867)</span></li>
<li>/branches/12/main/core_local.c <span style="color: grey">(411867)</span></li>
<li>/branches/12/main/bridge.c <span style="color: grey">(411867)</span></li>
<li>/branches/12/include/asterisk/core_unreal.h <span style="color: grey">(411867)</span></li>
<li>/branches/12/bridges/bridge_unreal.c <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3433/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>