<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/2508/">https://reviewboard.asterisk.org/r/2508/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 8th, 2013, 9:55 p.m. UTC, <b>Mark Michelson</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;">There are a bunch of functions in core_local.h where it's not clear why they were made public. For instance, why are the channel technology callbacks public? Why is the unreal_pvt destructor public?
You need to standardize on "unreal" or "local" because right now it appears that which one is used in specific instances is arbitrary. For instance, I'd call ast_</pre>
</blockquote>
<p>On May 8th, 2013, 11:19 p.m. UTC, <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;">There are two things here. The unreal framework and local channels using the unreal framework. The unreal framework is used to create channel types that are similar to local channels. The unreal framework is the equivalent to an abstract base class. chan_agent will use the unreal framework to create Agent;1--Agent;2 channels just like local channels thus the common channel technology callbacks need to be made public.</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;">You know what's really weird? I completely changed what this comment said because after looking at the implementation of core_local.c some more, it completely clicked what you were doing. I'm completely blaming reviewboard for keeping the old version of my comment when I published my review.
But like I said, I *changed* the comment, I didn't remove it.
What I actually wanted to say was that "unreal" and "local" seem to be intermixed too much here, which can cause confusion for someone trying to figure out what it is they should be using. So what I suggest is that the unreal and local stuff should be split apart into separate files. This way, it becomes more obvious to readers of the code that the unreal is an abstract channel API and local is a user of that API.</pre>
<br />
<p>- Mark</p>
<br />
<p>On May 8th, 2013, 11:36 p.m. UTC, rmudgett 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.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By rmudgett.</div>
<p style="color: grey;"><i>Updated May 8, 2013, 11:36 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-21713">ASTERISK-21713</a>
</div>
<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;">Part of the bridging work being done needs the ability to manipulate local channels in new ways:
1) When one-touch parking a multi-party bridge, a local channel needs to be created between the bridge being parked and the park holding bridge. In this case, the local;2 channel needs to be pushed into the parking bridge since there isn't a dialplan location to execute.
2) When attended transferring a multi-party bridge to an application like voicemail using channel driver protocol transfers. A local channel needs to be created between the transferred bridge and the application. In this case, the local;2 channel needs to masquerade into the channel running voicemail.
3) The ConfBridge announcer channels need to replace the chan_bridge channels with an unreal channel framework channel for implementation reasons. (The chan_bridge driver will be deleted as a result.)
4) The chan_agent channel driver will use the unreal channel framework to create Agent channels.
This patch refactors chan_local.c into core_local.c, extracts an unreal channel driver framework to create derivative channel types similar to local channels, and moves it into the system core.
The new API calls for custom local channel manipulation are:
ast_local_get_peer() - Get the other local channel in the pair. Useful for getting the local;2 channel after an ast_request().
ast_local_setup_bridge() - After performing an ast_request(), using this call will make the subsequent ast_call() push the local;2 channel into the specified bridge.
ast_local_setup_masquerade() - After performing an ast_request(), using this call will make the subsequent ast_call() masquerade the local;2 channel into the specified channel.
Changes/fixes in local/unreal channel behavior:
* Made unreal COLP indicate handling always set the caller information on the other unreal channel in the pair. Previously this happened only in the local;2 to local;1 direction.
* Fixed using the wrong callerid when checking if the exten exists in local_call().
* Renamed LOCAL_MOH_PASSTHRU to AST_UNREAL_MOH_INTERCEPT and invert the use of the flag to match the new name. This makes the unreal channels default to not keeping state information in case they optimize. Local channels still behave the same for the /m option. I would love to make local channels not default to handling the HOLD/UNHOLD frames because this represents state that will disappear when the local channels optimize out. Unfortunately, there is likely too much inertia expecting this behavior.
</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;">Local channels still optimize out.
The /n option prevents optimization.
The /m option still works.</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>/team/group/bridge_construction/CHANGES <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/apps/app_dial.c <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/apps/app_followme.c <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/apps/app_queue.c <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/bridges/bridge_builtin_features.c <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/channels/chan_agent.c <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/channels/chan_local.c <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/channels/chan_sip.c <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/include/asterisk/_private.h <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/include/asterisk/bridging.h <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/include/asterisk/ccss.h <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/include/asterisk/core_local.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/team/group/bridge_construction/main/asterisk.c <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/main/bridging.c <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/main/channel.c <span style="color: grey">(388107)</span></li>
<li>/team/group/bridge_construction/main/core_local.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/team/group/bridge_construction/main/pbx.c <span style="color: grey">(388107)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2508/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>