diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c index b35994895e1854ec5ff15982fecb264ec6d094d1..bae0b8c7a2d65d772aeed18034381e6edd2aa8a1 100644 --- a/bridges/bridge_softmix.c +++ b/bridges/bridge_softmix.c @@ -1690,7 +1690,8 @@ static void map_source_to_destinations(const char *source_stream_name, const cha } } -/*\brief stream_topology_changed callback +/*! + * \brief stream_topology_changed callback * * For most video modes, nothing beyond the ordinary is required. * For the SFU case, though, we need to completely remap the streams @@ -1728,34 +1729,52 @@ static void softmix_bridge_stream_topology_changed(struct ast_bridge *bridge, st /* Second traversal: Map specific video channels from their source to their destinations. * - * This is similar to what is done in ast_stream_topology_map(), except that - * video channels are handled differently. Each video source has it's own - * unique index on the bridge. this way, a particular channel's source video - * can be distributed to the appropriate destination streams on the other - * channels + * This is similar to what is done in ast_stream_topology_map(), + * except that video channels are handled differently. Each video + * source has it's own unique index on the bridge. This way, a + * particular channel's source video can be distributed to the + * appropriate destination streams on the other channels. */ AST_LIST_TRAVERSE(&bridge->channels, participant, entry) { int i; struct ast_stream_topology *topology; + ast_bridge_channel_lock(participant); ast_channel_lock(participant->chan); topology = ast_channel_get_stream_topology(participant->chan); + if (topology) { + /* + * Sigh. We have to clone to avoid deadlock in + * map_source_to_destinations() because topology + * is not an ao2 object. + */ + topology = ast_stream_topology_clone(topology); + } + if (!topology) { + /* Oh, my, we are in trouble. */ + ast_channel_unlock(participant->chan); + ast_bridge_channel_unlock(participant); + continue; + } for (i = 0; i < ast_stream_topology_get_count(topology); ++i) { struct ast_stream *stream = ast_stream_topology_get_stream(topology, i); - ast_bridge_channel_lock(participant); + if (is_video_source(stream)) { AST_VECTOR_APPEND(&media_types, AST_MEDIA_TYPE_VIDEO); AST_VECTOR_REPLACE(&participant->stream_map.to_bridge, i, AST_VECTOR_SIZE(&media_types) - 1); AST_VECTOR_REPLACE(&participant->stream_map.to_channel, AST_VECTOR_SIZE(&media_types) - 1, -1); - /* Unlock the participant to prevent potential deadlock - * in map_source_to_destinations + /* + * Unlock the channel and participant to prevent + * potential deadlock in map_source_to_destinations(). */ + ast_channel_unlock(participant->chan); ast_bridge_channel_unlock(participant); map_source_to_destinations(ast_stream_get_name(stream), ast_channel_name(participant->chan), AST_VECTOR_SIZE(&media_types) - 1, &bridge->channels); ast_bridge_channel_lock(participant); + ast_channel_lock(participant->chan); } else if (is_video_dest(stream, NULL, NULL)) { /* We expect to never read media from video destination channels, but just * in case, we should set their to_bridge value to -1. @@ -1777,10 +1796,12 @@ static void softmix_bridge_stream_topology_changed(struct ast_bridge *bridge, st AST_VECTOR_REPLACE(&participant->stream_map.to_bridge, i, index); AST_VECTOR_REPLACE(&participant->stream_map.to_channel, index, i); } - ast_bridge_channel_unlock(participant); } + ast_stream_topology_free(topology); + ast_channel_unlock(participant->chan); + ast_bridge_channel_unlock(participant); } } diff --git a/include/asterisk/bridge_channel.h b/include/asterisk/bridge_channel.h index 4d3326083a6e113cf78dfb95fc48e4818c39c80b..4504d6bcb7da0fb3fffb6e80dc69244c97677aa4 100644 --- a/include/asterisk/bridge_channel.h +++ b/include/asterisk/bridge_channel.h @@ -716,19 +716,24 @@ void ast_bridge_channel_feature_digit(struct ast_bridge_channel *bridge_channel, * \brief Maps a channel's stream topology to and from the bridge * \since 15.0.0 * - * When a channel joins a bridge or its associated stream topology is updated, each stream - * in the topology needs to be mapped according to its media type to the bridge. Calling - * this method creates a mapping of each stream on the channel indexed to the bridge's - * supported media types and vice versa (i.e. bridge's media types indexed to channel - * streams). - * - * The first channel to join the bridge creates the initial order for the bridge's media - * types (e.g. a one to one mapping is made). Subsequently added channels are mapped to - * that order adding more media types if/when the newly added channel has more streams - * and/or media types specified by the bridge. + * \details + * When a channel joins a bridge or its associated stream topology is + * updated, each stream in the topology needs to be mapped according + * to its media type to the bridge. Calling this method creates a + * mapping of each stream on the channel indexed to the bridge's + * supported media types and vice versa (i.e. bridge's media types + * indexed to channel streams). + * + * The first channel to join the bridge creates the initial order for + * the bridge's media types (e.g. a one to one mapping is made). + * Subsequently added channels are mapped to that order adding more + * media types if/when the newly added channel has more streams and/or + * media types specified by the bridge. * * \param bridge_channel Channel to map * + * \note The bridge_channel's bridge must be locked prior to calling this function. + * * \return Nothing */ void ast_bridge_channel_stream_map(struct ast_bridge_channel *bridge_channel); diff --git a/include/asterisk/bridge_technology.h b/include/asterisk/bridge_technology.h index 8cebe93266feaa311677d2fc4954c17b4eca4a0f..def7b19335f8fd144a86ee00b46e99e6d07b3a62 100644 --- a/include/asterisk/bridge_technology.h +++ b/include/asterisk/bridge_technology.h @@ -164,20 +164,30 @@ struct ast_bridge_technology { /*! * \brief Callback for when a request has been made to change a stream topology on a channel * - * This is called when a bridge receives a request to change the topology on the channel. A bridge - * technology should define a handler for this callback if it needs to update internals or intercept - * the request and not pass it on to other channels. This can be done by returning a nonzero value. + * \details + * This is called when a bridge receives a request to change the + * topology on the channel. A bridge technology should define a + * handler for this callback if it needs to update internals or + * intercept the request and not pass it on to other channels. + * This can be done by returning a nonzero value. * - * \retval 0 Frame accepted by the bridge. - * \retval -1 Frame rejected. + * \retval 0 Frame can pass to the bridge technology. + * \retval non-zero Frame intercepted by the bridge technology. + * + * \note On entry, bridge is already locked. */ int (*stream_topology_request_change)(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel); /*! * \brief Callback for when a stream topology changes on the channel * - * This is called when a bridge receives an indication that a topology has been changed on a channel - * and the new topology has been mapped to the bridge. A bridge technology should define a handler for - * this callback if it needs to update internals due to a channel's topology changing. + * \details + * This is called when a bridge receives an indication that a + * topology has been changed on a channel and the new topology has + * been mapped to the bridge. A bridge technology should define a + * handler for this callback if it needs to update internals due + * to a channel's topology changing. + * + * \note On entry, bridge is already locked. */ void (*stream_topology_changed)(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel); /*! TRUE if the bridge technology is currently suspended. */ diff --git a/main/bridge_channel.c b/main/bridge_channel.c index 1427fd0141f0bc9d0326bd241110238fc512acee..c465ec16eb4a63664dcf99a2e8413d5d9f236313 100644 --- a/main/bridge_channel.c +++ b/main/bridge_channel.c @@ -2434,6 +2434,7 @@ static const char *controls[] = { static void bridge_handle_trip(struct ast_bridge_channel *bridge_channel) { struct ast_frame *frame; + int blocked; if (!ast_strlen_zero(ast_channel_call_forward(bridge_channel->chan))) { /* TODO If early bridging is ever used by anything other than ARI, @@ -2485,10 +2486,16 @@ static void bridge_handle_trip(struct ast_bridge_channel *bridge_channel) ast_channel_publish_dial(NULL, bridge_channel->chan, NULL, controls[frame->subclass.integer]); break; case AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE: - if (bridge_channel->bridge->technology->stream_topology_request_change && - bridge_channel->bridge->technology->stream_topology_request_change( - bridge_channel->bridge, bridge_channel)) { - /* Topology change was denied so drop frame */ + ast_bridge_channel_lock_bridge(bridge_channel); + blocked = bridge_channel->bridge->technology->stream_topology_request_change + && bridge_channel->bridge->technology->stream_topology_request_change( + bridge_channel->bridge, bridge_channel); + ast_bridge_unlock(bridge_channel->bridge); + if (blocked) { + /* + * Topology change was intercepted by the bridge technology + * so drop frame. + */ bridge_frame_free(frame); return; } @@ -2498,12 +2505,14 @@ static void bridge_handle_trip(struct ast_bridge_channel *bridge_channel) * If a stream topology has changed then the bridge_channel's * media mapping needs to be updated. */ + ast_bridge_channel_lock_bridge(bridge_channel); if (bridge_channel->bridge->technology->stream_topology_changed) { bridge_channel->bridge->technology->stream_topology_changed( bridge_channel->bridge, bridge_channel); } else { ast_bridge_channel_stream_map(bridge_channel); } + ast_bridge_unlock(bridge_channel->bridge); break; default: break; @@ -2990,9 +2999,11 @@ struct ast_bridge_channel *bridge_channel_internal_alloc(struct ast_bridge *brid void ast_bridge_channel_stream_map(struct ast_bridge_channel *bridge_channel) { + ast_bridge_channel_lock(bridge_channel); ast_channel_lock(bridge_channel->chan); ast_stream_topology_map(ast_channel_get_stream_topology(bridge_channel->chan), &bridge_channel->bridge->media_types, &bridge_channel->stream_map.to_bridge, &bridge_channel->stream_map.to_channel); ast_channel_unlock(bridge_channel->chan); + ast_bridge_channel_unlock(bridge_channel); }