[Asterisk-code-review] core_unreal: Fix T.38 faxing when using local channels. (asterisk[16])
Kevin Harwell
asteriskteam at digium.com
Tue Feb 16 18:11:11 CST 2021
Kevin Harwell has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15332 )
Change subject: core_unreal: Fix T.38 faxing when using local channels.
......................................................................
core_unreal: Fix T.38 faxing when using local channels.
After some changes to streams and topologies, receiving fax through
local channels stopped working. This change adds a stream topology with
a stream of type IMAGE to the local channel pair and allows fax to be
received.
ASTERISK-29035 #close
Change-Id: Id103cc5c9295295d8e68d5628e76220f8f17e9fb
---
M include/asterisk/core_unreal.h
M main/core_unreal.c
2 files changed, 90 insertions(+), 0 deletions(-)
Approvals:
Kevin Harwell: Looks good to me, approved; Approved for Submit
ben kolodny: Looks good to me, but someone else must approve
George Joseph: Verified
diff --git a/include/asterisk/core_unreal.h b/include/asterisk/core_unreal.h
index a28d39d..c6e770b 100644
--- a/include/asterisk/core_unreal.h
+++ b/include/asterisk/core_unreal.h
@@ -98,6 +98,8 @@
/*! Base name of the unreal channels. exten at context or other name. */
char name[AST_MAX_EXTENSION + AST_MAX_CONTEXT + 2];
struct ast_stream_topology *reqtopology; /*!< Requested stream topology */
+ struct ast_stream_topology *owner_old_topology; /*!< Stored topology for owner side when we need to restore later (faxing) */
+ struct ast_stream_topology *chan_old_topology; /*!< Stored topology for chan side when we need to restore later (faxing) */
};
#define AST_UNREAL_IS_OUTBOUND(a, b) ((a) == (b)->chan ? 1 : 0)
diff --git a/main/core_unreal.c b/main/core_unreal.c
index bff1c3e..653ef78 100644
--- a/main/core_unreal.c
+++ b/main/core_unreal.c
@@ -623,6 +623,9 @@
{
struct ast_unreal_pvt *p = ast_channel_tech_pvt(ast);
int res = 0;
+ struct ast_channel *chan = NULL;
+ struct ast_channel *owner = NULL;
+ const struct ast_control_t38_parameters *parameters;
if (!p) {
return -1;
@@ -677,6 +680,89 @@
res = unreal_colp_stream_topology_request_change(p, ast, data);
}
break;
+ case AST_CONTROL_T38_PARAMETERS:
+ parameters = data;
+ if (parameters->request_response == AST_T38_NEGOTIATED) {
+ struct ast_stream *stream;
+ struct ast_stream_topology *new_topology;
+
+ stream = ast_stream_alloc("local_fax", AST_MEDIA_TYPE_IMAGE);
+ if (!stream) {
+ ast_log(LOG_ERROR, "Failed to allocate memory for stream.\n");
+ res = -1;
+ break;
+ }
+ new_topology = ast_stream_topology_alloc();
+ if (!new_topology) {
+ ast_log(LOG_ERROR, "Failed to allocate memory for stream topology.\n");
+ ast_free(stream);
+ res = -1;
+ break;
+ }
+ ast_stream_topology_append_stream(new_topology, stream);
+
+ /*
+ * Lock both parts of the local channel so we can store their topologies and replace them with
+ * one that has a stream with type IMAGE. We can just hold the reference on the unreal_pvt
+ * structure and bump it, then steal the ref later when we are restoring the topology.
+ *
+ * We use ast_unreal_lock_all here because we don't know if the ;1 or ;2 side will get the
+ * signaling and we need to be sure that the locking order is the same to prevent possible
+ * deadlocks.
+ */
+ ast_unreal_lock_all(p, &chan, &owner);
+
+ if (owner) {
+ p->owner_old_topology = ao2_bump(ast_channel_get_stream_topology(owner));
+ ast_channel_set_stream_topology(owner, new_topology);
+ }
+
+ if (chan) {
+ p->chan_old_topology = ao2_bump(ast_channel_get_stream_topology(chan));
+
+ /* Bump the ref for new_topology, since it will be used by both sides of the local channel */
+ ao2_ref(new_topology, +1);
+ ast_channel_set_stream_topology(chan, new_topology);
+ }
+
+ ao2_unlock(p);
+ } else if (parameters->request_response == AST_T38_TERMINATED) {
+ /*
+ * Lock both parts of the local channel so we can restore their topologies to the original.
+ * The topology should be on the unreal_pvt structure, with a ref that we can steal. Same
+ * conditions as above.
+ */
+ ast_unreal_lock_all(p, &chan, &owner);
+
+ if (owner) {
+ ast_channel_set_stream_topology(owner, p->owner_old_topology);
+ p->owner_old_topology = NULL;
+ }
+
+ if (chan) {
+ ast_channel_set_stream_topology(chan, p->chan_old_topology);
+ p->chan_old_topology = NULL;
+ }
+
+ ao2_unlock(p);
+ }
+
+ /*
+ * We unlock ast_unreal_pvt in the above conditionals since there's no way to
+ * tell if it's been unlocked already or not when we get to this point, but
+ * if either of these are not NULL, we know that they are locked and need to
+ * unlock them.
+ */
+ if (owner) {
+ ast_channel_unlock(owner);
+ ast_channel_unref(owner);
+ }
+
+ if (chan) {
+ ast_channel_unlock(chan);
+ ast_channel_unref(chan);
+ }
+ /* Fall through for all T38 conditions */
default:
res = unreal_queue_indicate(p, ast, condition, data, datalen);
break;
@@ -1012,6 +1098,8 @@
doomed->reqcap = NULL;
ast_stream_topology_free(doomed->reqtopology);
doomed->reqtopology = NULL;
+ ao2_cleanup(doomed->owner_old_topology);
+ ao2_cleanup(doomed->chan_old_topology);
}
struct ast_unreal_pvt *ast_unreal_alloc(size_t size, ao2_destructor_fn destructor, struct ast_format_cap *cap)
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15332
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Id103cc5c9295295d8e68d5628e76220f8f17e9fb
Gerrit-Change-Number: 15332
Gerrit-PatchSet: 4
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: ben kolodny <3232963 at gmail.com>
Gerrit-CC: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210216/db1ec630/attachment-0001.html>
More information about the asterisk-code-review
mailing list