Merge changes from topic 'ASTERISK-27212'

* changes:
  bridge_channel.c: Fix FRACK when mapping frames to the bridge.
  bridge: Fix softmix bridge deadlock.
This commit is contained in:
Jenkins2
2017-08-23 14:45:52 -05:00
committed by Gerrit Code Review
4 changed files with 139 additions and 41 deletions

View File

@@ -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);
}
}

View File

@@ -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).
* \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.
* 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);

View File

@@ -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. */

View File

@@ -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);
}