> On April 10, 2014, 6:52 p.m., Mark Michelson wrote: > > Well, this is really cool. I love reviews that mostly remove code in order > > to simplify something. Some concerns: > > > > 1) Documentation wise, there is a mixture of nomenclature used for the > > subchannels of a local channel. You have settled on the terms "master" and > > "outbound" for the two. In other places, they are referred to as ";1" and > > ";2" since that's how they end up getting named, and in other places they > > are referred to as "owner" and "chan" since that's what they're called in > > the unreal pvt structure. I think that something needs to be settled on so > > we can be consistent in what we call things. > > > > 2) Implementation wise, the biggest flaw I can spot is that the state of a > > bridge may change for the worse between when the optimization task is > > queued and when the optimization task actually executes. For instance, a > > third party may enter a bridge during that gap, resulting in a potentially > > bizarre channel arrangement afterwards. > > > > 3) Also implementation wise, I notice that the "swap" optimizations are the > > only kind you attempt to do. Any particular reason you do not handle > > "merge" optimizations?
1) Agreed 2) I'll see if I can add some more protection to confirm sanity 3) Things will never be in a state where a merge actually makes sense to do - Joshua ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3433/#review11554 ----------------------------------------------------------- On April 9, 2014, 7: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, 7: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 > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
