[Asterisk-code-review] res_pjsip: return all codecs on a re-INVITE without SDP (asterisk[16])
Henning Westerholt
asteriskteam at digium.com
Fri Aug 26 04:16:58 CDT 2022
Henning Westerholt has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/18990 )
Change subject: res_pjsip: return all codecs on a re-INVITE without SDP
......................................................................
res_pjsip: return all codecs on a re-INVITE without SDP
Currently chan_pjsip on receiving a re-INVITE without SDP will only return
the codecs that are previously negotiated and not offering all enabled
codecs.
This causes interoperability issues with different equipment (e.g. from Cisco)
for some of our customers and probably also in other scenarios involving 3PCC
infrastructure.
According to RFC 3261, section 14.2 it SHOULD return all codecs on a re-INVITE
without SDP
The PR proposes a new parameter to configure this behaviour:
all_codecs_on_empty_reinvite. It includes the code, documentation and
example configuration additions.
ASTERISK-30193 #close
Change-Id: I69763708d5039d512f391e296ee8a4d43a1e2148
---
M configs/samples/pjsip.conf.sample
M include/asterisk/res_pjsip.h
M res/res_pjsip/config_global.c
M res/res_pjsip/pjsip_config.xml
M res/res_pjsip_session.c
5 files changed, 79 insertions(+), 10 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/90/18990/1
diff --git a/configs/samples/pjsip.conf.sample b/configs/samples/pjsip.conf.sample
index 4436951..328e526 100644
--- a/configs/samples/pjsip.conf.sample
+++ b/configs/samples/pjsip.conf.sample
@@ -1277,6 +1277,9 @@
; that the User Agent is capable of accepting a REFER request with
; creating an implicit subscription (see RFC 4488).
; (default: "yes")
+;all_codecs_on_empty_reinvite=yes
+ ; If we should return all codecs on empty re-INVITE.
+ ; RFC 3261 specify it SHOULD be done like this.
;allow_sending_180_after_183=yes ; Allow Asterisk to send 180 Ringing to an endpoint
; after 183 Session Progress has been send.
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 1714cff..3c20fbf 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -3500,4 +3500,12 @@
*/
void ast_sip_transport_state_unregister(struct ast_sip_tpmgr_state_callback *element);
+/*!
+ * \brief Retrieve the system setting 'all_codecs_on_empty_reinvite'.
+ * \since unreleased
+ *
+ * \retval non zero if we should return all codecs on empty re-INVITE
+ */
+unsigned int ast_sip_get_all_codecs_on_empty_reinvite(void);
+
#endif /* _RES_PJSIP_H */
diff --git a/res/res_pjsip/config_global.c b/res/res_pjsip/config_global.c
index 4620c3f..1056e53 100644
--- a/res/res_pjsip/config_global.c
+++ b/res/res_pjsip/config_global.c
@@ -54,6 +54,7 @@
#define DEFAULT_SEND_CONTACT_STATUS_ON_UPDATE_REGISTRATION 0
#define DEFAULT_TASKPROCESSOR_OVERLOAD_TRIGGER TASKPROCESSOR_OVERLOAD_TRIGGER_GLOBAL
#define DEFAULT_NOREFERSUB 1
+#define DEFAULT_ALL_CODECS_ON_EMPTY_REINVITE 1
/*!
* \brief Cached global config object
@@ -119,6 +120,8 @@
enum ast_sip_taskprocessor_overload_trigger overload_trigger;
/*! Nonzero if norefersub is to be sent in Supported header */
unsigned int norefersub;
+ /* Nonzero if we should return all codecs on empty re-INVITE */
+ unsigned int all_codecs_on_empty_reinvite;
};
static void global_destructor(void *obj)
@@ -537,6 +540,21 @@
return norefersub;
}
+unsigned int ast_sip_get_all_codecs_on_empty_reinvite(void)
+{
+ unsigned int all_codecs_on_empty_reinvite;
+ struct global_config *cfg;
+
+ cfg = get_global_cfg();
+ if (!cfg) {
+ return DEFAULT_ALL_CODECS_ON_EMPTY_REINVITE;
+ }
+
+ all_codecs_on_empty_reinvite = cfg->all_codecs_on_empty_reinvite;
+ ao2_ref(cfg, -1);
+ return all_codecs_on_empty_reinvite;
+}
+
static int overload_trigger_handler(const struct aco_option *opt,
struct ast_variable *var, void *obj)
{
@@ -744,6 +762,9 @@
ast_sorcery_object_field_register(sorcery, "global", "norefersub",
DEFAULT_NOREFERSUB ? "yes" : "no",
OPT_YESNO_T, 1, FLDSET(struct global_config, norefersub));
+ ast_sorcery_object_field_register(sorcery, "global", "all_codecs_on_empty_reinvite",
+ DEFAULT_ALL_CODECS_ON_EMPTY_REINVITE ? "yes" : "no",
+ OPT_BOOL_T, 1, FLDSET(struct global_config, all_codecs_on_empty_reinvite));
if (ast_sorcery_instance_observer_add(sorcery, &observer_callbacks_global)) {
return -1;
diff --git a/res/res_pjsip/pjsip_config.xml b/res/res_pjsip/pjsip_config.xml
index e9abc86..7d87097 100644
--- a/res/res_pjsip/pjsip_config.xml
+++ b/res/res_pjsip/pjsip_config.xml
@@ -2071,6 +2071,13 @@
</para>
</description>
</configOption>
+ <configOption name="all_codecs_on_empty_reinvite" default="yes">
+ <synopsis>If we should return all codecs on empty re-INVITE</synopsis>
+ <description><para>
+ If we should return all codecs on empty re-INVITE. RFC 3261
+ specify this as a SHOULD requirement.
+ </para></description>
+ </configOption>
</configObject>
</configFile>
</configInfo>
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 3c55af7..de32bae 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -101,7 +101,7 @@
char stream_type[1];
};
-static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer);
+static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer, const unsigned int answer_all_codecs);
static int sdp_handler_list_hash(const void *obj, int flags)
{
@@ -1619,7 +1619,7 @@
pjmedia_sdp_neg_get_active_local(inv_session->neg, &previous_sdp);
}
}
- SCOPE_EXIT_RTN_VALUE(create_local_sdp(inv_session, session, previous_sdp));
+ SCOPE_EXIT_RTN_VALUE(create_local_sdp(inv_session, session, previous_sdp, 0));
}
static void set_from_header(struct ast_sip_session *session)
@@ -2543,7 +2543,7 @@
pjmedia_sdp_neg_set_remote_offer(inv_session->pool, inv_session->neg, previous_offer);
}
- new_answer = create_local_sdp(inv_session, session, previous_offer);
+ new_answer = create_local_sdp(inv_session, session, previous_offer, 0);
if (!new_answer) {
ast_log(LOG_WARNING, "Could not create a new local SDP answer for channel '%s'\n",
ast_channel_name(session->channel));
@@ -2832,7 +2832,7 @@
{
pjmedia_sdp_session *offer;
- if (!(offer = create_local_sdp(session->inv_session, session, NULL))) {
+ if (!(offer = create_local_sdp(session->inv_session, session, NULL, 0))) {
pjsip_inv_terminate(session->inv_session, 500, PJ_FALSE);
return -1;
}
@@ -4021,10 +4021,10 @@
goto end;
}
/* We are creating a local SDP which is an answer to their offer */
- local = create_local_sdp(invite->session->inv_session, invite->session, sdp_info->sdp);
+ local = create_local_sdp(invite->session->inv_session, invite->session, sdp_info->sdp, 0);
} else {
/* We are creating a local SDP which is an offer */
- local = create_local_sdp(invite->session->inv_session, invite->session, NULL);
+ local = create_local_sdp(invite->session->inv_session, invite->session, NULL, 0);
}
/* If we were unable to create a local SDP terminate the session early, it won't go anywhere */
@@ -5027,7 +5027,7 @@
return 0;
}
-static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer)
+static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer, const unsigned int answer_all_codecs)
{
static const pj_str_t STR_IN = { "IN", 2 };
static const pj_str_t STR_IP4 = { "IP4", 3 };
@@ -5060,11 +5060,19 @@
/* We've encountered a situation where we have been told to create a local SDP but noone has given us any indication
* of what kind of stream topology they would like. We try to not alter the current state of the SDP negotiation
* by using what is currently negotiated. If this is unavailable we fall back to what is configured on the endpoint.
+ * We will also do this if wanted by the answer_all_codecs flag.
*/
+ ast_trace(1, "no information about stream topology received\n");
ast_stream_topology_free(session->pending_media_state->topology);
- if (session->active_media_state->topology) {
+ if (session->active_media_state->topology && !answer_all_codecs) {
+ ast_trace(1, "using existing topology\n");
session->pending_media_state->topology = ast_stream_topology_clone(session->active_media_state->topology);
} else {
+ if (answer_all_codecs) {
+ ast_trace(1, "fall back to endpoint configuration - answer all codecs\n");
+ } else {
+ ast_trace(1, "fall back to endpoint configuration\n");
+ }
session->pending_media_state->topology = ast_stream_topology_clone(session->endpoint->media.topology);
}
if (!session->pending_media_state->topology) {
@@ -5198,7 +5206,7 @@
SCOPE_EXIT_RTN("%s: handle_incoming_sdp failed\n", ast_sip_session_get_name(session));
}
- if ((answer = create_local_sdp(inv, session, offer))) {
+ if ((answer = create_local_sdp(inv, session, offer, 0))) {
pjsip_inv_set_sdp_answer(inv, answer);
SCOPE_EXIT_RTN("%s: Set SDP answer\n", ast_sip_session_get_name(session));
}
@@ -5211,6 +5219,7 @@
const pjmedia_sdp_session *previous_sdp = NULL;
pjmedia_sdp_session *offer;
int i;
+ unsigned int answer_all_codecs = 0;
/* We allow PJSIP to produce an SDP if no channel is present. This may result
* in an incorrect SDP occurring, but if no channel is present then we are in
@@ -5218,10 +5227,27 @@
* produce an SDP doesn't need to worry about a channel being present or not,
* just in case.
*/
+ SCOPE_ENTER(3, "%s\n", ast_sip_session_get_name(session));
+ ast_trace(1, "session_inv_on_create_offer\n");
if (!session->channel) {
return;
}
+ /* Some devices send a re-INVITE offer with empty SDP. Asterisk by default return
+ * an answer with the current used codecs, which is not strictly compliant to RFC
+ * 3261 (SHOULD requirement). So we detect this condition and include all
+ * configured codecs in the answer if the workaround is activated.
+ */
+ if (inv->invite_tsx && inv->state == PJSIP_INV_STATE_CONFIRMED
+ && inv->invite_tsx->method.id == PJSIP_INVITE_METHOD) {
+ ast_trace(1, "re-INVITE\n");
+ if (inv->invite_tsx->role == PJSIP_ROLE_UAS && !pjmedia_sdp_neg_was_answer_remote(inv->neg)
+ && ast_sip_get_all_codecs_on_empty_reinvite()) {
+ ast_trace(1, "no codecs in re-INIVTE, include all codecs in the answer\n");
+ answer_all_codecs = 1;
+ }
+ }
+
if (inv->neg) {
if (pjmedia_sdp_neg_was_answer_remote(inv->neg)) {
pjmedia_sdp_neg_get_active_remote(inv->neg, &previous_sdp);
@@ -5230,7 +5256,11 @@
}
}
- offer = create_local_sdp(inv, session, previous_sdp);
+ if (answer_all_codecs) {
+ offer = create_local_sdp(inv, session, NULL, 1);
+ } else {
+ offer = create_local_sdp(inv, session, previous_sdp, 0);
+ }
if (!offer) {
return;
}
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18990
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I69763708d5039d512f391e296ee8a4d43a1e2148
Gerrit-Change-Number: 18990
Gerrit-PatchSet: 1
Gerrit-Owner: Henning Westerholt <hw at gilawa.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220826/6fbd1eae/attachment-0001.html>
More information about the asterisk-code-review
mailing list