> On April 9, 2014, 5:01 p.m., Matt Jordan wrote: > > /branches/12/channels/chan_sip.c, lines 7271-7299 > > <https://reviewboard.asterisk.org/r/3431/diff/1/?file=57177#file57177line7271> > > > > While this no longer worked - since we don't call sip_hangup until > > channels are well out of a bridge - I think it's worth pointing out what > > this was trying to do and why it's really not needed (even if it did work). > > > > The idea here was probably to set the statistics on the bridged channel > > such that whatever executed in the 'h' extension got the statistics from > > this RTP instance. Unfortunately, I'm not sure how that would have worked > > well in this case - ast_rtp_instance_set_stats_vars writes the channel > > variables for the RTCP statistics. If anything, this would have just > > overwritten the channel variables from one channel with another (most > > likely writing the bridged channel's statistics onto the original > > channel's). If both channels that were in the bridge still 'knew' of their > > bridged channel, they would have just swapped statistics. > > > > Since we have hangup handlers, the need for this logic is non-existent > > in the first place. > > Matt Jordan wrote: > Actually, I'm totally wrong here. I had to go back and look at the stats > function. It actually *does* write the variables onto both channels... oh > well. Things you realize after you hit the publish button.
Yes, the stats function does set stats on the channel and its bridged peer. In sip_hangup() there cannot be a bridge peer anymore. However, the stats were already set anyway if the channel was in a bridge because the BYE processing sets the stats on the channel and its peer. - rmudgett ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3431/#review11537 ----------------------------------------------------------- On April 9, 2014, 2:19 p.m., rmudgett wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3431/ > ----------------------------------------------------------- > > (Updated April 9, 2014, 2:19 p.m.) > > > Review request for Asterisk Developers. > > > Repository: Asterisk > > > Description > ------- > > The failing assertion ensures that the final snapshot gets generated so CDR > records can get finalized. The only place where a channel staging snapshot > flag could be left set is in the handle_request_bye(). The function could > return before clearing the flag because the channel could dissappear while > the function had to have the channel unlocked. > > * Fixed handle_request_bye() channel snapshot staging coverage area to not > have a return in the middle of it and be unable to clear the staging flag. > > * Pushed the channel snapshot staging coverage area into > ast_rtp_instance_set_stats_vars() to ensure that the staging is not > interrutped. > > * Made callers of ast_rtp_instance_set_stats_vars() not call it with channels > or channel driver private locks held to eliminate the deadlock potential. > The callers must hold references to the passed in channel and rtp objects. > > * Eliminated sip_hangup() trying to get the bridge peer. It is futile at > this point because the channel could never be in a bridge. > > * Moved sip_pvt unref in ast_hangup() and handle_request_do() to the end of > the function. The unref needs to happen after the last use of the pointer. > > > Diffs > ----- > > /branches/12/main/rtp_engine.c 412047 > /branches/12/channels/chan_sip.c 412047 > > Diff: https://reviewboard.asterisk.org/r/3431/diff/ > > > Testing > ------- > > I was unsuccessful in reproducing the testsuite channel staging assertion > failure. > However, SIP calls can still setup and teardown with the patch installed. > > > Thanks, > > rmudgett > >
-- _____________________________________________________________________ -- 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
