> 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