[Asterisk-code-review] stream: Enforce formats immutability and ensure formats exist. (asterisk[master])
Kevin Harwell
asteriskteam at digium.com
Thu Apr 23 09:16:53 CDT 2020
Kevin Harwell has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/14277 )
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
A doc/UPGRADE-staging/stream_immutable_formats.txt
M include/asterisk/stream.h
M main/stream.c
M res/res_pjsip_session.c
M res/res_pjsip_session/pjsip_session_caps.c
10 files changed, 59 insertions(+), 13 deletions(-)
Approvals:
Kevin Harwell: Looks good to me, but someone else must approve; Approved for Submit
George Joseph: Looks good to me, approved
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 5df39c3..99ef644 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/doc/UPGRADE-staging/stream_immutable_formats.txt b/doc/UPGRADE-staging/stream_immutable_formats.txt
new file mode 100644
index 0000000..2af5d9b
--- /dev/null
+++ b/doc/UPGRADE-staging/stream_immutable_formats.txt
@@ -0,0 +1,9 @@
+Subject: Core
+
+The streams API function ast_stream_get_formats is
+now defined as returning the format capabilities const.
+This has always been the case but was never enforced
+through the API itself. Any consumer of this API that
+is not treating the formats as immutable should update
+their code to create a new format capabilities and set
+it on the stream instead.
diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h
index 17275c5..bbeeacb 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 Get the count of the current negotiated formats of a stream
diff --git a/main/stream.c b/main/stream.c
index 41b7948..5966a86 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 558e706..67899e4 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -1752,9 +1752,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);
diff --git a/res/res_pjsip_session/pjsip_session_caps.c b/res/res_pjsip_session/pjsip_session_caps.c
index 7aa4c1f..ca7ef44 100644
--- a/res/res_pjsip_session/pjsip_session_caps.c
+++ b/res/res_pjsip_session/pjsip_session_caps.c
@@ -122,7 +122,7 @@
struct ast_stream *remote_stream)
{
struct ast_stream *joint_stream = ast_stream_clone(remote_stream, NULL);
- struct ast_format_cap *remote = ast_stream_get_formats(remote_stream);
+ const struct ast_format_cap *remote = ast_stream_get_formats(remote_stream);
enum ast_media_type media_type = ast_stream_get_type(remote_stream);
struct ast_format_cap *joint = ast_sip_create_joint_call_cap(remote,
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14277
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I32f29715330db4ff48edd6f1f359090458a9bfbe
Gerrit-Change-Number: 14277
Gerrit-PatchSet: 3
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200423/a2f881d2/attachment-0001.html>
More information about the asterisk-code-review
mailing list