bridge_softmix: Note why ast_stream_topology_set_stream cannot fail.
[asterisk/asterisk.git] / bridges / bridge_softmix.c
index 3dd2660..8de88f2 100644 (file)
@@ -79,7 +79,7 @@ struct softmix_stats {
 
 struct softmix_translate_helper_entry {
        int num_times_requested; /*!< Once this entry is no longer requested, free the trans_pvt
-                                     and re-init if it was usable. */
+                                                                 and re-init if it was usable. */
        struct ast_format *dst_format; /*!< The destination format for this helper */
        struct ast_trans_pvt *trans_pvt; /*!< the translator for this slot. */
        struct ast_frame *out_frame; /*!< The output frame from the last translation */
@@ -493,21 +493,20 @@ static int append_source_streams(struct ast_stream_topology *dest,
        for (i = 0; i < ast_stream_topology_get_count(source); ++i) {
                struct ast_stream *stream;
                struct ast_stream *stream_clone;
-               char *stream_clone_name;
-               size_t stream_clone_name_len;
+               char *stream_clone_name = NULL;
 
                stream = ast_stream_topology_get_stream(source, i);
                if (!is_video_source(stream)) {
                        continue;
                }
 
-               /* The +3 is for the two underscore separators and null terminator */
-               stream_clone_name_len = SOFTBRIDGE_VIDEO_DEST_LEN + strlen(channel_name) + strlen(ast_stream_get_name(stream)) + 3;
-               stream_clone_name = ast_alloca(stream_clone_name_len);
-               snprintf(stream_clone_name, stream_clone_name_len, "%s_%s_%s", SOFTBRIDGE_VIDEO_DEST_PREFIX,
-                       channel_name, ast_stream_get_name(stream));
+               if (ast_asprintf(&stream_clone_name, "%s_%s_%s", SOFTBRIDGE_VIDEO_DEST_PREFIX,
+                       channel_name, ast_stream_get_name(stream)) < 0) {
+                       return -1;
+               }
 
                stream_clone = ast_stream_clone(stream, stream_clone_name);
+               ast_free(stream_clone_name);
                if (!stream_clone) {
                        return -1;
                }
@@ -524,15 +523,34 @@ static int append_all_streams(struct ast_stream_topology *dest,
        const struct ast_stream_topology *source)
 {
        int i;
+       int dest_index = 0;
 
        for (i = 0; i < ast_stream_topology_get_count(source); ++i) {
                struct ast_stream *clone;
+               int added = 0;
 
                clone = ast_stream_clone(ast_stream_topology_get_stream(source, i), NULL);
                if (!clone) {
                        return -1;
                }
-               if (ast_stream_topology_append_stream(dest, clone) < 0) {
+
+               /* If we can reuse an existing removed stream then do so */
+               while (dest_index < ast_stream_topology_get_count(dest)) {
+                       struct ast_stream *stream = ast_stream_topology_get_stream(dest, dest_index);
+
+                       dest_index++;
+
+                       if (ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {
+                               /* This cannot fail because dest_index - 1 is less than the
+                                * current count in dest. */
+                               ast_stream_topology_set_stream(dest, dest_index - 1, clone);
+                               added = 1;
+                               break;
+                       }
+               }
+
+               /* If no removed stream exists that we took the place of append the stream */
+               if (!added && ast_stream_topology_append_stream(dest, clone) < 0) {
                        ast_stream_free(clone);
                        return -1;
                }
@@ -556,22 +574,24 @@ static int append_all_streams(struct ast_stream_topology *dest,
  */
 static void sfu_topologies_on_join(struct ast_bridge_channel *joiner, struct ast_bridge_channels_list *participants)
 {
-       struct ast_stream_topology *joiner_topology = NULL;
        struct ast_stream_topology *joiner_video = NULL;
-       struct ast_stream_topology *existing_video = NULL;
        struct ast_bridge_channel *participant;
+       int res;
+       struct softmix_channel *sc;
 
        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))) {
-               goto cleanup;
-       }
+       sc = joiner->tech_pvt;
 
-       existing_video = ast_stream_topology_alloc();
-       if (!existing_video) {
+       ast_channel_lock(joiner->chan);
+       res = append_source_streams(joiner_video, ast_channel_name(joiner->chan), ast_channel_get_stream_topology(joiner->chan));
+       sc->topology = ast_stream_topology_clone(ast_channel_get_stream_topology(joiner->chan));
+       ast_channel_unlock(joiner->chan);
+
+       if (res || !sc->topology) {
                goto cleanup;
        }
 
@@ -579,43 +599,31 @@ 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(sc->topology, ast_channel_name(participant->chan),
+                               ast_channel_get_stream_topology(participant->chan));
+               ast_channel_unlock(participant->chan);
+               if (res) {
                        goto cleanup;
                }
        }
 
-       joiner_topology = ast_stream_topology_clone(ast_channel_get_stream_topology(joiner->chan));
-       if (!joiner_topology) {
-               goto cleanup;
-       }
-       if (append_all_streams(joiner_topology, existing_video)) {
-               goto cleanup;
-       }
-       ast_channel_request_stream_topology_change(joiner->chan, joiner_topology, NULL);
+       ast_channel_request_stream_topology_change(joiner->chan, sc->topology, NULL);
 
        AST_LIST_TRAVERSE(participants, participant, entry) {
-               struct ast_stream_topology *participant_topology;
-
                if (participant == joiner) {
                        continue;
                }
-               participant_topology = ast_stream_topology_clone(ast_channel_get_stream_topology(participant->chan));
-               if (!participant_topology) {
-                       goto cleanup;
-               }
-               if (append_all_streams(participant_topology, joiner_video)) {
-                       ast_stream_topology_free(participant_topology);
+
+               sc = participant->tech_pvt;
+               if (append_all_streams(sc->topology, joiner_video)) {
                        goto cleanup;
                }
-               ast_channel_request_stream_topology_change(participant->chan, participant_topology, NULL);
-               ast_stream_topology_free(participant_topology);
+               ast_channel_request_stream_topology_change(participant->chan, sc->topology, NULL);
        }
 
 cleanup:
        ast_stream_topology_free(joiner_video);
-       ast_stream_topology_free(existing_video);
-       ast_stream_topology_free(joiner_topology);
 }
 
 /*! \brief Function called when a channel is joined into the bridge */
@@ -689,61 +697,36 @@ static int softmix_bridge_join(struct ast_bridge *bridge, struct ast_bridge_chan
        return 0;
 }
 
-static int remove_destination_streams(struct ast_stream_topology *dest,
-       const char *channel_name,
-       const struct ast_stream_topology *source)
+static void remove_destination_streams(struct ast_stream_topology *topology,
+       const char *channel_name)
 {
        int i;
 
-       for (i = 0; i < ast_stream_topology_get_count(source); ++i) {
+       for (i = 0; i < ast_stream_topology_get_count(topology); ++i) {
                struct ast_stream *stream;
-               struct ast_stream *stream_clone;
-
-               stream = ast_stream_topology_get_stream(source, i);
 
-               stream_clone = ast_stream_clone(stream, NULL);
-               if (!stream_clone) {
-                       continue;
-               }
+               stream = ast_stream_topology_get_stream(topology, i);
 
                if (is_video_dest(stream, channel_name, NULL)) {
-                       ast_stream_set_state(stream_clone, AST_STREAM_STATE_REMOVED);
-               }
-
-               if (ast_stream_topology_append_stream(dest, stream_clone) < 0) {
-                       ast_stream_free(stream_clone);
+                       ast_stream_set_state(stream, AST_STREAM_STATE_REMOVED);
                }
        }
-
-       return 0;
 }
 
 static int sfu_topologies_on_leave(struct ast_bridge_channel *leaver, struct ast_bridge_channels_list *participants)
 {
-       struct ast_stream_topology *leaver_topology;
        struct ast_bridge_channel *participant;
-
-       leaver_topology = ast_stream_topology_alloc();
-       if (!leaver_topology) {
-               return -1;
-       }
+       struct softmix_channel *sc;
 
        AST_LIST_TRAVERSE(participants, participant, entry) {
-               struct ast_stream_topology *participant_topology;
-
-               participant_topology = ast_stream_topology_alloc();
-               if (!participant_topology) {
-                       continue;
-               }
-
-               remove_destination_streams(participant_topology, ast_channel_name(leaver->chan), ast_channel_get_stream_topology(participant->chan));
-               ast_channel_request_stream_topology_change(participant->chan, participant_topology, NULL);
-               ast_stream_topology_free(participant_topology);
+               sc = participant->tech_pvt;
+               remove_destination_streams(sc->topology, ast_channel_name(leaver->chan));
+               ast_channel_request_stream_topology_change(participant->chan, sc->topology, NULL);
        }
 
-       remove_destination_streams(leaver_topology, "", ast_channel_get_stream_topology(leaver->chan));
-       ast_channel_request_stream_topology_change(leaver->chan, leaver_topology, NULL);
-       ast_stream_topology_free(leaver_topology);
+       sc = leaver->tech_pvt;
+       remove_destination_streams(sc->topology, "");
+       ast_channel_request_stream_topology_change(leaver->chan, sc->topology, NULL);
 
        return 0;
 }
@@ -753,17 +736,17 @@ static void softmix_bridge_leave(struct ast_bridge *bridge, struct ast_bridge_ch
 {
        struct softmix_channel *sc;
        struct softmix_bridge_data *softmix_data;
+
        softmix_data = bridge->tech_pvt;
        sc = bridge_channel->tech_pvt;
+       if (!sc) {
+               return;
+       }
 
        if (bridge->softmix.video_mode.mode == AST_BRIDGE_VIDEO_MODE_SFU) {
                sfu_topologies_on_leave(bridge_channel, &bridge->channels);
        }
 
-       if (!sc) {
-               return;
-       }
-
        if (bridge->softmix.binaural_active) {
                if (sc->binaural) {
                        set_binaural_data_leave(&softmix_data->convolve, sc->binaural_pos,
@@ -773,6 +756,8 @@ static void softmix_bridge_leave(struct ast_bridge *bridge, struct ast_bridge_ch
 
        bridge_channel->tech_pvt = NULL;
 
+       ast_stream_topology_free(sc->topology);
+
        /* Drop mutex lock */
        ast_mutex_destroy(&sc->lock);
 
@@ -954,6 +939,111 @@ static void softmix_bridge_write_voice(struct ast_bridge *bridge, struct ast_bri
        }
 }
 
+static int remove_all_original_streams(struct ast_stream_topology *dest,
+       const struct ast_stream_topology *source,
+       const struct ast_stream_topology *original)
+{
+       int i;
+
+       for (i = 0; i < ast_stream_topology_get_count(source); ++i) {
+               struct ast_stream *stream;
+               int original_index;
+
+               stream = ast_stream_topology_get_stream(source, i);
+
+               /* Mark the existing stream as removed so we get a new one, this will get
+                * reused on a subsequent renegotiation.
+                */
+               for (original_index = 0; original_index < ast_stream_topology_get_count(original); ++original_index) {
+                       struct ast_stream *original_stream = ast_stream_topology_get_stream(original, original_index);
+
+                       if (!strcmp(ast_stream_get_name(stream), ast_stream_get_name(original_stream))) {
+                               struct ast_stream *removed;
+
+                               /* Since the participant is still going to be in the bridge we
+                                * change the name so that routing does not attempt to route video
+                                * to this stream.
+                                */
+                               removed = ast_stream_clone(stream, "removed");
+                               if (!removed) {
+                                       return -1;
+                               }
+
+                               ast_stream_set_state(removed, AST_STREAM_STATE_REMOVED);
+
+                               /* The destination topology can only ever contain the same, or more,
+                                * streams than the original so this is safe.
+                                */
+                               if (ast_stream_topology_set_stream(dest, original_index, removed)) {
+                                       ast_stream_free(removed);
+                                       return -1;
+                               }
+
+                               break;
+                       }
+               }
+       }
+
+       return 0;
+}
+
+static void sfu_topologies_on_source_change(struct ast_bridge_channel *source, struct ast_bridge_channels_list *participants)
+{
+       struct ast_stream_topology *source_video = NULL;
+       struct ast_bridge_channel *participant;
+       int res;
+
+       source_video = ast_stream_topology_alloc();
+       if (!source_video) {
+               return;
+       }
+
+       ast_channel_lock(source->chan);
+       res = append_source_streams(source_video, ast_channel_name(source->chan), ast_channel_get_stream_topology(source->chan));
+       ast_channel_unlock(source->chan);
+       if (res) {
+               goto cleanup;
+       }
+
+       AST_LIST_TRAVERSE(participants, participant, entry) {
+               struct ast_stream_topology *original_topology;
+               struct softmix_channel *sc;
+
+               if (participant == source) {
+                       continue;
+               }
+
+               sc = participant->tech_pvt;
+
+               original_topology = ast_stream_topology_clone(sc->topology);
+               if (!original_topology) {
+                       goto cleanup;
+               }
+
+               /* We add all the source streams back in, if any removed streams are already present they will
+                * get used first followed by appending new ones.
+                */
+               if (append_all_streams(sc->topology, source_video)) {
+                       ast_stream_topology_free(original_topology);
+                       goto cleanup;
+               }
+
+               /* And the original existing streams get marked as removed. This causes the remote side to see
+                * a new stream for the source streams.
+                */
+               if (remove_all_original_streams(sc->topology, source_video, original_topology)) {
+                       ast_stream_topology_free(original_topology);
+                       goto cleanup;
+               }
+
+               ast_channel_request_stream_topology_change(participant->chan, sc->topology, NULL);
+               ast_stream_topology_free(original_topology);
+       }
+
+cleanup:
+       ast_stream_topology_free(source_video);
+}
+
 /*!
  * \internal
  * \brief Determine what to do with a control frame.
@@ -968,6 +1058,8 @@ static void softmix_bridge_write_voice(struct ast_bridge *bridge, struct ast_bri
  */
 static int softmix_bridge_write_control(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
 {
+       struct softmix_bridge_data *softmix_data = bridge->tech_pvt;
+
        /*
         * XXX Softmix needs to use channel roles to determine what to
         * do with control frames.
@@ -975,7 +1067,16 @@ static int softmix_bridge_write_control(struct ast_bridge *bridge, struct ast_br
 
        switch (frame->subclass.integer) {
        case AST_CONTROL_VIDUPDATE:
-               ast_bridge_queue_everyone_else(bridge, NULL, frame);
+               if (!bridge->softmix.video_mode.video_update_discard ||
+                       ast_tvdiff_ms(ast_tvnow(), softmix_data->last_video_update) > bridge->softmix.video_mode.video_update_discard) {
+                       ast_bridge_queue_everyone_else(bridge, NULL, frame);
+                       softmix_data->last_video_update = ast_tvnow();
+               }
+               break;
+       case AST_CONTROL_STREAM_TOPOLOGY_SOURCE_CHANGED:
+               if (bridge->softmix.video_mode.mode == AST_BRIDGE_VIDEO_MODE_SFU) {
+                       sfu_topologies_on_source_change(bridge_channel, &bridge->channels);
+               }
                break;
        default:
                break;
@@ -1021,14 +1122,10 @@ static int softmix_bridge_write(struct ast_bridge *bridge, struct ast_bridge_cha
                res = ast_bridge_queue_everyone_else(bridge, bridge_channel, frame);
                break;
        case AST_FRAME_VOICE:
-               if (bridge_channel) {
-                       softmix_bridge_write_voice(bridge, bridge_channel, frame);
-               }
+               softmix_bridge_write_voice(bridge, bridge_channel, frame);
                break;
        case AST_FRAME_VIDEO:
-               if (bridge_channel) {
-                       softmix_bridge_write_video(bridge, bridge_channel, frame);
-               }
+               softmix_bridge_write_video(bridge, bridge_channel, frame);
                break;
        case AST_FRAME_CONTROL:
                res = softmix_bridge_write_control(bridge, bridge_channel, frame);
@@ -1634,6 +1731,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) {
@@ -1645,11 +1743,13 @@ static void map_source_to_destinations(const char *source_stream_name, const cha
                                break;
                        }
                }
+               ast_channel_unlock(participant->chan);
                ast_bridge_channel_unlock(participant);
        }
 }
 
-/*\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
@@ -1679,47 +1779,60 @@ 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);
        }
 
        /* 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.
@@ -1741,8 +1854,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);
        }
 }
 
@@ -1970,7 +2087,6 @@ AST_TEST_DEFINE(sfu_remove_destination_streams)
                { "", 4, { 0, 1, 2, 3 }, },
        };
        struct ast_stream_topology *orig = NULL;
-       struct ast_stream_topology *result = NULL;
        int i;
 
        switch (cmd) {
@@ -1994,20 +2110,11 @@ AST_TEST_DEFINE(sfu_remove_destination_streams)
        for (i = 0; i < ARRAY_LEN(removal_results); ++i) {
                int j;
 
-               result = ast_stream_topology_alloc();
-               if (!result) {
-                       ast_test_status_update(test, "Unable to allocate result stream topology\n");
-                       goto end;
-               }
-
-               if (remove_destination_streams(result, removal_results[i].channel_name, orig)) {
-                       ast_test_status_update(test, "Failure while attempting to remove video streams\n");
-                       goto end;
-               }
+               remove_destination_streams(orig, removal_results[i].channel_name);
 
-               if (ast_stream_topology_get_count(result) != removal_results[i].num_streams) {
+               if (ast_stream_topology_get_count(orig) != removal_results[i].num_streams) {
                        ast_test_status_update(test, "Resulting topology has %d streams, when %d are expected\n",
-                               ast_stream_topology_get_count(result), removal_results[i].num_streams);
+                               ast_stream_topology_get_count(orig), removal_results[i].num_streams);
                        goto end;
                }
 
@@ -2016,7 +2123,7 @@ AST_TEST_DEFINE(sfu_remove_destination_streams)
                        struct ast_stream *expected;
                        int orig_index;
 
-                       actual = ast_stream_topology_get_stream(result, j);
+                       actual = ast_stream_topology_get_stream(orig, j);
 
                        orig_index = removal_results[i].params_index[j];
                        expected = ast_stream_topology_get_stream(orig, orig_index);
@@ -2047,7 +2154,6 @@ AST_TEST_DEFINE(sfu_remove_destination_streams)
 
 end:
        ast_stream_topology_free(orig);
-       ast_stream_topology_free(result);
        return res;
 }