> On Jan. 27, 2015, 9:57 a.m., rmudgett wrote:
> > /branches/13/main/bridge_channel.c, lines 1998-2001
> > <https://reviewboard.asterisk.org/r/4378/diff/1/?file=71097#file71097line1998>
> >
> >     The reason a swap channel is pulled after the new channel is pushed is 
> > because pulling the last channel out of a bridge may dissolve it!  This is 
> > not good.  If the AST_BRIDGE_FLAG_DISSOLVE_EMPTY flag is set the bridge 
> > will dissolve.  Most mixing bridges are going to have this flag set.
> >     
> >     The bridge technology must deal only with the channels that the bridge 
> > core has told it about with the technology join/leave callbacks.  
> > Otherwise, what is the point of the join/leave technology callbacks if the 
> > technology is going to look at the bridge channel member list at 
> > inappropriate times?
> 
> Joshua Colp wrote:
>     That flag can be unset and reset appropriately. The bridge is locked 
> during this operation.
>     
>     I firmly disagree with your statement about the bridge technology only 
> dealing with channels it has been told about. That means each bridge 
> technology has to DUPLICATE information that the core already has. And the 
> point of the join/leave callbacks is so the bridge technology can react to 
> the act of a channel joining or leaving a bridge. In case of 
> bridge_native_rtp that means setting up local bridging or doing reinvites.
> 
> Joshua Colp wrote:
>     Additionally: During testing there was always one other channel in the 
> bridge so the dissolve check never kicked in.
> 
> Matt Jordan wrote:
>     I'm going to agree with Josh on this one. The mixing technologies should 
> not be required to maintain their own state of who is in the bridge and who 
> is not. That feels like the domain of the bridging framework itself. It is 
> not an unreasonable assumption for the mixing technologies to expect that the 
> list of channels in the bridge is accurate during the various callbacks.
>     
>     It may be worthwhile to be a bit paranoid and do a check on the dissolve 
> flag and unset/reset it through this operation. I can't think of a situation 
> where we only have a single channel in a bridge and we do a swap on it, but I 
> suppose it could theoretically occur.

Rather than temporarily clearing and resetting the 
AST_BRIDGE_FLAG_DISSOLVE_EMPTY, how about deferring the clearing of 
bridge_channel->swap till after the swap pull and if the 
bridge->v_table->push() fails.  Then in bridge_channel_dissolve_check() not 
dissolve the bridge on the last channel leaving if there is a swap pointer.

like this:

bridge_channel_dissolve_check()
{
...
  if !bridge_channel->swap && AST_BRIDGE_FLAG_DISSOLVE_EMPTY && 
!bridge->num_channels
    /* Last channel leaving the bridge turns off the lights. */
    bridge_dissolve()
    return

...
}

bridge_channel_internal_push()
{
...
  if bridge->v_table->push fails
     ...
     bridge_channel->swap = NULL
     ...
     return -1

  if swap
    bridge_channel_internal_pull()
  bridge_channel->swap = NULL
...
}


- rmudgett


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


On Jan. 27, 2015, 10:34 a.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4378/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2015, 10:34 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Currently there exists two issues which prevent direct media from being 
> reinvited depending on the scenario:
> 
> 1. During a swap operation for a brief period of time there will exist 3 
> channels in a bridge. This is NOT handled by the bridge_native_rtp module and 
> causes it to not reinvite one of the channels that it should when it may be 
> leaving. As it's a reasonable expectation for a bridge technology which can 
> only handle 2 channels to only ever see 2 I've moved the operation which 
> causes the swap channel to leave to before the new channel is actually added 
> to the bridge. This means bridge_native_rtp only sees the two channels it saw 
> previously and reinvites occur as expected.
> 
> 2. If the res_pjsip_sdp_rtp module received a re-invite *AFTER* the session 
> had been established it did not notify upstream that things such as the 
> bridge_native_rtp module should re-evaluate and potentially reinvite the 
> remote side. The res_pjsip_sdp_rtp module will now do this using the 
> UPDATE_RTP_PEER control frame if an offer is received after the session is 
> established.
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/res_pjsip_sdp_rtp.c 431113 
>   /branches/13/main/bridge_channel.c 431113 
> 
> Diff: https://reviewboard.asterisk.org/r/4378/diff/
> 
> 
> Testing
> -------
> 
> Tried various scenarios including attended transfers and multiple Asterisk 
> instances in the path. Previously media would go via the wrong route or not 
> at all. With patch reinvites occur 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

Reply via email to