[Asterisk-code-review] core: Add stream topology changing primitives with tests. (asterisk[master])

Anonymous Coward asteriskteam at digium.com
Wed Mar 15 17:23:31 CDT 2017


Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5131 )

Change subject: core: Add stream topology changing primitives with tests.
......................................................................


core: Add stream topology changing primitives with tests.

This change adds a few things to facilitate stream topology changing:

1. Control frame types have been added for use by the channel driver
to notify the application that the channel wants to change the stream
topology or that a stream topology change has been accepted. They are
also used by the indicate interface to the channel that the application
uses to indicate it wants to do the same.

2. Legacy behavior has been adopted in ast_read() such that if a
channel requests a stream topology change it is denied automatically
and the current stream topology is preserved if the application is
not capable of handling streams.

Tests have also been written which confirm the multistream and
non-multistream behavior.

ASTERISK-26839

Change-Id: Ia68ef22bca8e8457265ca4f0f9de600cbcc10bc9
---
M channels/chan_iax2.c
M funcs/func_frame_trace.c
M include/asterisk/channel.h
M include/asterisk/frame.h
M main/channel.c
M tests/test_stream.c
6 files changed, 329 insertions(+), 0 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Mark Michelson: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Matthew Fredrickson: Looks good to me, but someone else must approve



diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index e2f575d..6d2eda3 100644
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -1430,6 +1430,10 @@
 		/* Intended only for the sending machine's local channel structure. */
 	case AST_CONTROL_MASQUERADE_NOTIFY:
 		/* Intended only for masquerades when calling ast_indicate_data(). */
+	case AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE:
+		/* Intended only for internal stream topology manipulation. */
+	case AST_CONTROL_STREAM_TOPOLOGY_CHANGED:
+		/* Intended only for internal stream topology change notification. */
 	case AST_CONTROL_STREAM_STOP:
 	case AST_CONTROL_STREAM_SUSPEND:
 	case AST_CONTROL_STREAM_RESTART:
diff --git a/funcs/func_frame_trace.c b/funcs/func_frame_trace.c
index 8a0b3dd..49abfdf 100644
--- a/funcs/func_frame_trace.c
+++ b/funcs/func_frame_trace.c
@@ -336,6 +336,12 @@
 			/* Should never happen. */
 			ast_assert(0);
 			break;
+		case AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE:
+			ast_verbose("SubClass: STREAM_TOPOLOGY_REQUEST_CHANGE\n");
+			break;
+		case AST_CONTROL_STREAM_TOPOLOGY_CHANGED:
+			ast_verbose("SubClass: STREAM_TOPOLOGY_CHANGED\n");
+			break;
 		case AST_CONTROL_STREAM_STOP:
 			ast_verbose("SubClass: STREAM_STOP\n");
 			break;
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 3ae1e2f..9a3a967 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -4850,4 +4850,41 @@
  */
 int ast_channel_is_multistream(struct ast_channel *chan);
 
+/*!
+ * \brief Request that the stream topology of a channel change
+ *
+ * \param chan The channel to change
+ * \param topology The new stream topology
+ *
+ * \pre chan is locked
+ *
+ * \retval 0 request has been accepted to be attempted
+ * \retval -1 request could not be attempted
+ *
+ * \note This function initiates an asynchronous request to change the stream topology. It is not
+ *       guaranteed that the topology will change and until an AST_CONTROL_STREAM_TOPOLOGY_CHANGED
+ *       frame is received from the channel the current handler of the channel must tolerate the
+ *       stream topology as it currently exists.
+ *
+ * \note This interface is provided for applications and resources to request that the topology change.
+ *       It is not for use by the channel driver itself.
+ */
+int ast_channel_request_stream_topology_change(struct ast_channel *chan, struct ast_stream_topology *topology);
+
+/*!
+ * \brief Provide notice to a channel that the stream topology has changed
+ *
+ * \param chan The channel to provide notice to
+ * \param topology The new stream topology
+ *
+ * \pre chan is locked
+ *
+ * \retval 0 success
+ * \retval -1 failure
+ *
+ * \note This interface is provided for applications and resources to accept a topology change.
+ *       It is not for use by the channel driver itself.
+ */
+int ast_channel_stream_topology_changed(struct ast_channel *chan, struct ast_stream_topology *topology);
+
 #endif /* _ASTERISK_CHANNEL_H */
diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h
index c56539a..2f6c365 100644
--- a/include/asterisk/frame.h
+++ b/include/asterisk/frame.h
@@ -297,6 +297,8 @@
 	AST_CONTROL_UPDATE_RTP_PEER = 32, /*!< Interrupt the bridge and have it update the peer */
 	AST_CONTROL_PVT_CAUSE_CODE = 33, /*!< Contains an update to the protocol-specific cause-code stored for branching dials */
 	AST_CONTROL_MASQUERADE_NOTIFY = 34,	/*!< A masquerade is about to begin/end. (Never sent as a frame but directly with ast_indicate_data().) */
+	AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE = 35,    /*!< Channel indication that a stream topology change has been requested */
+	AST_CONTROL_STREAM_TOPOLOGY_CHANGED = 36,           /*!< Channel indication that a stream topology change has occurred */
 
 	/*
 	 * WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
diff --git a/main/channel.c b/main/channel.c
index 12a30e0..15c7fa4 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -4068,6 +4068,19 @@
 				}
 				ast_frfree(f);
 				f = &ast_null_frame;
+			} else if (f->subclass.integer == AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE && dropnondefault) {
+				/* The caller of this function is incapable of handling streams so we don't accept the change request
+				 * and stick to the streams currently on the channel.
+				 */
+				ast_channel_stream_topology_changed(chan, ast_channel_get_stream_topology(chan));
+				ast_frfree(f);
+				f = &ast_null_frame;
+			} else if (f->subclass.integer == AST_CONTROL_STREAM_TOPOLOGY_CHANGED && dropnondefault) {
+				/* The caller of this function is incapable of handling streams so we absord the notification that the
+				 * stream topology has changed.
+				 */
+				ast_frfree(f);
+				f = &ast_null_frame;
 			}
 			break;
 		case AST_FRAME_DTMF_END:
@@ -4494,6 +4507,8 @@
 	case AST_CONTROL_UPDATE_RTP_PEER:
 	case AST_CONTROL_PVT_CAUSE_CODE:
 	case AST_CONTROL_MASQUERADE_NOTIFY:
+	case AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE:
+	case AST_CONTROL_STREAM_TOPOLOGY_CHANGED:
 	case AST_CONTROL_STREAM_STOP:
 	case AST_CONTROL_STREAM_SUSPEND:
 	case AST_CONTROL_STREAM_REVERSE:
@@ -4792,6 +4807,8 @@
 	case AST_CONTROL_MCID:
 	case AST_CONTROL_MASQUERADE_NOTIFY:
 	case AST_CONTROL_UPDATE_RTP_PEER:
+	case AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE:
+	case AST_CONTROL_STREAM_TOPOLOGY_CHANGED:
 	case AST_CONTROL_STREAM_STOP:
 	case AST_CONTROL_STREAM_SUSPEND:
 	case AST_CONTROL_STREAM_REVERSE:
@@ -11147,3 +11164,27 @@
 {
 	return ast_channel_internal_errno();
 }
+
+int ast_channel_request_stream_topology_change(struct ast_channel *chan, struct ast_stream_topology *topology)
+{
+	ast_assert(chan != NULL);
+	ast_assert(topology != NULL);
+
+	if (!ast_channel_is_multistream(chan) || !ast_channel_tech(chan)->indicate) {
+		return -1;
+	}
+
+	return ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE, topology, sizeof(topology));
+}
+
+int ast_channel_stream_topology_changed(struct ast_channel *chan, struct ast_stream_topology *topology)
+{
+	ast_assert(chan != NULL);
+	ast_assert(topology != NULL);
+
+	if (!ast_channel_is_multistream(chan) || !ast_channel_tech(chan)->indicate) {
+		return -1;
+	}
+
+	return ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_CHANGED, topology, sizeof(topology));
+}
diff --git a/tests/test_stream.c b/tests/test_stream.c
index 7c78976..3bab67c 100644
--- a/tests/test_stream.c
+++ b/tests/test_stream.c
@@ -813,6 +813,8 @@
 	int frame_count;
 	int streams;
 	int frames_per_read;
+	unsigned int indicated_change_request;
+	unsigned int indicated_changed;
 };
 
 static struct ast_frame *mock_channel_read(struct ast_channel *chan)
@@ -856,6 +858,19 @@
 	struct mock_channel_pvt *pvt = ast_channel_tech_pvt(chan);
 
 	pvt->wrote = 1;
+
+	return 0;
+}
+
+static int mock_channel_indicate(struct ast_channel *chan, int condition, const void *data, size_t datalen)
+{
+	struct mock_channel_pvt *pvt = ast_channel_tech_pvt(chan);
+
+	if (condition == AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE) {
+		pvt->indicated_change_request = 1;
+	} else if (condition == AST_CONTROL_STREAM_TOPOLOGY_CHANGED) {
+		pvt->indicated_changed = 1;
+	}
 
 	return 0;
 }
