app_confbridge: Add "all" variants of REMB behavior.

When producing a combined REMB value the normal behavior
is to have a REMB value which is unique for each sender
based on all of their receivers. This can result in one
sender having low bitrate while all the rest are high.

This change adds "all" variants which produces a bridge
level REMB value instead. All REMB reports are combined
together into a single REMB value that is the same for
each sender.

ASTERISK-28401

Change-Id: I883e6cc26003b497c8180b346111c79a131ba88c

diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index 33cfbb3e597c0d0159ce490be42667d310969ec4..6d141bd4e1f6b947748665b9b3cd63e7d440a8a2 100644 (file)
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -1565,6 +1565,12 @@ static struct confbridge_conference *join_conference_bridge(const char *conferen
                                ast_brige_set_remb_behavior(conference->bridge, AST_BRIDGE_VIDEO_SFU_REMB_LOWEST);
                        } else if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST)) {
                                ast_brige_set_remb_behavior(conference->bridge, AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST);
+                       } else if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE_ALL)) {
+                               ast_brige_set_remb_behavior(conference->bridge, AST_BRIDGE_VIDEO_SFU_REMB_AVERAGE_ALL);
+                       } else if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_REMB_BEHAVIOR_LOWEST_ALL)) {
+                               ast_brige_set_remb_behavior(conference->bridge, AST_BRIDGE_VIDEO_SFU_REMB_LOWEST_ALL);
+                       } else if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST_ALL)) {
+                               ast_brige_set_remb_behavior(conference->bridge, AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST_ALL);
                        }
                }
 

diff --git a/apps/confbridge/conf_config_parser.c b/apps/confbridge/conf_config_parser.c
index 53061ba60f05b3bf1ed59f3d5ff19046c86f6ddf..f732c45402085756c3e964c69d1872815773ed3e 100644 (file)
--- a/apps/confbridge/conf_config_parser.c
+++ b/apps/confbridge/conf_config_parser.c
@@ -505,6 +505,18 @@
                                                        <enum name="highest">
                                                                <para>The highest estimated maximum bitrate is forwarded to the sender.</para>
                                                        </enum>
+                                                       <enum name="average_all">
+                                                               <para>The average of all estimated maximum bitrates is taken from all
+                                                               receivers in the bridge and a single value is sent to each sender.</para>
+                                                       </enum>
+                                                       <enum name="lowest_all">
+                                                               <para>The lowest estimated maximum bitrate of all receivers in the bridge
+                                                               is taken and sent to each sender.</para>
+                                                       </enum>
+                                                       <enum name="highest_all">
+                                                               <para>The highest estimated maximum bitrate of all receivers in the bridge
+                                                               is taken and sent to each sender.</para>
+                                                       </enum>
                                                </enumlist>
                                        </description>
                                </configOption>
@@ -1737,7 +1749,8 @@ static char *handle_cli_confbridge_show_bridge_profile(struct ast_cli_entry *e,
 
        switch (b_profile.flags
                & (BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE | BRIDGE_OPT_REMB_BEHAVIOR_LOWEST
-                       | BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST)) {
+                       | BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST | BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE_ALL
+                       | BRIDGE_OPT_REMB_BEHAVIOR_LOWEST_ALL | BRIDGE_OPT_REMB_BEHAVIOR_LOWEST_ALL)) {
        case BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE:
                ast_cli(a->fd, "REMB Behavior:           average\n");
                break;
@@ -1747,6 +1760,15 @@ static char *handle_cli_confbridge_show_bridge_profile(struct ast_cli_entry *e,
        case BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST:
                ast_cli(a->fd, "REMB Behavior:           highest\n");
                break;
+       case BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE_ALL:
+               ast_cli(a->fd, "REMB Behavior:           average_all\n");
+               break;
+       case BRIDGE_OPT_REMB_BEHAVIOR_LOWEST_ALL:
+               ast_cli(a->fd, "REMB Behavior:           lowest_all\n");
+               break;
+       case BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST_ALL:
+               ast_cli(a->fd, "REMB Behavior:           highest_all\n");
+               break;
        default:
                ast_assert(0);
                break;
@@ -2108,7 +2130,10 @@ static int remb_behavior_handler(const struct aco_option *opt, struct ast_variab
 
        ast_clear_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE |
                BRIDGE_OPT_REMB_BEHAVIOR_LOWEST |
-               BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST);
+               BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST |
+               BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE_ALL |
+               BRIDGE_OPT_REMB_BEHAVIOR_LOWEST_ALL |
+               BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST_ALL);
 
        if (!strcasecmp(var->value, "average")) {
                ast_set_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE);
@@ -2116,6 +2141,12 @@ static int remb_behavior_handler(const struct aco_option *opt, struct ast_variab
                ast_set_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_LOWEST);
        } else if (!strcasecmp(var->value, "highest")) {
                ast_set_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST);
