[asterisk-dev] [Code Review] 2558: Eliminate most calls to ast_channel_internal_bridged_channel().

rmudgett reviewboard at asterisk.org
Mon May 20 20:05:25 CDT 2013



> On May 21, 2013, 12:41 a.m., Matt Jordan wrote:
> > /team/group/bridge_construction/channels/chan_vpb.cc, lines 2269-2277
> > <https://reviewboard.asterisk.org/r/2558/diff/1/?file=38352#file38352line2269>
> >
> >     I think there should still be an ast_verb here indicating whether or not it got an Asterisk bridge.
> >     
> >     The verbose message just prior to this informs the user that we're checking for bridges, and that there wasn't a native bridge. If we find a bridge, we should tell them.

This old channel driver is using those ast_verb(5,..) calls as ast_debug() calls.  This is the main monitor loop so these messages are going to spam if you have the verbosity level set to 5 or higher.

I'm just going to also remove the silly ast_verb() message.


> On May 21, 2013, 12:41 a.m., Matt Jordan wrote:
> > /team/group/bridge_construction/main/channel.c, lines 6648-6659
> > <https://reviewboard.asterisk.org/r/2558/diff/1/?file=38354#file38354line6648>
> >
> >     We may be better off making this a function of the bridging framework.
> >     
> >     Essentially, as each channel joins a bridge, the oldest linkedid should be propagated between all pairs of channels.
> >     
> >     This is currently called in two places:
> >     (1) In channel masquerade. To some extent this shouldn't really be done any longer - we don't really want a channel to have its linkedid change, even if it replaces a channel that had an older linkedid. The two channels aren't really 'related', they're simply swapping with each other.
> >     (2) In features.c as two channels are bridged.
> >     
> >     We don't have to do this now, but you may want to update the BUGBUG to reflect that this should be handled by bridging (unless you're in an infinite wait bridge...)

Updated the BUGBUG comment.  BRIDGEPEER needs to be handled similarly.


- rmudgett


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


On May 20, 2013, 11:03 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2558/
> -----------------------------------------------------------
> 
> (Updated May 20, 2013, 11:03 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The new bridging API will never set the ast_channel_internal_bridged_channel().  This patch removes most calls to ast_channel_internal_bridged_channel().
> 
> 
> Diffs
> -----
> 
>   /team/group/bridge_construction/main/channel.c 389333 
>   /team/group/bridge_construction/include/asterisk/channel.h 389333 
>   /team/group/bridge_construction/channels/chan_vpb.cc 389333 
>   /team/group/bridge_construction/channels/chan_unistim.c 389333 
>   /team/group/bridge_construction/apps/app_dumpchan.c 389333 
>   /team/group/bridge_construction/CHANGES 389333 
>   /team/group/bridge_construction/main/cli.c 389333 
>   /team/group/bridge_construction/main/features.c 389333 
>   /team/group/bridge_construction/main/manager.c 389333 
> 
> Diff: https://reviewboard.asterisk.org/r/2558/diff/
> 
> 
> Testing
> -------
> 
> Checked output of DumpChan() output.
> Checked output of AMI Status action.
> Checked CLI "core show channels verbose" and "core show channel x".
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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


More information about the asterisk-dev mailing list