bridge: Fix stream topology/participant locking and video misrouting.

This change fixes a few locking issues and some video misrouting.

1. When accessing the stream topology of a channel the channel lock
must be held to guarantee the topology remains valid.

2. When a channel was joined to a bridge the bridge specific
implementation for stream mapping was not invoked, causing video
to be misrouted for a brief period of time.

ASTERISK-27182

Change-Id: I5d2f779248b84d41c5bb3896bf22ba324b336b03

diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index c5428a854516c562014257d094a95540aed57906..b35994895e1854ec5ff15982fecb264ec6d094d1 100644 (file)
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -577,13 +577,18 @@ static void sfu_topologies_on_join(struct ast_bridge_channel *joiner, struct ast
        struct ast_stream_topology *joiner_video = NULL;
        struct ast_stream_topology *existing_video = NULL;
        struct ast_bridge_channel *participant;
+       int res;
 
        joiner_video = ast_stream_topology_alloc();
        if (!joiner_video) {
                return;
        }
 
-       if (append_source_streams(joiner_video, ast_channel_name(joiner->chan), ast_channel_get_stream_topology(joiner->chan))) {
+       ast_channel_lock(joiner->chan);
+       res = append_source_streams(joiner_video, ast_channel_name(joiner->chan), ast_channel_get_stream_topology(joiner->chan));
+       ast_channel_unlock(joiner->chan);
+
+       if (res) {
                goto cleanup;
        }
 
@@ -596,13 +601,18 @@ static void sfu_topologies_on_join(struct ast_bridge_channel *joiner, struct ast
                if (participant == joiner) {
                        continue;
                }
-               if (append_source_streams(existing_video, ast_channel_name(participant->chan),
-                               ast_channel_get_stream_topology(participant->chan))) {
+               ast_channel_lock(participant->chan);
+               res = append_source_streams(existing_video, ast_channel_name(participant->chan),
+                               ast_channel_get_stream_topology(participant->chan));
+               ast_channel_unlock(participant->chan);
+               if (res) {
                        goto cleanup;
                }
        }
 
+       ast_channel_lock(joiner->chan);
        joiner_topology = ast_stream_topology_clone(ast_channel_get_stream_topology(joiner->chan));
+       ast_channel_unlock(joiner->chan);
        if (!joiner_topology) {
                goto cleanup;
        }
@@ -617,7 +627,9 @@ static void sfu_topologies_on_join(struct ast_bridge_channel *joiner, struct ast
                if (participant == joiner) {
                        continue;
                }
+               ast_channel_lock(participant->chan);
                participant_topology = ast_stream_topology_clone(ast_channel_get_stream_topology(participant->chan));
+               ast_channel_unlock(participant->chan);
                if (!participant_topology) {
                        goto cleanup;
                }
@@ -753,12 +765,16 @@ static int sfu_topologies_on_leave(struct ast_bridge_channel *leaver, struct ast
                        continue;
                }
 
+               ast_channel_lock(participant->chan);
                remove_destination_streams(participant_topology, ast_channel_name(leaver->chan), ast_channel_get_stream_topology(participant->chan));
+               ast_channel_unlock(participant->chan);
                ast_channel_request_stream_topology_change(participant->chan, participant_topology, NULL);
                ast_stream_topology_free(participant_topology);
        }
 
+       ast_channel_lock(leaver->chan);
        remove_destination_streams(leaver_topology, "", ast_channel_get_stream_topology(leaver->chan));
+       ast_channel_unlock(leaver->chan);
        ast_channel_request_stream_topology_change(leaver->chan, leaver_topology, NULL);
        ast_stream_topology_free(leaver_topology);
 
@@ -1657,6 +1673,7 @@ static void map_source_to_destinations(const char *source_stream_name, const cha
                }
 
                ast_bridge_channel_lock(participant);
+               ast_channel_lock(participant->chan);
                topology = ast_channel_get_stream_topology(participant->chan);
 
                for (i = 0; i < ast_stream_topology_get_count(topology); ++i) {
@@ -1668,6 +1685,7 @@ static void map_source_to_destinations(const char *source_stream_name, const cha
                                break;
                        }
                }
+               ast_channel_unlock(participant->chan);
                ast_bridge_channel_unlock(participant);
        }
 }
@@ -1702,16 +1720,9 @@ static void softmix_bridge_stream_topology_changed(struct ast_bridge *bridge, st
 
        /* First traversal: re-initialize all of the participants' stream maps */
        AST_LIST_TRAVERSE(&bridge->channels, participant, entry) {
-               int size;
-
                ast_bridge_channel_lock(participant);
-               size = ast_stream_topology_get_count(ast_channel_get_stream_topology(participant->chan));
-
-               AST_VECTOR_FREE(&participant->stream_map.to_channel);
-               AST_VECTOR_FREE(&participant->stream_map.to_bridge);
-
-               AST_VECTOR_INIT(&participant->stream_map.to_channel, size);
-               AST_VECTOR_INIT(&participant->stream_map.to_bridge, size);
+               AST_VECTOR_RESET(&participant->stream_map.to_channel, AST_VECTOR_ELEM_CLEANUP_NOOP);
+               AST_VECTOR_RESET(&participant->stream_map.to_bridge, AST_VECTOR_ELEM_CLEANUP_NOOP);
                ast_bridge_channel_unlock(participant);
        }
 
@@ -1727,6 +1738,8 @@ static void softmix_bridge_stream_topology_changed(struct ast_bridge *bridge, st
                int i;
                struct ast_stream_topology *topology;
 
+               ast_channel_lock(participant->chan);
+
                topology = ast_channel_get_stream_topology(participant->chan);
 
                for (i = 0; i < ast_stream_topology_get_count(topology); ++i) {
@@ -1766,6 +1779,8 @@ static void softmix_bridge_stream_topology_changed(struct ast_bridge *bridge, st
                        }
                        ast_bridge_channel_unlock(participant);
                }
+
+               ast_channel_unlock(participant->chan);
        }
 }
 

diff --git a/main/bridge.c b/main/bridge.c
index a1a1a6f5599a664ae7505de44da6da6dec3e55ac..b732d5fc55c76b3841b27a0f03a34a1471e1376c 100644 (file)
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -446,7 +446,12 @@ static void bridge_channel_complete_join(struct ast_bridge *bridge, struct ast_b
         * media types vector. This way all streams map to the same media type index for
         * a given channel.
         */
-       ast_bridge_channel_stream_map(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);
+       }
 }
 
 /*!

diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 2e943000cc1001a2ac0ad3e64fc63aa71cccb9f0..1427fd0141f0bc9d0326bd241110238fc512acee 100644 (file)
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -2344,16 +2344,23 @@ static void bridge_channel_handle_write(struct ast_bridge_channel *bridge_channe
        case AST_FRAME_NULL:
                break;
        default:
-               if (fr->stream_num > 0 &&
-                               (fr->stream_num >= (int)AST_VECTOR_SIZE(&bridge_channel->stream_map.to_channel) ||
-                               AST_VECTOR_GET(&bridge_channel->stream_map.to_channel, fr->stream_num) == -1)) {
-                       /* Nowhere to write to, so drop it */
-                       break;
-               }
+               /* Assume that there is no mapped stream for this */
+               num = -1;
 
-               /* Find what stream number to write to for the channel */
-               num = fr->stream_num < 0 ? -1 :
-                       AST_VECTOR_GET(&bridge_channel->stream_map.to_channel, fr->stream_num);
+               if (fr->stream_num > -1) {
+                       ast_bridge_channel_lock(bridge_channel);
+                       if (fr->stream_num < (int)AST_VECTOR_SIZE(&bridge_channel->stream_map.to_channel)) {
+                               num = AST_VECTOR_GET(&bridge_channel->stream_map.to_channel, fr->stream_num);
+                       }
+                       ast_bridge_channel_unlock(bridge_channel);
+
+                       /* If there is no mapped stream after checking the mapping then there is nowhere
+                        * to write this frame to, so drop it.
+                        */
+                       if (num == -1) {
+                               break;
+                       }
+               }
 
                /* Write the frame to the channel. */
                bridge_channel->activity = BRIDGE_CHANNEL_THREAD_SIMPLE;
@@ -2983,7 +2990,9 @@ 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_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);
 }