@@ -1542,6 +1557,222 @@
 	return res;
 }
 
+AST_TEST_DEFINE(stream_topology_change_request_from_application_non_multistream)
+{
+	struct ast_channel_tech tech = {
+		.read = mock_channel_read,
+		.indicate = mock_channel_indicate,
+		.hangup = mock_channel_hangup,
+	};
+	struct ast_channel *mock_channel;
+	struct mock_channel_pvt *pvt;
+	enum ast_test_result_state res = AST_TEST_PASS;
+	int change_res;
+	RAII_VAR(struct ast_stream_topology *, topology, NULL, ast_stream_topology_free);
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "stream_topology_change_request_from_application_non_multistream";
+		info->category = "/main/stream/";
+		info->summary = "stream topology changing on non-multistream channel test";
+		info->description =
+			"Test that an application trying to change the stream topology of a non-multistream channel gets a failure";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	mock_channel = make_channel(test, 1, &tech);
+	ast_test_validate_cleanup(test, mock_channel, res, done);
+
+	pvt = ast_channel_tech_pvt(mock_channel);
+	pvt->indicated_change_request = 0;
+	pvt->indicated_changed = 0;
+
+	topology = ast_stream_topology_alloc();
+	ast_test_validate_cleanup(test, topology, res, done);
+
+	change_res = ast_channel_request_stream_topology_change(mock_channel, topology);
+
+	ast_test_validate_cleanup(test, change_res == -1, res, done);
+	ast_test_validate_cleanup(test, !pvt->indicated_change_request, res, done);
+
+	change_res = ast_channel_stream_topology_changed(mock_channel, topology);
+
+	ast_test_validate_cleanup(test, change_res == -1, res, done);
+	ast_test_validate_cleanup(test, !pvt->indicated_changed, res, done);
+
+done:
+	ast_hangup(mock_channel);
+
+	return res;
+}
+
+AST_TEST_DEFINE(stream_topology_change_request_from_channel_non_multistream)
+{
+	struct ast_channel_tech tech = {
+		.read_stream = mock_channel_read,
+		.write_stream = mock_channel_write_stream,
+		.indicate = mock_channel_indicate,
+		.hangup = mock_channel_hangup,
+	};
+	struct ast_channel *mock_channel;
+	struct mock_channel_pvt *pvt;
+	enum ast_test_result_state res = AST_TEST_PASS;
+	RAII_VAR(struct ast_stream_topology *, topology, NULL, ast_stream_topology_free);
+	struct ast_frame request_change = {
+		.frametype = AST_FRAME_CONTROL,
+		.subclass.integer = AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE,
+	};
+	struct ast_frame *fr = NULL;
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "stream_topology_change_request_from_channel_non_multistream";
+		info->category = "/main/stream/";
+		info->summary = "channel requesting stream topology change to non-multistream application test";
+		info->description =
+			"Test that a channel requesting a stream topology change from a non-multistream application does not work";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	mock_channel = make_channel(test, 1, &tech);
+	ast_test_validate_cleanup(test, mock_channel, res, done);
+
+	pvt = ast_channel_tech_pvt(mock_channel);
+	pvt->indicated_changed = 0;
+
+	topology = ast_stream_topology_alloc();
+	ast_test_validate_cleanup(test, topology, res, done);
+
+	request_change.data.ptr = topology;
+	ast_queue_frame(mock_channel, &request_change);
+
+	fr = ast_read(mock_channel);
+	ast_test_validate_cleanup(test, fr, res, done);
+	ast_test_validate_cleanup(test, fr == &ast_null_frame, res, done);
+	ast_test_validate_cleanup(test, pvt->indicated_changed, res, done);
+
+done:
+	if (fr) {
+		ast_frfree(fr);
+	}
+	ast_hangup(mock_channel);
+
+	return res;
+}
+
+AST_TEST_DEFINE(stream_topology_change_request_from_application)
+{
+	struct ast_channel_tech tech = {
+		.read_stream = mock_channel_read,
+		.write_stream = mock_channel_write_stream,
+		.indicate = mock_channel_indicate,
+		.hangup = mock_channel_hangup,
+	};
+	struct ast_channel *mock_channel;
+	struct mock_channel_pvt *pvt;
+	enum ast_test_result_state res = AST_TEST_PASS;
+	int change_res;
+	RAII_VAR(struct ast_stream_topology *, topology, NULL, ast_stream_topology_free);
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "stream_topology_change_request_from_application";
+		info->category = "/main/stream/";
+		info->summary = "stream topology change request from application test";
+		info->description =
+			"Test that an application changing the stream topology of a multistream capable channel receives success";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	mock_channel = make_channel(test, 1, &tech);
+	ast_test_validate_cleanup(test, mock_channel, res, done);
+
+	pvt = ast_channel_tech_pvt(mock_channel);
+	pvt->indicated_change_request = 0;
+	pvt->indicated_changed = 0;
+
+	topology = ast_stream_topology_alloc();
+	ast_test_validate_cleanup(test, topology, res, done);
+
+	change_res = ast_channel_request_stream_topology_change(mock_channel, topology);
+
+	ast_test_validate_cleanup(test, !change_res, res, done);
+	ast_test_validate_cleanup(test, pvt->indicated_change_request, res, done);
+
+	change_res = ast_channel_stream_topology_changed(mock_channel, topology);
+
+	ast_test_validate_cleanup(test, !change_res, res, done);
+	ast_test_validate_cleanup(test, pvt->indicated_changed, res, done);
+
+done:
+	ast_hangup(mock_channel);
+
+	return res;
+}
+
+AST_TEST_DEFINE(stream_topology_change_request_from_channel)
+{
+	struct ast_channel_tech tech = {
+		.read_stream = mock_channel_read,
+		.write_stream = mock_channel_write_stream,
+		.indicate = mock_channel_indicate,
+		.hangup = mock_channel_hangup,
+	};
+	struct ast_channel *mock_channel;
+	struct mock_channel_pvt *pvt;
+	enum ast_test_result_state res = AST_TEST_PASS;
+	RAII_VAR(struct ast_stream_topology *, topology, NULL, ast_stream_topology_free);
+	struct ast_frame request_change = {
+		.frametype = AST_FRAME_CONTROL,
+		.subclass.integer = AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE,
+	};
+	struct ast_frame *fr = NULL;
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "stream_topology_change_request_from_channel";
+		info->category = "/main/stream/";
+		info->summary = "channel requesting stream topology change to multistream application test";
+		info->description =
+			"Test that a channel requesting a stream topology change from a multistream application works";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	mock_channel = make_channel(test, 1, &tech);
+	ast_test_validate_cleanup(test, mock_channel, res, done);
+
+	pvt = ast_channel_tech_pvt(mock_channel);
+	pvt->indicated_changed = 0;
+
+	topology = ast_stream_topology_alloc();
+	ast_test_validate_cleanup(test, topology, res, done);
+
+	request_change.data.ptr = topology;
+	ast_queue_frame(mock_channel, &request_change);
+
+	fr = ast_read_stream(mock_channel);
+	ast_test_validate_cleanup(test, fr, res, done);
+	ast_test_validate_cleanup(test, fr->frametype == AST_FRAME_CONTROL, res, done);
+	ast_test_validate_cleanup(test, fr->subclass.integer == AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE, res, done);
+	ast_test_validate_cleanup(test, !pvt->indicated_changed, res, done);
+
+done:
+	if (fr) {
+		ast_frfree(fr);
+	}
+	ast_hangup(mock_channel);
+
+	return res;
+}
+
 static int unload_module(void)
 {
 	AST_TEST_UNREGISTER(stream_create);
@@ -1562,6 +1793,10 @@
 	AST_TEST_UNREGISTER(stream_write_multistream);
 	AST_TEST_UNREGISTER(stream_read_non_multistream);
 	AST_TEST_UNREGISTER(stream_read_multistream);
+	AST_TEST_UNREGISTER(stream_topology_change_request_from_application_non_multistream);
+	AST_TEST_UNREGISTER(stream_topology_change_request_from_channel_non_multistream);
+	AST_TEST_UNREGISTER(stream_topology_change_request_from_application);
+	AST_TEST_UNREGISTER(stream_topology_change_request_from_channel);
 	return 0;
 }
 
@@ -1584,6 +1819,10 @@
 	AST_TEST_REGISTER(stream_write_multistream);
 	AST_TEST_REGISTER(stream_read_non_multistream);
 	AST_TEST_REGISTER(stream_read_multistream);
+	AST_TEST_REGISTER(stream_topology_change_request_from_application_non_multistream);
+	AST_TEST_REGISTER(stream_topology_change_request_from_channel_non_multistream);
+	AST_TEST_REGISTER(stream_topology_change_request_from_application);
+	AST_TEST_REGISTER(stream_topology_change_request_from_channel);
 	return AST_MODULE_LOAD_SUCCESS;
 }
 

-- 
To view, visit https://gerrit.asterisk.org/5131
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia68ef22bca8e8457265ca4f0f9de600cbcc10bc9
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>



More information about the asterisk-code-review mailing list