[Asterisk-code-review] ACN: Changes specific to the core (asterisk[master])

George Joseph asteriskteam at digium.com
Mon Jul 20 14:55:34 CDT 2020


George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/14688 )


Change subject: ACN: Changes specific to the core
......................................................................

ACN: Changes specific to the core

Allow passing a topology from the called channel back to the
calling channel.

 * Added a new function ast_queue_answer() that accepts a stream
   topology and queues an ANSWER CONTROL frame with it as the
   data.  This allows the called channel to indicate its resolved
   topology.

 * Added a new virtual function to the channel tech structure
   answer_with_stream_topology() that allows the calling channel
   to receive the called channel's topology.  Added
   ast_raw_answer_with_stream_topology() that invokes that virtual
   function.

 * Modified app_dial.c and features.c to grab the topology from the
   ANSWER frame queued by the answering channel and send it to
   the calling channel with ast_raw_answer_with_stream_topology().

 * Modified frame.c to automatically cleanup the reference
   to the topology on ANSWER frames.

Added a few debugging messages to stream.c.

Change-Id: I0115d2ed68d6bae0f87e85abcf16c771bdaf992c
---
M apps/app_dial.c
M include/asterisk/channel.h
M main/channel.c
M main/features.c
M main/frame.c
M main/stream.c
6 files changed, 179 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/88/14688/1

diff --git a/apps/app_dial.c b/apps/app_dial.c
index 95f36d7..54a59d1 100644
--- a/apps/app_dial.c
+++ b/apps/app_dial.c
@@ -1204,7 +1204,8 @@
 	struct privacy_args *pa,
 	const struct cause_args *num_in, int *result, char *dtmf_progress,
 	const int ignore_cc,
-	struct ast_party_id *forced_clid, struct ast_party_id *stored_clid)
+	struct ast_party_id *forced_clid, struct ast_party_id *stored_clid,
+	struct ast_bridge_config *config)
 {
 	struct cause_args num = *num_in;
 	int prestart = num.busy + num.congestion + num.nochan;
@@ -1418,6 +1419,18 @@
 							}
 						}
 						peer = c;
+						if (f->data.ptr != NULL && f->datalen == sizeof(struct ast_stream_topology *)) {
+							config->peer_topology = *((struct ast_stream_topology **)f->data.ptr);
+							/*
+							 * We need to bump the refcount on the topology to prevent it
+							 * from being cleaned up when the frame is cleaned up.
+							 */
+							ao2_bump(config->peer_topology);
+							ast_trace(2, "%s Found topology in frame: %p %p %s\n",
+								ast_channel_name(peer), f->data.ptr, config->peer_topology,
+								ast_str_tmp(256, ast_stream_topology_to_str(config->peer_topology, &STR_TMP)));
+						}
+
 						/* Inform everyone else that they've been canceled.
 						 * The dial end event for the peer will be sent out after
 						 * other Dial options have been handled.
@@ -2838,7 +2851,7 @@
 	}
 
 	peer = wait_for_answer(chan, &out_chans, &to, peerflags, opt_args, &pa, &num, &result,
-		dtmf_progress, ignore_cc, &forced_clid, &stored_clid);
+		dtmf_progress, ignore_cc, &forced_clid, &stored_clid, &config);
 
 	if (!peer) {
 		if (result) {
@@ -3267,6 +3280,7 @@
 				ast_channel_setoption(chan, AST_OPTION_OPRMODE, &oprmode, sizeof(oprmode), 0);
 			}
 			setup_peer_after_bridge_goto(chan, peer, &opts, opt_args);
+
 			res = ast_bridge_call(chan, peer, &config);
 		}
 	}
@@ -3304,6 +3318,18 @@
 	}
 
 done:
+	if (config.peer_topology) {
+		ast_trace(2, "%s Cleaning up topology: %p %s\n",
+			ast_channel_name(peer), &config.peer_topology,
+			ast_str_tmp(256, ast_stream_topology_to_str(config.peer_topology, &STR_TMP)));
+
+		/*
+		 * At this point, the channel driver that answered should have bumped the
+		 * topology refcount for itself.  Here we're cleaning up the reference we added
+		 * in wait_for_answer().
+		 */
+		ast_stream_topology_free(config.peer_topology);
+	}
 	if (config.warning_sound) {
 		ast_free((char *)config.warning_sound);
 	}
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index cc90c83..e150215 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -708,6 +708,19 @@
 	int (* const answer)(struct ast_channel *chan);
 
 	/*!
+	 * \brief Answer the channel with topology
+	 * \since 18
+	 *
+	 * \param chan The channel to answer
+	 * \param topology The topology to use, probably the peer's.
+	 *
+	 * \note The topology may be NULL when the peer doesn't support streams
+	 * or, in the case where transcoding is in effect, when this channel should use
+	 * its existing topology.
+	 */
+	int (* const answer_with_stream_topology)(struct ast_channel *chan, struct ast_stream_topology *topology);
+
+	/*!
 	 * \brief Read a frame (or chain of frames from the same stream), in standard format (see frame.h)
 	 *
 	 * \param chan channel to read frames from
@@ -1081,6 +1094,12 @@
 	 * exist when the end_bridge_callback is called, then it needs to be fixed up properly
 	 */
 	void (*end_bridge_callback_data_fixup)(struct ast_bridge_config *bconfig, struct ast_channel *originator, struct ast_channel *terminator);
