[Asterisk-code-review] stream: Enforce formats immutability and ensure formats exist. (asterisk[16])

Joshua Colp asteriskteam at digium.com
Tue Apr 21 05:06:40 CDT 2020


Joshua Colp has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/14276 )


Change subject: stream: Enforce formats immutability and ensure formats exist.
......................................................................

stream: Enforce formats immutability and ensure formats exist.

Some places in Asterisk did not treat the formats on a stream
as immutable when they are.

The ast_stream_get_formats function is now const to enforce this
and parts of Asterisk have been updated to take this into account.
Some violations of this were also fixed along the way.

An additional minor tweak is that streams are now allocated with
an empty format capabilities structure removing the need in various
places to check that one is present on the stream.

ASTERISK-28846

Change-Id: I32f29715330db4ff48edd6f1f359090458a9bfbe
---
M bridges/bridge_native_rtp.c
M bridges/bridge_simple.c
M bridges/bridge_softmix.c
M channels/chan_pjsip.c
M channels/pjsip/dialplan_functions.c
M include/asterisk/stream.h
M main/stream.c
M res/res_pjsip_session.c
8 files changed, 50 insertions(+), 13 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/76/14276/1

diff --git a/bridges/bridge_native_rtp.c b/bridges/bridge_native_rtp.c
index a6addf2..19c6ab5 100644
--- a/bridges/bridge_native_rtp.c
+++ b/bridges/bridge_native_rtp.c
@@ -859,7 +859,7 @@
 	struct ast_stream_topology *requested_topology)
 {
 	struct ast_stream *stream;
-	struct ast_format_cap *audio_formats = NULL;
+	const struct ast_format_cap *audio_formats = NULL;
 	struct ast_stream_topology *new_topology;
 	int i;
 
@@ -886,6 +886,8 @@
 
 	if (audio_formats) {
 		for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {
+			struct ast_format_cap *joint;
+
 			stream = ast_stream_topology_get_stream(new_topology, i);
 
 			if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||
@@ -893,8 +895,16 @@
 				continue;
 			}
 
-			ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats,
+			joint = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
+			if (!joint) {
+				continue;
+			}
+
+			ast_format_cap_append_from_cap(joint, ast_stream_get_formats(stream),
 				AST_MEDIA_TYPE_AUDIO);
+			ast_format_cap_append_from_cap(joint, audio_formats, AST_MEDIA_TYPE_AUDIO);
+			ast_stream_set_formats(stream, joint);
+			ao2_ref(joint, -1);
 		}
 	}
 
diff --git a/bridges/bridge_simple.c b/bridges/bridge_simple.c
index 545b3ad..abda774 100644
--- a/bridges/bridge_simple.c
+++ b/bridges/bridge_simple.c
@@ -63,7 +63,7 @@
 	struct ast_stream_topology *requested_topology)
 {
 	struct ast_stream *stream;
-	struct ast_format_cap *audio_formats = NULL;
+	const struct ast_format_cap *audio_formats = NULL;
 	struct ast_stream_topology *new_topology;
 	int i;
 
@@ -90,6 +90,8 @@
 
 	if (audio_formats) {
 		for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {
+			struct ast_format_cap *joint;
+
 			stream = ast_stream_topology_get_stream(new_topology, i);
 
 			if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||
@@ -97,8 +99,16 @@
 				continue;
 			}
 
-			ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats,
+			joint = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
+			if (!joint) {
+				continue;
+			}
+
+			ast_format_cap_append_from_cap(joint, ast_stream_get_formats(stream),
 				AST_MEDIA_TYPE_AUDIO);
+			ast_format_cap_append_from_cap(joint, audio_formats, AST_MEDIA_TYPE_AUDIO);
+			ast_stream_set_formats(stream, joint);
+			ao2_ref(joint, -1);
 		}
 	}
 
diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index e1c6734..09218ce 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -2575,7 +2575,7 @@
 static int validate_stream(struct ast_test *test, struct ast_stream *stream,
 	const struct stream_parameters *params)
 {
-	struct ast_format_cap *stream_caps;
+	const struct ast_format_cap *stream_caps;
 	struct ast_format_cap *params_caps;
 
 	if (ast_stream_get_type(stream) != params->type) {
diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 04dd200..c8b6fe7 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -820,7 +820,7 @@
 {
 	struct ast_stream_topology *topology = session->active_media_state->topology;
 	struct ast_stream *stream = ast_stream_topology_get_stream(topology, f->stream_num);
-	struct ast_format_cap *cap = ast_stream_get_formats(stream);
+	const struct ast_format_cap *cap = ast_stream_get_formats(stream);
 
 	return ast_format_cap_iscompatible_format(cap, f->subclass.format) != AST_FORMAT_CMP_NOT_EQUAL;
 }
diff --git a/channels/pjsip/dialplan_functions.c b/channels/pjsip/dialplan_functions.c
index 7cc4506..2ace3ad 100644
--- a/channels/pjsip/dialplan_functions.c
+++ b/channels/pjsip/dialplan_functions.c
@@ -1235,7 +1235,7 @@
 	struct ast_stream_topology *topology;
 	int idx;
 	struct ast_stream *stream = NULL;
-	struct ast_format_cap *caps;
+	const struct ast_format_cap *caps;
 	size_t accum = 0;
 
 	if (session->inv_session->dlg->state == PJSIP_DIALOG_STATE_ESTABLISHED) {
@@ -1352,9 +1352,18 @@
 	if (!stream) {
 		return 0;
 	}
-	caps = ast_stream_get_formats(stream);
+
+	caps = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
+	if (!caps) {
+		return -1;
+	}
+
+	ast_format_cap_append_from_cap(caps, ast_stream_get_formats(stream),
+		AST_MEDIA_TYPE_UNKNOWN);
 	ast_format_cap_remove_by_type(caps, data->media_type);
 	ast_format_cap_update_by_allow_disallow(caps, data->value, 1);
+	ast_stream_set_formats(stream, caps);
+	ao2_ref(caps, -1);
 
 	return 0;
 }
diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h
index ade740d..381ef36 100644
--- a/include/asterisk/stream.h
+++ b/include/asterisk/stream.h
@@ -164,7 +164,7 @@
  *
  * \since 15
  */
-struct ast_format_cap *ast_stream_get_formats(const struct ast_stream *stream);
+const struct ast_format_cap *ast_stream_get_formats(const struct ast_stream *stream);
 
 /*!
  * \brief Set the current negotiated formats of a stream
diff --git a/main/stream.c b/main/stream.c
index 626fa3a..02e6a54 100644
--- a/main/stream.c
+++ b/main/stream.c
@@ -108,6 +108,12 @@
 	stream->group = -1;
 	strcpy(stream->name, S_OR(name, "")); /* Safe */
 
+	stream->formats = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
+	if (!stream->formats) {
+		ast_free(stream);
+		return NULL;
+	}
+
 	return stream;
 }
 
@@ -179,7 +185,7 @@
 	stream->type = type;
 }
 
-struct ast_format_cap *ast_stream_get_formats(const struct ast_stream *stream)
+const struct ast_format_cap *ast_stream_get_formats(const struct ast_stream *stream)
 {
 	ast_assert(stream != NULL);
 
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 83ba6f5..be96bcb 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -1748,9 +1748,11 @@
 							continue;
 						} else {
 							/* However if the stream is otherwise remaining the same we can keep the formats
-							 * that exist on it already which allows media to continue to flow.
+							 * that exist on it already which allows media to continue to flow. We don't modify
+							 * the format capabilities but do need to cast it so that ao2_bump can raise the
+							 * reference count.
 							 */
-							joint_cap = ao2_bump(ast_stream_get_formats(existing_stream));
+							joint_cap = ao2_bump((struct ast_format_cap *)ast_stream_get_formats(existing_stream));
 						}
 					}
 					ast_stream_set_formats(stream, joint_cap);
@@ -2700,7 +2702,7 @@
 
 		for (i = 0; i < ast_stream_topology_get_count(req_topology); ++i) {
 			struct ast_stream *req_stream;
-			struct ast_format_cap *req_cap;
+			const struct ast_format_cap *req_cap;
 			struct ast_format_cap *joint_cap;
 			struct ast_stream *clone_stream;
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I32f29715330db4ff48edd6f1f359090458a9bfbe
Gerrit-Change-Number: 14276
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200421/1944f608/attachment-0001.html>


More information about the asterisk-code-review mailing list