----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3997/#review13457 -----------------------------------------------------------
/branches/12/bridges/bridge_native_rtp.c <https://reviewboard.asterisk.org/r/3997/#comment23983> The same pointer check was to detect that there was only one channel in the bridge and shouldn't be necessary anymore. The NULL checks are necessary though. :) /branches/12/bridges/bridge_native_rtp.c <https://reviewboard.asterisk.org/r/3997/#comment23982> bc0 == bc0 ? The same pointer check was to detect that there was only one channel in the bridge and shouldn't be necessary anymore. The NULL checks are necessary though. :) /branches/12/bridges/bridge_native_rtp.c <https://reviewboard.asterisk.org/r/3997/#comment23980> sterisk? /branches/12/bridges/bridge_native_rtp.c <https://reviewboard.asterisk.org/r/3997/#comment23981> Missing return? The channel cannot leave if not part of the bridge tech. /branches/12/bridges/bridge_native_rtp.c <https://reviewboard.asterisk.org/r/3997/#comment23984> Shouldn't the pvt->bcx pointer be cleared anyway since we have just confirmed that it is in the pvt struct? /branches/12/bridges/bridge_native_rtp.c <https://reviewboard.asterisk.org/r/3997/#comment23985> For safety (high paranoia level :) ), you might want to swap the order of these lines. - rmudgett On Oct. 5, 2014, 6:08 p.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3997/ > ----------------------------------------------------------- > > (Updated Oct. 5, 2014, 6:08 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-24237 > https://issues.asterisk.org/jira/browse/ASTERISK-24237 > > > Repository: Asterisk > > > Description > ------- > > When a native RTP bridge that is remotely bridging its participants switches > to a softmix bridge, it may not properly re-INVITE the media for one or both > participants back to Asterisk. This is due to two factors: > > (1) The current bridge_native_rtp code only re-INVITEs if it believes the > channel will survive the bridge operation. Currently, that code is failing, > as it expects the channels to have a soft hangup flag set on it indicating > that a redirect has occurred or that the channel is going to leave the > bridge. (The code did not take into account a smart bridge operation). > (2) When the bridge layer performs a smart bridge, it passes a dummy bridge > down into the old mixing technology when it is stopped. That breaks the > native RTP bridge, as it looks to bridge->channels to know which channels to > re-INVITE back. That list has no entries, as the dummy bridge does not > populate that value. > > This patch modifies bridge_native_rtp such that it keeps track of the > channels itself. Given how tricky this code is - both smart bridging and > native RTP bridging - this keeps the mixing technology insulated from changes > in the core, which is probably a good thing. > > > Diffs > ----- > > /branches/12/bridges/bridge_native_rtp.c 423232 > > Diff: https://reviewboard.asterisk.org/r/3997/diff/ > > > Testing > ------- > > The tests that extercised this code the most are the PJSIP blind transfer > tests, as they change the bridge mixing technology from native_rtp to simple > and back in various tests. Shocking the callee_with_hold/caller_with_hold > tests worked right off the bat. The direct media tests still fail, but this > is not surprising as the messages from Asterisk arrive interleaved, which is > not something SIPp handles well (at all). > > > Thanks, > > Matt Jordan > >
-- _____________________________________________________________________ -- 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
