[Asterisk-code-review] fax: Fix crashes in PJSIP re-negotiation scenarios. (asterisk[16])
Joshua Colp
asteriskteam at digium.com
Mon Apr 20 10:22:39 CDT 2020
Joshua Colp has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/14274 )
Change subject: fax: Fix crashes in PJSIP re-negotiation scenarios.
......................................................................
fax: Fix crashes in PJSIP re-negotiation scenarios.
This change fixes a few re-negotiation issues
uncovered with fax.
1. The fax support uses its own mechanism for
re-negotiation by conveying T.38 information in
its own frames. The new support for re-negotiating
when adding/removing/changing streams was also
being triggered for this causing multiple re-INVITEs.
The new support will no longer trigger when
transitioning between fax.
2. In off-nominal re-negotiation cases it was
possible for some state information to be left
over and used by the next re-negotiation. This
is now cleared.
3. Both bridge_simple and bridge_native_rtp were
modifying an immutable format capabilities instead
of creating a new one and replacing the existing.
ASTERISK-28811
ASTERISK-28839
Change-Id: I8ed5924b53be9fe06a385c58817e5584b0f25cc2
---
M bridges/bridge_native_rtp.c
M bridges/bridge_simple.c
M res/res_pjsip_session.c
3 files changed, 49 insertions(+), 10 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/74/14274/1
diff --git a/bridges/bridge_native_rtp.c b/bridges/bridge_native_rtp.c
index a6addf2..b998e34 100644
--- a/bridges/bridge_native_rtp.c
+++ b/bridges/bridge_native_rtp.c
@@ -876,7 +876,8 @@
stream = ast_stream_topology_get_stream(existing_topology, i);
if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||
- ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {
+ ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED ||
+ !ast_stream_get_formats(stream)) {
continue;
}
@@ -886,6 +887,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 +896,19 @@
continue;
}
- ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats,
- AST_MEDIA_TYPE_AUDIO);
+ joint = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
+ if (!joint) {
+ continue;
+ }
+
+ if (ast_stream_get_formats(stream)) {
+ 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..7d5c801 100644
--- a/bridges/bridge_simple.c
+++ b/bridges/bridge_simple.c
@@ -80,7 +80,8 @@
stream = ast_stream_topology_get_stream(existing_topology, i);
if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||
- ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {
+ ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED ||
+ !ast_stream_get_formats(stream)) {
continue;
}
@@ -90,6 +91,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 +100,19 @@
continue;
}
- ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats,
- AST_MEDIA_TYPE_AUDIO);
+ joint = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
+ if (!joint) {
+ continue;
+ }
+
+ if (ast_stream_get_formats(stream)) {
+ 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/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 83ba6f5..502d7c8 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -1067,11 +1067,14 @@
if (topology) {
ast_channel_set_stream_topology(session->channel, topology);
/* If this is a remotely done renegotiation that has changed the stream topology notify what is
- * currently handling this channel.
+ * currently handling this channel. Note that fax uses its own process, so if we are transitioning
+ * between audio and fax or vice versa we don't notify.
*/
if (pjmedia_sdp_neg_was_answer_remote(session->inv_session->neg) == PJ_FALSE &&
session->active_media_state && session->active_media_state->topology &&
- !ast_stream_topology_equal(session->active_media_state->topology, topology)) {
+ !ast_stream_topology_equal(session->active_media_state->topology, topology) &&
+ !session->active_media_state->default_session[AST_MEDIA_TYPE_IMAGE] &&
+ !session->pending_media_state->default_session[AST_MEDIA_TYPE_IMAGE]) {
changed = 2;
}
}
@@ -2047,6 +2050,7 @@
pjsip_dialog *dlg;
RAII_VAR(struct ast_sip_session *, session, NULL, ao2_cleanup);
pjsip_rdata_sdp_info *sdp_info;
+ int deferred;
if (rdata->msg_info.msg->line.req.method.id != PJSIP_INVITE_METHOD ||
!(dlg = pjsip_ua_find_dialog(&rdata->msg_info.cid->id, &rdata->msg_info.to->tag, &rdata->msg_info.from->tag, PJ_FALSE)) ||
@@ -2136,7 +2140,11 @@
return PJ_FALSE;
}
- if (!sdp_requires_deferral(session, sdp_info->sdp)) {
+ deferred = sdp_requires_deferral(session, sdp_info->sdp);
+ if (deferred == -1) {
+ ast_sip_session_media_state_reset(session->pending_media_state);
+ return PJ_FALSE;
+ } else if (!deferred) {
return PJ_FALSE;
}
@@ -4334,6 +4342,7 @@
session = inv->mod_data[session_module.id];
if (handle_incoming_sdp(session, offer)) {
+ ast_sip_session_media_state_reset(session->pending_media_state);
return;
}
@@ -4412,7 +4421,9 @@
return;
}
- handle_negotiated_sdp(session, local, remote);
+ if (handle_negotiated_sdp(session, local, remote)) {
+ ast_sip_session_media_state_reset(session->pending_media_state);
+ }
}
static pjsip_redirect_op session_inv_on_redirected(pjsip_inv_session *inv, const pjsip_uri *target, const pjsip_event *e)
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14274
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I8ed5924b53be9fe06a385c58817e5584b0f25cc2
Gerrit-Change-Number: 14274
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/20200420/a873ab13/attachment-0001.html>
More information about the asterisk-code-review
mailing list