[Asterisk-code-review] app_confbridge: Always set minimum video update interval. (asterisk[16])

[Joshua Colp]

Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/18805 )

Change subject: app_confbridge: Always set minimum video update interval.
......................................................................

app_confbridge: Always set minimum video update interval.

Currently, if multiple video-enabled ConfBridges are
conferenced together, we immediately get into a scenario
where an infinite sequence of video updates fills up
the taskprocessor queue and causes memory consumption
to climb unabated until Asterisk is killed. This is due
to the core bridging mechanism that provides video updates
(softmix_bridge_write_control in bridge_softmix.c)
continously updating all the channels in the bridge with
video updates.

The logic to do so in the core is that the video updates
should be provided if the video_update_discard property
for the bridge is 0, or if enough time has elapsed since
the last video update. Thus, we already have a safeguard
built in to ensure the scenario described above does not
happen. Currently, however, this safeguard is not being
adequately ensured.

In app_confbridge, the video_update_discard property
defaults to 2000, which is a healthy value that should
completely prevent this issue. However, this value is
only set onto the bridge in the SFU video mode. This
leaves video modes such as follow_talker completely
vulnerable, since video_update_discard will actually
be 0, since the default or set value was never applied.
As a result, the core bridging mechanism will always
try to provide video updates regardless of when the last
one was sent.

To prevent this issue from happening, we now always
set the video_update_discard property on the bridge
with the value from the bridge profile. The app_confbridge
defaults will thus ensure that infinite video updates
no longer happen in any video mode.

ASTERISK-29907 #close

Change-Id: I4accb2536ac62797950468e9930f12ef7dd486b2
---
M apps/app_confbridge.c
1 file changed, 3 insertions(+), 1 deletion(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
  Kevin Harwell: Looks good to me, approved



diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index 492649c..1733556 100644
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -1804,7 +1804,6 @@
 			ast_bridge_set_talker_src_video_mode(conference->bridge);
 		} else if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_VIDEO_SRC_SFU)) {
 			ast_bridge_set_sfu_video_mode(conference->bridge);
-			ast_bridge_set_video_update_discard(conference->bridge, conference->b_profile.video_update_discard);
 			ast_bridge_set_remb_send_interval(conference->bridge, conference->b_profile.remb_send_interval);
 			if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_REMB_BEHAVIOR_AVERAGE)) {
 				ast_brige_set_remb_behavior(conference->bridge, AST_BRIDGE_VIDEO_SFU_REMB_AVERAGE);
@@ -1824,6 +1823,9 @@
 			}
 		}
 
+		/* Always set the minimum interval between video updates, to avoid infinite video updates. */
+		ast_bridge_set_video_update_discard(conference->bridge, conference->b_profile.video_update_discard);
+
 		if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_ENABLE_EVENTS)) {
 			ast_bridge_set_send_sdp_label(conference->bridge, 1);
 		}

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18805
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I4accb2536ac62797950468e9930f12ef7dd486b2
Gerrit-Change-Number: 18805
Gerrit-PatchSet: 2
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220713/242f91da/attachment.html>