[Asterisk-code-review] app confbridge: Use bridge join hook to send join and leave... (asterisk[master])
Joshua Colp
asteriskteam at digium.com
Mon Oct 1 06:24:22 CDT 2018
Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/10313 )
Change subject: app_confbridge: Use bridge join hook to send join and leave events
......................................................................
app_confbridge: Use bridge join hook to send join and leave events
The first attempt at publishing confbridge events to participants
involved publishing them at the same time stasis events were
created. This caused issues with bridge and channel locks. The
second attempt involved publishing them when the stasis events
were received by the code that published the confbridge AMI events.
This caused timing issues because, depending on resources available,
the event could be received before channels actually joined the
bridge and would therefore fail to send messages to the participant.
This attempt reverts to the original mechanism with one exception.
The join and leave events are published via bridge join and leave
hooks. This guarantees the states of the channels and bridge and
provides deterministic timing for event publishing.
Change-Id: I2660074f8a30a5224cb953d5e047ee84484a9036
---
M apps/app_confbridge.c
M apps/confbridge/confbridge_manager.c
M apps/confbridge/include/confbridge.h
3 files changed, 75 insertions(+), 14 deletions(-)
Approvals:
Benjamin Keith Ford: Looks good to me, but someone else must approve
Richard Mudgett: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Approved for Submit
diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index 1c6b464..a4e5c67 100644
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -576,6 +576,10 @@
return;
}
+ if (ast_test_flag(&conference->b_profile, BRIDGE_OPT_ENABLE_EVENTS)) {
+ conf_send_event_to_participants(conference, chan, msg);
+ }
+
if (channel_topic) {
stasis_publish(ast_channel_topic(chan), msg);
} else {
@@ -2311,6 +2315,25 @@
return 0;
}
+struct confbridge_hook_data {
+ struct confbridge_conference *conference;
+ struct confbridge_user *user;
+ enum ast_bridge_hook_type hook_type;
+};
+
+static int send_event_hook_callback(struct ast_bridge_channel *bridge_channel, void *data)
+{
+ struct confbridge_hook_data *hook_data = data;
+
+ if (hook_data->hook_type == AST_BRIDGE_HOOK_TYPE_JOIN) {
+ send_join_event(hook_data->user, hook_data->conference);
+ } else {
+ send_leave_event(hook_data->user, hook_data->conference);
+ }
+
+ return 0;
+}
+
/*! \brief The ConfBridge application */
static int confbridge_exec(struct ast_channel *chan, const char *data)
{
@@ -2328,6 +2351,9 @@
.tech_args.silence_threshold = DEFAULT_SILENCE_THRESHOLD,
.tech_args.drop_silence = 0,
};
+ struct confbridge_hook_data *join_hook_data;
+ struct confbridge_hook_data *leave_hook_data;
+
AST_DECLARE_APP_ARGS(args,
AST_APP_ARG(conf_name);
AST_APP_ARG(b_profile_name);
@@ -2510,8 +2536,39 @@
conf_moh_unsuspend(&user);
- /* Join our conference bridge for real */
- send_join_event(&user, conference);
+ join_hook_data = ast_malloc(sizeof(*join_hook_data));
+ if (!join_hook_data) {
+ res = -1;
+ goto confbridge_cleanup;
+ }
+ join_hook_data->user = &user;
+ join_hook_data->conference = conference;
+ join_hook_data->hook_type = AST_BRIDGE_HOOK_TYPE_JOIN;
+ res = ast_bridge_join_hook(&user.features, send_event_hook_callback,
+ join_hook_data, ast_free_ptr, 0);
+ if (res) {
+ ast_free(join_hook_data);
+ ast_log(LOG_ERROR, "Couldn't add bridge join hook for channel '%s'\n", ast_channel_name(chan));
+ goto confbridge_cleanup;
+ }
+
+ leave_hook_data = ast_malloc(sizeof(*leave_hook_data));
+ if (!leave_hook_data) {
+ /* join_hook_data is cleaned up by ast_bridge_features_cleanup via the goto */
+ res = -1;
+ goto confbridge_cleanup;
+ }
+ leave_hook_data->user = &user;
+ leave_hook_data->conference = conference;
+ leave_hook_data->hook_type = AST_BRIDGE_HOOK_TYPE_LEAVE;
+ res = ast_bridge_leave_hook(&user.features, send_event_hook_callback,
+ leave_hook_data, ast_free_ptr, 0);
+ if (res) {
+ /* join_hook_data is cleaned up by ast_bridge_features_cleanup via the goto */
+ ast_free(leave_hook_data);
+ ast_log(LOG_ERROR, "Couldn't add bridge leave hook for channel '%s'\n", ast_channel_name(chan));
+ goto confbridge_cleanup;
+ }
if (ast_bridge_join_hook(&user.features, join_callback, NULL, NULL, 0)) {
async_play_sound_ready(user.chan);
@@ -2533,8 +2590,6 @@
pbx_builtin_setvar_helper(chan, "CONFBRIDGE_RESULT", "HANGUP");
}
- send_leave_event(&user, conference);
-
/* if we're shutting down, don't attempt to do further processing */
if (ast_shutting_down()) {
/*
diff --git a/apps/confbridge/confbridge_manager.c b/apps/confbridge/confbridge_manager.c
index 51112ba..a7f2fce 100644
--- a/apps/confbridge/confbridge_manager.c
+++ b/apps/confbridge/confbridge_manager.c
@@ -395,6 +395,9 @@
struct ast_stream *stream;
struct ast_channel *chan = dir == LABEL_DIRECTION_SRC ? dest_chan : src_chan;
+ if (!chan) {
+ return;
+ }
topology = ast_channel_get_stream_topology(chan);
stream = get_stream(topology, AST_MEDIA_TYPE_VIDEO);
if (stream) {
@@ -458,8 +461,8 @@
ast_json_free(json);
}
-static void send_event_to_participants(struct confbridge_conference *conference,
- struct ast_channel *chan, struct stasis_message * msg)
+void conf_send_event_to_participants(struct confbridge_conference *conference,
+ struct ast_channel *chan, struct stasis_message *msg)
{
struct ast_bridge_blob *obj = stasis_message_data(msg);
struct ast_json *extras = obj->blob;
@@ -597,13 +600,6 @@
struct confbridge_conference *conference = conf_find_bridge(conference_name);
channel_text = ast_manager_build_channel_state_string(blob->channel);
-
- if (conference && ast_test_flag(&conference->b_profile, BRIDGE_OPT_ENABLE_EVENTS)) {
- struct ast_channel *chan = ast_channel_get_by_name(blob->channel->name);
-
- send_event_to_participants(conference, chan, message);
- ast_channel_cleanup(chan);
- }
ao2_cleanup(conference);
}
diff --git a/apps/confbridge/include/confbridge.h b/apps/confbridge/include/confbridge.h
index 51ff9a4..ac403d8 100644
--- a/apps/confbridge/include/confbridge.h
+++ b/apps/confbridge/include/confbridge.h
@@ -701,6 +701,16 @@
*/
struct confbridge_conference *conf_find_bridge(const char *conference_name);
-
+/*!
+ * \brief Send events to bridge participants.
+ * \since 15.7
+ * \since 16.1
+ *
+ * \param conference The conference bridge
+ * \param chan The channel triggering the action
+ * \param msg The stasis message describing the event
+ */
+void conf_send_event_to_participants(struct confbridge_conference *conference,
+ struct ast_channel *chan, struct stasis_message *msg);
#endif
--
To view, visit https://gerrit.asterisk.org/10313
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2660074f8a30a5224cb953d5e047ee84484a9036
Gerrit-Change-Number: 10313
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181001/4d4b96e4/attachment-0001.html>
More information about the asterisk-code-review
mailing list