[asterisk-dev] [Code Review] 2508: Create unreal channel framework for derivative channels like local channels.

Mark Michelson reviewboard at asterisk.org
Wed May 8 16:55:11 CDT 2013 

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


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_


/team/group/bridge_construction/include/asterisk/core_local.h
<https://reviewboard.asterisk.org/r/2508/#comment16474>

    What's this about?



/team/group/bridge_construction/include/asterisk/core_local.h
<https://reviewboard.asterisk.org/r/2508/#comment16476>

    Vossel is leaking :)



/team/group/bridge_construction/include/asterisk/core_local.h
<https://reviewboard.asterisk.org/r/2508/#comment16472>

    Is the joining of the bridge synchronous or asynchronous? In other words, will ast_call() return when the channel joins the bridge or when the channel leaves the bridge()?
    
    Also, this function should take an ast_bridge_features() struct as a parameter.



/team/group/bridge_construction/main/core_local.c
<https://reviewboard.asterisk.org/r/2508/#comment16475>

    Should probably get rid of the ast_ prefix if this structure is not public.



/team/group/bridge_construction/main/core_local.c
<https://reviewboard.asterisk.org/r/2508/#comment16486>

    I think destructors for unreal channels are handled a bit backwards.
    
    It makes sense to have ast_unreal_alloc() take a custom destructor. However, why not have ast_unreal_destructor() always passed as the destructor to ao2_alloc() and then have a callback within ast_unreal_destructor() call the custom function that was passed into ast_unreal_alloc()?
    
    This way, destructors passed into ast_unreal_alloc() don't have to remember to call ast_unreal_destructor().



/team/group/bridge_construction/main/core_local.c
<https://reviewboard.asterisk.org/r/2508/#comment16485>

    There have been three new operations added that result in searches through the container for a specific local_pvt. Do you think this would be grounds for using a more search-friendly container type?


- Mark Michelson


On May 7, 2013, 10:55 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2508/
> -----------------------------------------------------------
> 
> (Updated May 7, 2013, 10:55 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21713
>     https://issues.asterisk.org/jira/browse/ASTERISK-21713
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   /team/group/bridge_construction/CHANGES 387867 
>   /team/group/bridge_construction/apps/app_dial.c 387867 
>   /team/group/bridge_construction/apps/app_followme.c 387867 
>   /team/group/bridge_construction/apps/app_queue.c 387867 
>   /team/group/bridge_construction/bridges/bridge_builtin_features.c 387867 
>   /team/group/bridge_construction/channels/chan_agent.c 387867 
>   /team/group/bridge_construction/channels/chan_local.c 387867 
>   /team/group/bridge_construction/channels/chan_sip.c 387867 
>   /team/group/bridge_construction/include/asterisk/_private.h 387867 
>   /team/group/bridge_construction/include/asterisk/bridging.h 387867 
>   /team/group/bridge_construction/include/asterisk/ccss.h 387867 
>   /team/group/bridge_construction/include/asterisk/core_local.h PRE-CREATION 
>   /team/group/bridge_construction/main/asterisk.c 387867 
>   /team/group/bridge_construction/main/bridging.c 387867 
>   /team/group/bridge_construction/main/channel.c 387867 
>   /team/group/bridge_construction/main/core_local.c PRE-CREATION 
>   /team/group/bridge_construction/main/pbx.c 387867 
> 
> Diff: https://reviewboard.asterisk.org/r/2508/diff/
> 
> 
> Testing
> -------
> 
> Local channels still optimize out.
> The /n option prevents optimization.
> The /m option still works.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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

More information about the asterisk-dev mailing list