+	/*!
+	 * When the peer queues an ANSWER control frame, it can indicate it's resolved topology.
+	 * If the calling channel implements the answer_with_stream_topology callback, the
+	 * peer's topology will be passed to the calling channel.
+	 */
+	struct ast_stream_topology *peer_topology;
 };
 
 struct chanmon;
@@ -1379,6 +1398,23 @@
 			   const void *data, size_t datalen);
 
 /*!
+ * \brief Queue an ANSWER control frame with topology
+ *
+ * \param chan channel to queue frame onto
+ * \param topology topology to be passed through the core to the peer channel
+ *
+ * \retval 0 success
+ * \retval non-zero failure
+ *
+ * \note The channel does not need to be locked before calling this function.
+ * The topology does NOT have it's reference count bumped automatically so
+ * bump it yourself if you need to.
+ */
+
+/*! \brief Queue an ANSWER control frame with topology */
+int ast_queue_answer(struct ast_channel *chan, const struct ast_stream_topology *topology);
+
+/*!
  * \brief Change channel name
  *
  * \pre Absolutely all channels _MUST_ be unlocked before calling this function.
@@ -1802,6 +1838,31 @@
 int ast_raw_answer(struct ast_channel *chan);
 
 /*!
+ * \brief Answer a channel passing in a stream topology
+ * \since 18
+ *
+ * \param chan channel to answer
+ * \param topology the peer's stream topology
+ *
+ * This function answers a channel and handles all necessary call
+ * setup functions.
+ *
+ * \note The channel passed does not need to be locked, but is locked
+ * by the function when needed.
+ *
+ * \note Unlike ast_answer(), this function will not wait for media
+ * flow to begin. The caller should be careful before sending media
+ * to the channel before incoming media arrives, as the outgoing
+ * media may be lost.
+ *
+ * \note The topology is usually that of the peer channel and may be NULL.
+ *
+ * \retval 0 on success
+ * \retval non-zero on failure
+ */
+int ast_raw_answer_with_stream_topology(struct ast_channel *chan, struct ast_stream_topology *topology);
+
+/*!
  * \brief Answer a channel, with a selectable delay before returning
  *
  * \param chan channel to answer
@@ -5054,4 +5115,18 @@
  */
 void *ast_channel_get_stream_topology_change_source(struct ast_channel *chan);
 
+/*!
+ * \brief Checks if a channel's technology implements a particular callback function
+ * \since 18
+ *
+ * \param chan The channel
+ * \param function The function to look for
+ *
+ * \retval 1 if the channel has a technology set and it implements the function
+ * \retval 0 if the channel doesn't have a technology set or it doesn't implement the function
+ */
+#define ast_channel_has_tech_function(chan, function) \
+	(ast_channel_tech(chan) ? ast_channel_tech(chan)->function != NULL : 0)
+
+
 #endif /* _ASTERISK_CHANNEL_H */
diff --git a/main/channel.c b/main/channel.c
index 8dd008d..bb82ee2 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1238,6 +1238,25 @@
 	return ast_queue_frame(chan, &f);
 }
 
