[asterisk-commits] mmichelson: trunk r400452 - in /trunk: ./ bridges/ include/asterisk/
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Thu Oct 3 15:22:22 CDT 2013
Author: mmichelson
Date: Thu Oct 3 15:22:17 2013
New Revision: 400452
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=400452
Log:
Fix assumption in bridge_native_rtp.c regarding number of participants in a bridge.
When a party leaves a bridge, there may be more participants in the bridge than expected.
As such, it is important not to make assumptions regarding the list of channels in a
bridge.
This change makes it so that when a party leaves a native RTP bridge, we unbridge it and
the party it was bridged with. Previously, the first and last channels in the list were
unbridged since it was assumed that these were the two channels that had been bridged. As
previously stated, a new party had been inserted into the bridge, so this logic did not
work properly.
(closes issue ASTERISK-22615)
reported by Matt Jordan
Review: https://reviewboard.asterisk.org/r/2899
........
Merged revisions 400403 from http://svn.asterisk.org/svn/asterisk/branches/12
Modified:
trunk/ (props changed)
trunk/bridges/bridge_native_rtp.c
trunk/include/asterisk/bridge_technology.h
Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-12-merged' - no diff available.
Modified: trunk/bridges/bridge_native_rtp.c
URL: http://svnview.digium.com/svn/asterisk/trunk/bridges/bridge_native_rtp.c?view=diff&rev=400452&r1=400451&r2=400452
==============================================================================
--- trunk/bridges/bridge_native_rtp.c (original)
+++ trunk/bridges/bridge_native_rtp.c Thu Oct 3 15:22:17 2013
@@ -183,10 +183,89 @@
return 0;
}
-static void native_rtp_bridge_stop(struct ast_bridge *bridge, struct ast_channel *target)
-{
- struct ast_bridge_channel *c0 = AST_LIST_FIRST(&bridge->channels);
- struct ast_bridge_channel *c1 = AST_LIST_LAST(&bridge->channels);
+/*!
+ * \internal
+ * \brief Given a bridge channel, get its RTP instance
+ *
+ * The returned ast_rtp_instance has its refcount bumped.
+ *
+ * \param bridge_channel Take a guess
+ * \retval NULL No RTP instance on this bridge channel
+ * \retval non-NULL The RTP instance on this bridge channel
+ */
+static struct ast_rtp_instance *bridge_channel_get_rtp_instance(struct ast_bridge_channel *bridge_channel)
+{
+ struct ast_rtp_glue *glue;
+ struct ast_rtp_instance *instance;
+
+ glue = ast_rtp_instance_get_glue(ast_channel_tech(bridge_channel->chan)->type);
+ if (!glue) {
+ return NULL;
+ }
+
+ glue->get_rtp_info(bridge_channel->chan, &instance);
+ return instance;
+}
+
+/*!
+ * \internal
+ * \brief Determine which two channels are bridged together
+ *
+ * Because of the nature of swapping, when the time comes for a channel to
+ * leave a native RTP bridge, it may be that there are more than two channels
+ * in the list of bridge channels. Therefore, it is important to correctly
+ * determine which two channels were bridged together.
+ *
+ * \param bridge The involved bridge
+ * \param leaving The bridge channel that is leaving the native RTP bridge
+ * \param[out] c0 The first bridged channel
+ * \param[out] c1 The second bridged channel
+ */
+static void find_bridged_channels(struct ast_bridge *bridge, struct ast_bridge_channel *leaving,
+ struct ast_bridge_channel **c0, struct ast_bridge_channel **c1)
+{
+ RAII_VAR(struct ast_rtp_instance *, leaving_instance, bridge_channel_get_rtp_instance(leaving), ao2_cleanup);
+ struct ast_bridge_channel *iter;
+
+ if (!leaving_instance) {
+ return;
+ }
+
+ AST_LIST_TRAVERSE(&bridge->channels, iter, entry) {
+ RAII_VAR(struct ast_rtp_instance *, instance, NULL, ao2_cleanup);
+
+ if (iter == leaving) {
+ continue;
+ }
+
+ instance = bridge_channel_get_rtp_instance(iter);
+ if (!instance) {
+ continue;
+ }
+
+ if (instance == ast_rtp_instance_get_bridged(leaving_instance)) {
+ break;
+ }
+ }
+ *c0 = leaving;
+ *c1 = iter;
+ return;
+}
+
+/*!
+ * \internal
+ * \brief Stop native RTP bridging of two channels
+ *
+ * \param bridge The bridge that had native RTP bridging happening on it
+ * \param target If remote RTP bridging, the channel that is placed on hold.
+ * \param leaving If this is called because a channel is leaving, this is the
+ * bridge channel that is leaving the bridge
+ */
+static void native_rtp_bridge_stop(struct ast_bridge *bridge, struct ast_channel *target,
+ struct ast_bridge_channel *leaving)
+{
+ struct ast_bridge_channel *c0 = NULL;
+ struct ast_bridge_channel *c1 = NULL;
enum ast_rtp_glue_result native_type;
struct ast_rtp_glue *glue0, *glue1 = NULL;
RAII_VAR(struct ast_rtp_instance *, instance0, NULL, ao2_cleanup);
@@ -194,7 +273,21 @@
RAII_VAR(struct ast_rtp_instance *, vinstance0, NULL, ao2_cleanup);
RAII_VAR(struct ast_rtp_instance *, vinstance1, NULL, ao2_cleanup);
- if (c0 == c1) {
+ if (bridge->num_channels == 2) {
+ c0 = AST_LIST_FIRST(&bridge->channels);
+ c1 = AST_LIST_LAST(&bridge->channels);
+ } else if (bridge->num_channels > 2) {
+ /* When a channel leaves a native RTP bridge, it is possible for
+ * more channels to exist in the bridge than when the RTP bridge
+ * was started. Thus we need to determine which two channels were
+ * bridged based on the leaving channel
+ */
+ find_bridged_channels(bridge, leaving, &c0, &c1);
+ } else {
+ return;
+ }
+
+ if (!c0 || !c1) {
return;
}
@@ -254,7 +347,7 @@
if (bridge) {
if (f->subclass.integer == AST_CONTROL_HOLD) {
- native_rtp_bridge_stop(bridge, chan);
+ native_rtp_bridge_stop(bridge, chan, NULL);
} else if ((f->subclass.integer == AST_CONTROL_UNHOLD) || (f->subclass.integer == AST_CONTROL_UPDATE_RTP_PEER)) {
native_rtp_bridge_start(bridge, chan);
}
@@ -420,7 +513,7 @@
{
native_rtp_bridge_framehook_detach(bridge_channel);
- native_rtp_bridge_stop(bridge, NULL);
+ native_rtp_bridge_stop(bridge, NULL, bridge_channel);
}
static int native_rtp_bridge_write(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
Modified: trunk/include/asterisk/bridge_technology.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/bridge_technology.h?view=diff&rev=400452&r1=400451&r2=400452
==============================================================================
--- trunk/include/asterisk/bridge_technology.h (original)
+++ trunk/include/asterisk/bridge_technology.h Thu Oct 3 15:22:17 2013
@@ -113,6 +113,10 @@
* \brief Remove a channel from a bridging technology instance for a bridge.
*
* \note On entry, bridge is already locked.
+ * \note Do not make assumptions about the number of channels in the bridge when
+ * this callback is called. When a channel is swapped into a bridge for another
+ * channel, the leave callback is called after the new channel has been added to
+ * the bridge.
*/
void (*leave)(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel);
/*!
More information about the asterisk-commits
mailing list