diff options
| -rw-r--r-- | bridges/bridge_softmix.c | 41 | ||||
| -rw-r--r-- | include/asterisk/bridge_channel.h | 25 | ||||
| -rw-r--r-- | include/asterisk/bridge_technology.h | 26 | ||||
| -rw-r--r-- | main/bridge_channel.c | 90 |
4 files changed, 140 insertions, 42 deletions
diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c index 2d71fc37c..59b16b78e 100644 --- a/bridges/bridge_softmix.c +++ b/bridges/bridge_softmix.c @@ -1686,7 +1686,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 @@ -1724,34 +1725,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. @@ -1773,10 +1792,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 4d3326083..4504d6bcb 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 8cebe9326..def7b1933 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 1427fd014..66292bf92 100644 --- a/main/bridge_channel.c +++ b/main/bridge_channel.c @@ -626,11 +626,11 @@ void ast_bridge_channel_kick(struct ast_bridge_channel *bridge_channel, int caus /*! * \internal - * \brief Write an \ref ast_frame onto the bridge channel + * \brief Write an \ref ast_frame into the bridge * \since 12.0.0 * - * \param bridge_channel Which channel to queue the frame onto. - * \param frame The frame to write onto the bridge_channel + * \param bridge_channel Which channel is queueing the frame. + * \param frame The frame to write into the bridge * * \retval 0 on success. * \retval -1 on error. @@ -638,19 +638,73 @@ void ast_bridge_channel_kick(struct ast_bridge_channel *bridge_channel, int caus static int bridge_channel_write_frame(struct ast_bridge_channel *bridge_channel, struct ast_frame *frame) { const struct ast_control_t38_parameters *t38_parameters; + int unmapped_stream_num; int deferred; ast_assert(frame->frametype != AST_FRAME_BRIDGE_ACTION_SYNC); ast_bridge_channel_lock_bridge(bridge_channel); + /* Map the frame to the bridge. */ + if (ast_channel_is_multistream(bridge_channel->chan)) { + unmapped_stream_num = frame->stream_num; + switch (frame->frametype) { + case AST_FRAME_VOICE: + case AST_FRAME_VIDEO: + case AST_FRAME_TEXT: + case AST_FRAME_IMAGE: + /* Media frames need to be mapped to an appropriate write stream */ + if (frame->stream_num < 0) { + /* Map to default stream */ + frame->stream_num = -1; + break; + } + ast_bridge_channel_lock(bridge_channel); + if (frame->stream_num < (int)AST_VECTOR_SIZE(&bridge_channel->stream_map.to_bridge)) { + frame->stream_num = AST_VECTOR_GET( + &bridge_channel->stream_map.to_bridge, frame->stream_num); + if (0 <= frame->stream_num) { + ast_bridge_channel_unlock(bridge_channel); + break; + } + } + ast_bridge_channel_unlock(bridge_channel); + ast_bridge_unlock(bridge_channel->bridge); + /* + * Ignore frame because we don't know how to map the frame + * or the bridge is not expecting any media from that + * stream. + */ + return 0; + case AST_FRAME_DTMF_BEGIN: + case AST_FRAME_DTMF_END: + /* + * XXX It makes sense that DTMF could be on any audio stream. + * For now we will only put it on the default audio stream. + */ + default: + frame->stream_num = -1; + break; + } + } else { + unmapped_stream_num = -1; + frame->stream_num = -1; + } + deferred = bridge_channel->bridge->technology->write(bridge_channel->bridge, bridge_channel, frame); if (deferred) { struct ast_frame *dup; dup = ast_frdup(frame); if (dup) { + /* + * We have to unmap the deferred frame so it comes back + * in like a new frame. + */ + dup->stream_num = unmapped_stream_num; + ast_bridge_channel_lock(bridge_channel); AST_LIST_INSERT_HEAD(&bridge_channel->deferred_queue, dup, frame_list); + ast_bridge_channel_unlock(bridge_channel); } } @@ -760,12 +814,14 @@ void bridge_channel_queue_deferred_frames(struct ast_bridge_channel *bridge_chan { struct ast_frame *frame; + ast_bridge_channel_lock(bridge_channel); ast_channel_lock(bridge_channel->chan); while ((frame = AST_LIST_REMOVE_HEAD(&bridge_channel->deferred_queue, frame_list))) { ast_queue_frame_head(bridge_channel->chan, frame); ast_frfree(frame); } ast_channel_unlock(bridge_channel->chan); + ast_bridge_channel_unlock(bridge_channel); } /*! @@ -2434,6 +2490,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, @@ -2457,13 +2514,8 @@ static void bridge_handle_trip(struct ast_bridge_channel *bridge_channel) return; } - if (ast_channel_is_multistream(bridge_channel->chan) && - (frame->frametype == AST_FRAME_IMAGE || frame->frametype == AST_FRAME_TEXT || - frame->frametype == AST_FRAME_VIDEO || frame->frametype == AST_FRAME_VOICE)) { - /* Media frames need to be mapped to an appropriate write stream */ - frame->stream_num = AST_VECTOR_GET( - &bridge_channel->stream_map.to_bridge, frame->stream_num); - } else { + if (!ast_channel_is_multistream(bridge_channel->chan)) { + /* This may not be initialized by non-multistream channel drivers */ frame->stream_num = -1; } @@ -2485,10 +2537,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 +2556,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 +3050,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); } |