<p>Kevin Harwell <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/14277">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Kevin Harwell: Looks good to me, but someone else must approve; Approved for Submit
George Joseph: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">stream: Enforce formats immutability and ensure formats exist.<br><br>Some places in Asterisk did not treat the formats on a stream<br>as immutable when they are.<br><br>The ast_stream_get_formats function is now const to enforce this<br>and parts of Asterisk have been updated to take this into account.<br>Some violations of this were also fixed along the way.<br><br>An additional minor tweak is that streams are now allocated with<br>an empty format capabilities structure removing the need in various<br>places to check that one is present on the stream.<br><br>ASTERISK-28846<br><br>Change-Id: I32f29715330db4ff48edd6f1f359090458a9bfbe<br>---<br>M bridges/bridge_native_rtp.c<br>M bridges/bridge_simple.c<br>M bridges/bridge_softmix.c<br>M channels/chan_pjsip.c<br>M channels/pjsip/dialplan_functions.c<br>A doc/UPGRADE-staging/stream_immutable_formats.txt<br>M include/asterisk/stream.h<br>M main/stream.c<br>M res/res_pjsip_session.c<br>M res/res_pjsip_session/pjsip_session_caps.c<br>10 files changed, 59 insertions(+), 13 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/bridges/bridge_native_rtp.c b/bridges/bridge_native_rtp.c</span><br><span>index a6addf2..19c6ab5 100644</span><br><span>--- a/bridges/bridge_native_rtp.c</span><br><span>+++ b/bridges/bridge_native_rtp.c</span><br><span>@@ -859,7 +859,7 @@</span><br><span> struct ast_stream_topology *requested_topology)</span><br><span> {</span><br><span> struct ast_stream *stream;</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_format_cap *audio_formats = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct ast_format_cap *audio_formats = NULL;</span><br><span> struct ast_stream_topology *new_topology;</span><br><span> int i;</span><br><span> </span><br><span>@@ -886,6 +886,8 @@</span><br><span> </span><br><span> if (audio_formats) {</span><br><span> for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_format_cap *joint;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> stream = ast_stream_topology_get_stream(new_topology, i);</span><br><span> </span><br><span> if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||</span><br><span>@@ -893,8 +895,16 @@</span><br><span> continue;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats,</span><br><span style="color: hsl(120, 100%, 40%);">+ joint = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!joint) {</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_format_cap_append_from_cap(joint, ast_stream_get_formats(stream),</span><br><span> AST_MEDIA_TYPE_AUDIO);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_format_cap_append_from_cap(joint, audio_formats, AST_MEDIA_TYPE_AUDIO);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_stream_set_formats(stream, joint);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(joint, -1);</span><br><span> }</span><br><span> }</span><br><span> </span><br><span>diff --git a/bridges/bridge_simple.c b/bridges/bridge_simple.c</span><br><span>index 545b3ad..abda774 100644</span><br><span>--- a/bridges/bridge_simple.c</span><br><span>+++ b/bridges/bridge_simple.c</span><br><span>@@ -63,7 +63,7 @@</span><br><span> struct ast_stream_topology *requested_topology)</span><br><span> {</span><br><span> struct ast_stream *stream;</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_format_cap *audio_formats = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct ast_format_cap *audio_formats = NULL;</span><br><span> struct ast_stream_topology *new_topology;</span><br><span> int i;</span><br><span> </span><br><span>@@ -90,6 +90,8 @@</span><br><span> </span><br><span> if (audio_formats) {</span><br><span> for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_format_cap *joint;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> stream = ast_stream_topology_get_stream(new_topology, i);</span><br><span> </span><br><span> if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||</span><br><span>@@ -97,8 +99,16 @@</span><br><span> continue;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats,</span><br><span style="color: hsl(120, 100%, 40%);">+ joint = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!joint) {</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_format_cap_append_from_cap(joint, ast_stream_get_formats(stream),</span><br><span> AST_MEDIA_TYPE_AUDIO);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_format_cap_append_from_cap(joint, audio_formats, AST_MEDIA_TYPE_AUDIO);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_stream_set_formats(stream, joint);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(joint, -1);</span><br><span> }</span><br><span> }</span><br><span> </span><br><span>diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c</span><br><span>index e1c6734..09218ce 100644</span><br><span>--- a/bridges/bridge_softmix.c</span><br><span>+++ b/bridges/bridge_softmix.c</span><br><span>@@ -2575,7 +2575,7 @@</span><br><span> static int validate_stream(struct ast_test *test, struct ast_stream *stream,</span><br><span> const struct stream_parameters *params)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_format_cap *stream_caps;</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct ast_format_cap *stream_caps;</span><br><span> struct ast_format_cap *params_caps;</span><br><span> </span><br><span> if (ast_stream_get_type(stream) != params->type) {</span><br><span>diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c</span><br><span>index 5df39c3..99ef644 100644</span><br><span>--- a/channels/chan_pjsip.c</span><br><span>+++ b/channels/chan_pjsip.c</span><br><span>@@ -820,7 +820,7 @@</span><br><span> {</span><br><span> struct ast_stream_topology *topology = session->active_media_state->topology;</span><br><span> struct ast_stream *stream = ast_stream_topology_get_stream(topology, f->stream_num);</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_format_cap *cap = ast_stream_get_formats(stream);</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct ast_format_cap *cap = ast_stream_get_formats(stream);</span><br><span> </span><br><span> return ast_format_cap_iscompatible_format(cap, f->subclass.format) != AST_FORMAT_CMP_NOT_EQUAL;</span><br><span> }</span><br><span>diff --git a/channels/pjsip/dialplan_functions.c b/channels/pjsip/dialplan_functions.c</span><br><span>index 7cc4506..2ace3ad 100644</span><br><span>--- a/channels/pjsip/dialplan_functions.c</span><br><span>+++ b/channels/pjsip/dialplan_functions.c</span><br><span>@@ -1235,7 +1235,7 @@</span><br><span> struct ast_stream_topology *topology;</span><br><span> int idx;</span><br><span> struct ast_stream *stream = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_format_cap *caps;</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct ast_format_cap *caps;</span><br><span> size_t accum = 0;</span><br><span> </span><br><span> if (session->inv_session->dlg->state == PJSIP_DIALOG_STATE_ESTABLISHED) {</span><br><span>@@ -1352,9 +1352,18 @@</span><br><span> if (!stream) {</span><br><span> return 0;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- caps = ast_stream_get_formats(stream);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ caps = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!caps) {</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_format_cap_append_from_cap(caps, ast_stream_get_formats(stream),</span><br><span style="color: hsl(120, 100%, 40%);">+ AST_MEDIA_TYPE_UNKNOWN);</span><br><span> ast_format_cap_remove_by_type(caps, data->media_type);</span><br><span> ast_format_cap_update_by_allow_disallow(caps, data->value, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_stream_set_formats(stream, caps);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(caps, -1);</span><br><span> </span><br><span> return 0;</span><br><span> }</span><br><span>diff --git a/doc/UPGRADE-staging/stream_immutable_formats.txt b/doc/UPGRADE-staging/stream_immutable_formats.txt</span><br><span>new file mode 100644</span><br><span>index 0000000..2af5d9b</span><br><span>--- /dev/null</span><br><span>+++ b/doc/UPGRADE-staging/stream_immutable_formats.txt</span><br><span>@@ -0,0 +1,9 @@</span><br><span style="color: hsl(120, 100%, 40%);">+Subject: Core</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+The streams API function ast_stream_get_formats is</span><br><span style="color: hsl(120, 100%, 40%);">+now defined as returning the format capabilities const.</span><br><span style="color: hsl(120, 100%, 40%);">+This has always been the case but was never enforced</span><br><span style="color: hsl(120, 100%, 40%);">+through the API itself. Any consumer of this API that</span><br><span style="color: hsl(120, 100%, 40%);">+is not treating the formats as immutable should update</span><br><span style="color: hsl(120, 100%, 40%);">+their code to create a new format capabilities and set</span><br><span style="color: hsl(120, 100%, 40%);">+it on the stream instead.</span><br><span>diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h</span><br><span>index 17275c5..bbeeacb 100644</span><br><span>--- a/include/asterisk/stream.h</span><br><span>+++ b/include/asterisk/stream.h</span><br><span>@@ -164,7 +164,7 @@</span><br><span> *</span><br><span> * \since 15</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">-struct ast_format_cap *ast_stream_get_formats(const struct ast_stream *stream);</span><br><span style="color: hsl(120, 100%, 40%);">+const struct ast_format_cap *ast_stream_get_formats(const struct ast_stream *stream);</span><br><span> </span><br><span> /*!</span><br><span> * \brief Get the count of the current negotiated formats of a stream</span><br><span>diff --git a/main/stream.c b/main/stream.c</span><br><span>index 41b7948..5966a86 100644</span><br><span>--- a/main/stream.c</span><br><span>+++ b/main/stream.c</span><br><span>@@ -108,6 +108,12 @@</span><br><span> stream->group = -1;</span><br><span> strcpy(stream->name, S_OR(name, "")); /* Safe */</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ stream->formats = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!stream->formats) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_free(stream);</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> return stream;</span><br><span> }</span><br><span> </span><br><span>@@ -179,7 +185,7 @@</span><br><span> stream->type = type;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-struct ast_format_cap *ast_stream_get_formats(const struct ast_stream *stream)</span><br><span style="color: hsl(120, 100%, 40%);">+const struct ast_format_cap *ast_stream_get_formats(const struct ast_stream *stream)</span><br><span> {</span><br><span> ast_assert(stream != NULL);</span><br><span> </span><br><span>diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c</span><br><span>index 558e706..67899e4 100644</span><br><span>--- a/res/res_pjsip_session.c</span><br><span>+++ b/res/res_pjsip_session.c</span><br><span>@@ -1752,9 +1752,11 @@</span><br><span> continue;</span><br><span> } else {</span><br><span> /* However if the stream is otherwise remaining the same we can keep the formats</span><br><span style="color: hsl(0, 100%, 40%);">- * that exist on it already which allows media to continue to flow.</span><br><span style="color: hsl(120, 100%, 40%);">+ * that exist on it already which allows media to continue to flow. We don't modify</span><br><span style="color: hsl(120, 100%, 40%);">+ * the format capabilities but do need to cast it so that ao2_bump can raise the</span><br><span style="color: hsl(120, 100%, 40%);">+ * reference count.</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- joint_cap = ao2_bump(ast_stream_get_formats(existing_stream));</span><br><span style="color: hsl(120, 100%, 40%);">+ joint_cap = ao2_bump((struct ast_format_cap *)ast_stream_get_formats(existing_stream));</span><br><span> }</span><br><span> }</span><br><span> ast_stream_set_formats(stream, joint_cap);</span><br><span>diff --git a/res/res_pjsip_session/pjsip_session_caps.c b/res/res_pjsip_session/pjsip_session_caps.c</span><br><span>index 7aa4c1f..ca7ef44 100644</span><br><span>--- a/res/res_pjsip_session/pjsip_session_caps.c</span><br><span>+++ b/res/res_pjsip_session/pjsip_session_caps.c</span><br><span>@@ -122,7 +122,7 @@</span><br><span> struct ast_stream *remote_stream)</span><br><span> {</span><br><span> struct ast_stream *joint_stream = ast_stream_clone(remote_stream, NULL);</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_format_cap *remote = ast_stream_get_formats(remote_stream);</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct ast_format_cap *remote = ast_stream_get_formats(remote_stream);</span><br><span> enum ast_media_type media_type = ast_stream_get_type(remote_stream);</span><br><span> </span><br><span> struct ast_format_cap *joint = ast_sip_create_joint_call_cap(remote,</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/14277">change 14277</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/14277"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I32f29715330db4ff48edd6f1f359090458a9bfbe </div>
<div style="display:none"> Gerrit-Change-Number: 14277 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>