+       } else if (!strcasecmp(var->value, "average_all")) {
+               ast_set_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE_ALL);
+       } else if (!strcasecmp(var->value, "lowest_all")) {
+               ast_set_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_LOWEST_ALL);
+       } else if (!strcasecmp(var->value, "highest_all")) {
+               ast_set_flag(b_profile, BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST_ALL);
        } else {
                return -1;
        }

diff --git a/apps/confbridge/include/confbridge.h b/apps/confbridge/include/confbridge.h
index 4b8249f414696e391d5837fa6ba06c1018af9167..237431e7d9db59650f65b06f3490865724a2e9f3 100644 (file)
--- a/apps/confbridge/include/confbridge.h
+++ b/apps/confbridge/include/confbridge.h
@@ -82,6 +82,9 @@ enum bridge_profile_flags {
        BRIDGE_OPT_REMB_BEHAVIOR_LOWEST = (1 << 9), /*!< The lowest estimated maximum bitrate is sent to the sender */
        BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST = (1 << 10), /*!< The highest estimated maximum bitrate is sent to the sender */
        BRIDGE_OPT_ENABLE_EVENTS = (1 << 11), /*!< Enable sending events to participants */
+       BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE_ALL = (1 << 12), /*!< The average of all REMB reports in the entire bridge is sent to each sender */
+       BRIDGE_OPT_REMB_BEHAVIOR_LOWEST_ALL = (1 << 13), /*!< The lowest estimated maximum bitrate from all receivers is sent to each sender */
+       BRIDGE_OPT_REMB_BEHAVIOR_HIGHEST_ALL = (1 << 14), /*!< The highest estimated maximum bitrate from all receivers is sent to each sender */
 };
 
 enum conf_menu_action_id {

diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index f9f852064a046e3cfef353e738fe0a5c49ffca7c..a90c7169d5386e25fd1784f3a258bbe40ba3daf5 100644 (file)
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -1332,6 +1332,36 @@ static int softmix_bridge_write(struct ast_bridge *bridge, struct ast_bridge_cha
        return res;
 }
 
+static void remb_collect_report_all(struct ast_bridge *bridge, struct softmix_bridge_data *softmix_data,
+       float bitrate)
+{
+       if (!softmix_data->bitrate) {
+               softmix_data->bitrate = bitrate;
+               return;
+       }
+
+       switch (bridge->softmix.video_mode.mode_data.sfu_data.remb_behavior) {
+       case AST_BRIDGE_VIDEO_SFU_REMB_AVERAGE_ALL:
+               softmix_data->bitrate = (softmix_data->bitrate + bitrate) / 2;
+               break;
+       case AST_BRIDGE_VIDEO_SFU_REMB_LOWEST_ALL:
+               if (bitrate < softmix_data->bitrate) {
+                       softmix_data->bitrate = bitrate;
+               }
+               break;
+       case AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST_ALL:
+               if (bitrate > softmix_data->bitrate) {
+                       softmix_data->bitrate = bitrate;
+               }
+               break;
+       case AST_BRIDGE_VIDEO_SFU_REMB_AVERAGE:
+       case AST_BRIDGE_VIDEO_SFU_REMB_LOWEST:
+       case AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST:
+               /* These will never actually get hit due to being handled by remb_collect_report below */
+               break;
+       }
+}
+
 static void remb_collect_report(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel,
        struct softmix_bridge_data *softmix_data, struct softmix_channel *sc)
 {
@@ -1355,6 +1385,14 @@ static void remb_collect_report(struct ast_bridge *bridge, struct ast_bridge_cha
                return;
        }
 
+       /* If we are using the "all" variants then we should use the bridge bitrate to store information */
+       if (bridge->softmix.video_mode.mode_data.sfu_data.remb_behavior == AST_BRIDGE_VIDEO_SFU_REMB_AVERAGE_ALL ||
+               bridge->softmix.video_mode.mode_data.sfu_data.remb_behavior == AST_BRIDGE_VIDEO_SFU_REMB_LOWEST_ALL ||
+               bridge->softmix.video_mode.mode_data.sfu_data.remb_behavior == AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST_ALL) {
+               remb_collect_report_all(bridge, softmix_data, bitrate);
+               return;
+       }
+
        for (i = 0; i < AST_VECTOR_SIZE(&sc->video_sources); ++i) {
                struct softmix_remb_collector *collector;
 
@@ -1380,6 +1418,11 @@ static void remb_collect_report(struct ast_bridge *bridge, struct ast_bridge_cha
                                collector->bitrate = bitrate;
                        }
                        break;
+               case AST_BRIDGE_VIDEO_SFU_REMB_AVERAGE_ALL:
+               case AST_BRIDGE_VIDEO_SFU_REMB_LOWEST_ALL:
+               case AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST_ALL:
+                       /* These will never actually get hit due to being handled by remb_collect_report_all above */
+                       break;
                }
        }
 
@@ -1390,8 +1433,10 @@ static void remb_collect_report(struct ast_bridge *bridge, struct ast_bridge_cha
        sc->remb.br_exp = 0;
 }
 
-static void remb_send_report(struct ast_bridge_channel *bridge_channel, struct softmix_channel *sc)
+static void remb_send_report(struct ast_bridge_channel *bridge_channel, struct softmix_bridge_data *softmix_data,
+       struct softmix_channel *sc)
 {
+       float bitrate = softmix_data->bitrate;
        int i;
        int exp;
 
@@ -1399,6 +1444,12 @@ static void remb_send_report(struct ast_bridge_channel *bridge_channel, struct s
                return;
        }
 
+       /* If there is no bridge level bitrate fall back to collector level */
+       if (!bitrate) {
+               bitrate = sc->remb_collector->bitrate;
+               sc->remb_collector->bitrate = 0;
+       }
+
        /* We always do this calculation as even when the bitrate is zero the browser
         * still prefers it to be accurate instead of lying.
         *
@@ -1442,10 +1493,10 @@ static void remb_send_report(struct ast_bridge_channel *bridge_channel, struct s
         * Precision on the "lower" end is lost due to zeros being shifted in. This loss is
         * both expected and acceptable.
         */
-       frexp(sc->remb_collector->bitrate, &exp);
+       frexp(bitrate, &exp);
        exp = exp > 18 ? exp - 18 : 0;
 
-       sc->remb_collector->feedback.remb.br_mantissa = sc->remb_collector->bitrate / (1 << exp);
+       sc->remb_collector->feedback.remb.br_mantissa = bitrate / (1 << exp);
        sc->remb_collector->feedback.remb.br_exp = exp;
 
        for (i = 0; i < AST_VECTOR_SIZE(&bridge_channel->stream_map.to_bridge); ++i) {
@@ -1466,8 +1517,6 @@ static void remb_send_report(struct ast_bridge_channel *bridge_channel, struct s
                sc->remb_collector->frame.stream_num = bridge_num;
                ast_bridge_channel_queue_frame(bridge_channel, &sc->remb_collector->frame);
        }
-
-       sc->remb_collector->bitrate = 0;
 }
 
 static void gather_softmix_stats(struct softmix_stats *stats,
@@ -1827,10 +1876,15 @@ static int softmix_mixing_loop(struct ast_bridge *bridge)
                        ast_bridge_channel_queue_frame(bridge_channel, &sc->write_frame);
 
                        if (remb_update) {
-                               remb_send_report(bridge_channel, sc);
+                               remb_send_report(bridge_channel, softmix_data, sc);
                        }
                }
 
+               if (remb_update) {
+                       /* In case we are doing bridge level REMB reset the bitrate so we start fresh */
+                       softmix_data->bitrate = 0;
+               }
+
                update_all_rates = 0;
                if (!stat_iteration_counter) {
                        update_all_rates = analyse_softmix_stats(&stats, softmix_data,

diff --git a/bridges/bridge_softmix/include/bridge_softmix_internal.h b/bridges/bridge_softmix/include/bridge_softmix_internal.h
index 15856b3786572467f9c26e0869bcc3028bd59b8e..ca70c3cb39885c267b0c3db4ce46b089b1b94099 100644 (file)
--- a/bridges/bridge_softmix/include/bridge_softmix_internal.h
+++ b/bridges/bridge_softmix/include/bridge_softmix_internal.h
@@ -216,6 +216,8 @@ struct softmix_bridge_data {
        struct timeval last_remb_update;
        /*! Per-bridge stream REMB collectors, which flow back to video source */
        AST_VECTOR(, struct softmix_remb_collector *) remb_collectors;
+       /*! Per-bridge REMB bitrate */
+       float bitrate;
 };
 
 struct softmix_mixing_array {

diff --git a/configs/samples/confbridge.conf.sample b/configs/samples/confbridge.conf.sample
index a214f345ba65a9d7370d3048d036b94c01e52332..e1481818be0480028459c953d5b4fdd403dd0417 100644 (file)
--- a/configs/samples/confbridge.conf.sample
+++ b/configs/samples/confbridge.conf.sample
@@ -254,7 +254,11 @@ type=bridge
 ;remb_behavior=average     ; How the combined REMB report for an SFU video bridge is constructed. If set to "average" then
                            ; the estimated maximum bitrate of each receiver is used to construct an average bitrate. If
                            ; set to "lowest" the lowest maximum bitrate is forwarded to the sender. If set to "highest"
-                           ; the highest maximum bitrate is forwarded to the sender. This defaults to "average".
+                           ; the highest maximum bitrate is forwarded to the sender. If set to "average_all" a single average
+                           ; is generated from every receiver and the same value is sent to every sender. If set to
+                           ; "lowest_all" the lowest maximum bitrate of all receivers is sent to every sender. If set to
+                           ; "highest_all" the highest maximum bitrate of all receivers is sent to every sender. This
+                           ; defaults to "average".
 
 ;enable_events=no          ; If enabled, recipients who joined the bridge via a channel driver
                            ; that supports Enhanced Messaging (currently only chan_pjsip) will

diff --git a/doc/CHANGES-staging/app_confbridge_remb_behavior_all.txt b/doc/CHANGES-staging/app_confbridge_remb_behavior_all.txt
new file mode 100644 (file)
index 0000000..6110a6f
--- /dev/null
+++ b/doc/CHANGES-staging/app_confbridge_remb_behavior_all.txt
@@ -0,0 +1,7 @@
+Subject: ConfBridge
+
+Add "average_all", "highest_all", and "lowest_all" values for
+the remb_behavior option. These values operate on a bridge
+level instead of a per-source level. This means that a single
+REMB value is calculated and sent to every sender, instead of
+a REMB value that is unique for the specific sender..

diff --git a/include/asterisk/bridge.h b/include/asterisk/bridge.h
index 621ed0d248af92d0edac2efa496300f83480f6e6..9be38a81160fc28cd406a5a4ff6e4be04b386729 100644 (file)
--- a/include/asterisk/bridge.h
+++ b/include/asterisk/bridge.h
@@ -135,6 +135,12 @@ enum ast_bridge_video_sfu_remb_behavior {
        AST_BRIDGE_VIDEO_SFU_REMB_LOWEST,
        /*! The highest reported bitrate is forwarded to the sender */
        AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST,
+       /*! The average of all reports WITHIN the bridge is sent to each sender */
+       AST_BRIDGE_VIDEO_SFU_REMB_AVERAGE_ALL,
+       /*! The lowest reported bitrate from all channels in the bridge is forwarded to each sender */
+       AST_BRIDGE_VIDEO_SFU_REMB_LOWEST_ALL,
+       /*! The highest reported bitrate from all channels in the bridge is forwarded to each sender */
+       AST_BRIDGE_VIDEO_SFU_REMB_HIGHEST_ALL,
 };
 
 /*! \brief This is used for selective forwarding unit configuration */