+/*! \brief Queue an ANSWER control frame with topology */
+int ast_queue_answer(struct ast_channel *chan, const struct ast_stream_topology *topology)
+{
+	struct ast_frame f = {
+		AST_FRAME_CONTROL,
+		.subclass.integer = AST_CONTROL_ANSWER,
+		/*
+		 * Passing the address of the topology pointer may seem odd but __ast_queue_frame
+		 * copies the _contents_ of the address to the data portion of the frame so the pointer
+		 * to the topology is what's stored in the data.  Then it sets f->data.ptr to point
+		 * to that. When the frame is read, you can get the pointer to the topology like so...
+		 * struct ast_stream_topology *topology = *((struct ast_stream_topology **)f->data.ptr)
+		 */
+		.data.ptr = (void *) &topology,
+		.datalen = sizeof(topology)
+	};
+	return ast_queue_frame(chan, &f);
+}
+
 /*! \brief Set defer DTMF flag on channel */
 int ast_channel_defer_dtmf(struct ast_channel *chan)
 {
@@ -2619,7 +2638,8 @@
 	}
 }
 
-int ast_raw_answer(struct ast_channel *chan)
+
+int ast_raw_answer_with_stream_topology(struct ast_channel *chan, struct ast_stream_topology *topology)
 {
 	int res = 0;
 	SCOPE_TRACE(1, "%s\n", ast_channel_name(chan));
@@ -2650,7 +2670,10 @@
 	case AST_STATE_RINGING:
 	case AST_STATE_RING:
 		ast_channel_lock(chan);
-		if (ast_channel_tech(chan)->answer) {
+		if (ast_channel_tech(chan)->answer_with_stream_topology) {
+			res = ast_channel_tech(chan)->answer_with_stream_topology(chan, topology);
+
+		} else if (ast_channel_tech(chan)->answer) {
 			res = ast_channel_tech(chan)->answer(chan);
 		}
 		ast_setstate(chan, AST_STATE_UP);
@@ -2667,6 +2690,11 @@
 	return res;
 }
 
+int ast_raw_answer(struct ast_channel *chan)
+{
+	return ast_raw_answer_with_stream_topology(chan, NULL);
+}
+
 int __ast_answer(struct ast_channel *chan, unsigned int delay)
 {
 	int res = 0;
diff --git a/main/features.c b/main/features.c
index 51cc3ed..20ed8f4 100644
--- a/main/features.c
+++ b/main/features.c
@@ -76,6 +76,7 @@
 #include "asterisk/stasis_channels.h"
 #include "asterisk/features_config.h"
 #include "asterisk/max_forwards.h"
+#include "asterisk/stream.h"
 
 /*** DOCUMENTATION
 	<application name="Bridge" language="en_US">
@@ -558,12 +559,17 @@
 	set_config_flags(chan, config);
 
 	/* Answer if need be */
+
+	res = 0;
+
 	if (ast_channel_state(chan) != AST_STATE_UP) {
-		if (ast_raw_answer(chan)) {
+		res = ast_raw_answer_with_stream_topology(chan, config->peer_topology);
+		if (res != 0) {
 			return -1;
 		}
 	}
 
+
 #ifdef FOR_DEBUG
 	/* show the two channels and cdrs involved in the bridge for debug & devel purposes */
 	ast_channel_log("Pre-bridge CHAN Channel info", chan);
diff --git a/main/frame.c b/main/frame.c
index 4eeb3b6..cd723a2 100644
--- a/main/frame.c
+++ b/main/frame.c
@@ -139,6 +139,11 @@
 				|| fr->frametype == AST_FRAME_IMAGE) {
 				ao2_cleanup(fr->subclass.format);
 			}
+			if (fr->frametype == AST_FRAME_CONTROL && fr->subclass.integer == AST_CONTROL_ANSWER
+				&& fr->datalen == sizeof(struct ast_stream_topology *)) {
+				struct ast_stream_topology *topology = *((struct ast_stream_topology **)fr->data.ptr);
+				ao2_cleanup(topology);
+			}
 
 			AST_LIST_INSERT_HEAD(&frames->list, fr, frame_list);
 			frames->size++;
@@ -147,6 +152,12 @@
 	}
 #endif
 
+	if (fr->frametype == AST_FRAME_CONTROL && fr->subclass.integer == AST_CONTROL_ANSWER
+		&& fr->datalen == sizeof(struct ast_stream_topology *)) {
+		struct ast_stream_topology *topology = *((struct ast_stream_topology **)fr->data.ptr);
+		ao2_cleanup(topology);
+	}
+
 	if (fr->mallocd & AST_MALLOCD_DATA) {
 		if (fr->data.ptr) {
 			ast_free(fr->data.ptr - fr->offset);
@@ -208,6 +219,12 @@
 		return fr;
 	}
 
+	if (fr->frametype == AST_FRAME_CONTROL && fr->subclass.integer == AST_CONTROL_ANSWER
+		&& fr->datalen == sizeof(struct ast_stream_topology *)) {
+		struct ast_stream_topology *topology = *((struct ast_stream_topology **)fr->data.ptr);
+		ao2_bump(topology);
+	}
+
 	if (!(fr->mallocd & AST_MALLOCD_HDR)) {
 		/* Allocate a new header if needed */
 		if (!(out = ast_frame_header_new(file, line, func))) {
@@ -349,6 +366,13 @@
 		(f->frametype == AST_FRAME_IMAGE)) {
 		ao2_bump(out->subclass.format);
 	}
+	if (f->frametype == AST_FRAME_CONTROL && f->subclass.integer == AST_CONTROL_ANSWER
+		&& f->datalen == sizeof(struct ast_stream_topology *)) {
+		struct ast_stream_topology *topology = *((struct ast_stream_topology **)f->data.ptr);
+		ao2_bump(topology);
+	}
+
+
 	out->datalen = f->datalen;
 	out->samples = f->samples;
 	out->delivery = f->delivery;
diff --git a/main/stream.c b/main/stream.c
index a21177d..01b07ca 100644
--- a/main/stream.c
+++ b/main/stream.c
@@ -602,6 +602,16 @@
 			ast_format_cap_append(joint_caps, single, 0);
 			ao2_ref(single, -1);
 		}
+	} else {
+		if (error_message) {
+			ast_str_append(error_message, 0, "No common formats available for media type '%s' ",
+				ast_codec_media_type2str(pending_stream->type));
+			ast_format_cap_append_names(preferred_caps, error_message);
+			ast_str_append(error_message, 0, "<>");
+			ast_format_cap_append_names(nonpreferred_caps, error_message);
+			ast_str_append(error_message, 0, " with prefs: ");
+			ast_stream_codec_prefs_to_str(prefs, error_message);
+		}
 	}
 
 	joint_stream = ast_stream_clone(pending_stream, NULL);
@@ -613,7 +623,7 @@
 	/* ref to joint_caps will be transferred to the stream */
 	ast_stream_set_formats(joint_stream, joint_caps);
 
-	if (TRACE_ATLEAST(1)) {
+	if (TRACE_ATLEAST(3)) {
 		struct ast_str *buf = ast_str_create((AST_FORMAT_CAP_NAMES_LEN * 3) + AST_STREAM_MAX_CODEC_PREFS_LENGTH);
 		if (buf) {
 			ast_str_set(&buf, 0, "Resolved '%s' stream ", ast_codec_media_type2str(pending_stream->type));
@@ -1040,7 +1050,10 @@
 			ast_stream_set_state(joint_stream, AST_STREAM_STATE_REMOVED);
 		} else {
 			joint_stream = ast_stream_create_resolved(pending_stream, configured_stream, prefs, error_message);
-			if (ast_stream_get_format_count(joint_stream) == 0) {
+			if (!joint_stream) {
+				ao2_cleanup(joint_topology);
+				return NULL;
+			} else if (ast_stream_get_format_count(joint_stream) == 0) {
 				ast_stream_set_state(joint_stream, AST_STREAM_STATE_REMOVED);
 			}
 		}

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I0115d2ed68d6bae0f87e85abcf16c771bdaf992c
Gerrit-Change-Number: 14688
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200720/27d614d5/attachment-0001.html>


More information about the asterisk-code-review mailing list