From 1c5e68580af4556de86c2c33200fe3d43d499bed Mon Sep 17 00:00:00 2001 From: "Joshua C. Colp" Date: Tue, 21 Apr 2020 06:52:24 -0300 Subject: [PATCH] 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 --- bridges/bridge_native_rtp.c | 14 ++++++++++++-- bridges/bridge_simple.c | 14 ++++++++++++-- bridges/bridge_softmix.c | 2 +- channels/chan_pjsip.c | 2 +- channels/pjsip/dialplan_functions.c | 13 +++++++++++-- doc/UPGRADE-staging/stream_immutable_formats.txt | 9 +++++++++ include/asterisk/stream.h | 2 +- main/stream.c | 8 +++++++- res/res_pjsip_session.c | 6 ++++-- res/res_pjsip_session/pjsip_session_caps.c | 2 +- 10 files changed, 59 insertions(+), 13 deletions(-) create mode 100644 doc/UPGRADE-staging/stream_immutable_formats.txt diff --git a/bridges/bridge_native_rtp.c b/bridges/bridge_native_rtp.c index a6addf2711..19c6ab525b 100644 --- a/bridges/bridge_native_rtp.c +++ b/bridges/bridge_native_rtp.c @@ -859,7 +859,7 @@ static struct ast_stream_topology *native_rtp_request_stream_topology_update( 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 @@ static struct ast_stream_topology *native_rtp_request_stream_topology_update( 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 @@ static struct ast_stream_topology *native_rtp_request_stream_topology_update( 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 545b3ad1cb..abda774dd5 100644 --- a/bridges/bridge_simple.c +++ b/bridges/bridge_simple.c @@ -63,7 +63,7 @@ static struct ast_stream_topology *simple_bridge_request_stream_topology_update( 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 @@ static struct ast_stream_topology *simple_bridge_request_stream_topology_update( 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 @@ static struct ast_stream_topology *simple_bridge_request_stream_topology_update( 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 e1c6734966..09218ce996 100644 --- a/bridges/bridge_softmix.c +++ b/bridges/bridge_softmix.c @@ -2575,7 +2575,7 @@ fail: 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 5df39c3bd6..99ef6440da 100644 --- a/channels/chan_pjsip.c +++ b/channels/chan_pjsip.c @@ -820,7 +820,7 @@ static int is_compatible_format(struct ast_sip_session *session, struct ast_fram { 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 7cc4506a5e..2ace3ade38 100644 --- a/channels/pjsip/dialplan_functions.c +++ b/channels/pjsip/dialplan_functions.c @@ -1235,7 +1235,7 @@ static int media_offer_read_av(struct ast_sip_session *session, char *buf, 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 @@ static int media_offer_write_av(void *obj) 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 0000000000..2af5d9b271 --- /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 17275c58ae..bbeeacb06a 100644 --- a/include/asterisk/stream.h +++ b/include/asterisk/stream.h @@ -164,7 +164,7 @@ void ast_stream_set_type(struct ast_stream *stream, enum ast_media_type type); * * \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 41b7948d3f..5966a86f3a 100644 --- a/main/stream.c +++ b/main/stream.c @@ -108,6 +108,12 @@ struct ast_stream *ast_stream_alloc(const char *name, enum ast_media_type type) 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 @@ void ast_stream_set_type(struct ast_stream *stream, enum ast_media_type type) 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 558e706a0b..67899e4180 100644 --- a/res/res_pjsip_session.c +++ b/res/res_pjsip_session.c @@ -1752,9 +1752,11 @@ static int sip_session_refresh(struct ast_sip_session *session, 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 7aa4c1fd49..ca7ef44638 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 *ast_sip_session_create_joint_call_stream(const struct ast_sip 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,