[Asterisk-code-review] pjsip: Fix a few media bugs with reinvites and asymmetric pa... (asterisk[master])
Joshua Colp
asteriskteam at digium.com
Tue Oct 25 06:18:14 CDT 2016
Joshua Colp has uploaded a new change for review. ( https://gerrit.asterisk.org/4174 )
Change subject: pjsip: Fix a few media bugs with reinvites and asymmetric payloads.
......................................................................
pjsip: Fix a few media bugs with reinvites and asymmetric payloads.
When channel format changes occurred as a result of an RTP
re-negotiation the bridge was not informed this had happened.
As a result the bridge technology was not re-evaluated and the
channel may have been in a bridge technology that was incompatible
with its formats. The bridge is now unbridged and the technology
re-evaluated when this occurs.
The chan_pjsip module also allowed asymmetric codecs for sending
and receiving. This did not work with all devices and caused one
way audio problems. The default has been changed to NOT do this
but to match the sending codec to the receiving codec. For users
who want asymmetric codecs an option has been added, asymmetric_rtp_codec,
which will return chan_pjsip to the previous behavior.
The codecs returned by the chan_pjsip module when queried by
the bridge_native_rtp module were also not reflective of the
actual negotiated codecs. The nativeformats are now returned as
they reflect the actual negotiated codecs.
ASTERISK-26423 #close
Change-Id: I6ec88c6e3912f52c334f1a26983ccb8f267020dc
---
M CHANGES
M channels/chan_pjsip.c
M configs/samples/pjsip.conf.sample
A contrib/ast-db-manage/config/versions/4468b4a91372_add_pjsip_asymmetric_rtp_codec.py
M include/asterisk/res_pjsip.h
M res/res_pjsip.c
M res/res_pjsip/pjsip_configuration.c
M res/res_pjsip_sdp_rtp.c
8 files changed, 72 insertions(+), 7 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/74/4174/1
diff --git a/CHANGES b/CHANGES
index 622973c..b1e3985 100644
--- a/CHANGES
+++ b/CHANGES
@@ -47,6 +47,13 @@
res_pjsip_multihomed module has also been moved into core res_pjsip to ensure
that messages are updated with the correct address information in all cases.
+chan_pjsip
+------------------
+ * The default behavior for RTP codecs has been changed. The sending codec will
+ now match the receiving codec. This can be turned off and behavior reverted
+ to asymmetric using the "asymmetric_rtp_codec" endpoint option. If this
+ option is set then the sending and received codec are allowed to differ.
+
------------------------------------------------------------------------------
--- Functionality changes from Asterisk 14.0.0 to Asterisk 14.1.0 ----------
------------------------------------------------------------------------------
diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 00d4a14..9f8f7b7 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -219,9 +219,7 @@
/*! \brief Function called by RTP engine to get peer capabilities */
static void chan_pjsip_get_codec(struct ast_channel *chan, struct ast_format_cap *result)
{
- struct ast_sip_channel_pvt *channel = ast_channel_tech_pvt(chan);
-
- ast_format_cap_append_from_cap(result, channel->session->endpoint->media.codecs, AST_MEDIA_TYPE_UNKNOWN);
+ ast_format_cap_append_from_cap(result, ast_channel_nativeformats(chan), AST_MEDIA_TYPE_UNKNOWN);
}
/*! \brief Destructor function for \ref transport_info_data */
@@ -704,13 +702,24 @@
session = channel->session;
- if (ast_format_cap_iscompatible_format(session->endpoint->media.codecs, f->subclass.format) == AST_FORMAT_CMP_NOT_EQUAL) {
- ast_debug(1, "Oooh, got a frame with format of %s on channel '%s' when endpoint '%s' is not configured for it\n",
- ast_format_get_name(f->subclass.format), ast_channel_name(ast),
- ast_sorcery_object_get_id(session->endpoint));
+ if (ast_format_cap_iscompatible_format(ast_channel_nativeformats(ast), f->subclass.format) == AST_FORMAT_CMP_NOT_EQUAL) {
+ ast_debug(1, "Oooh, got a frame with format of %s on channel '%s' when it has not been negotiated\n",
+ ast_format_get_name(f->subclass.format), ast_channel_name(ast));
ast_frfree(f);
return &ast_null_frame;
+ } else if (session->endpoint->asymmetric_rtp_codec &&
+ ast_format_cmp(ast_channel_rawwriteformat(ast), f->subclass.format) == AST_FORMAT_CMP_NOT_EQUAL) {
+ /* For maximum compatibility we ensure that the write format matches that of the received media */
+ ast_debug(1, "Oooh, got a frame with format of %s on channel '%s' when we're sending '%s', switching to match\n",
+ ast_format_get_name(f->subclass.format), ast_channel_name(ast),
+ ast_format_get_name(ast_channel_rawwriteformat(ast)));
+ ast_channel_set_rawwriteformat(ast, f->subclass.format);
+ ast_set_write_format(ast, ast_channel_writeformat(ast));
+
+ if (ast_channel_is_bridged(ast)) {
+ ast_channel_set_unbridged_nolock(ast, 1);
+ }
}
if (session->dsp) {
diff --git a/configs/samples/pjsip.conf.sample b/configs/samples/pjsip.conf.sample
index 3bb9dc5..6595423 100644
--- a/configs/samples/pjsip.conf.sample
+++ b/configs/samples/pjsip.conf.sample
@@ -759,6 +759,8 @@
; rather than advertising all joint codec capabilities. This
; limits the other side's codec choice to exactly what we prefer.
; default is no.
+;asymmetric_rtp_codec= ; Allow the sending and receiving codec to differ and
+ ; not be automatically matched (default: "no")
;==========================AUTH SECTION OPTIONS=========================
;[auth]
diff --git a/contrib/ast-db-manage/config/versions/4468b4a91372_add_pjsip_asymmetric_rtp_codec.py b/contrib/ast-db-manage/config/versions/4468b4a91372_add_pjsip_asymmetric_rtp_codec.py
new file mode 100644
index 0000000..c121495
--- /dev/null
+++ b/contrib/ast-db-manage/config/versions/4468b4a91372_add_pjsip_asymmetric_rtp_codec.py
@@ -0,0 +1,31 @@
+"""add pjsip asymmetric rtp codec
+
+Revision ID: 4468b4a91372
+Revises: a6ef36f1309
+Create Date: 2016-10-25 10:57:20.808815
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '4468b4a91372'
+down_revision = 'a6ef36f1309'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects.postgresql import ENUM
+
+YESNO_NAME = 'yesno_values'
+YESNO_VALUES = ['yes', 'no']
+
+def upgrade():
+ ############################# Enums ##############################
+
+ # yesno_values have already been created, so use postgres enum object
+ # type to get around "already created" issue - works okay with mysql
+ yesno_values = ENUM(*YESNO_VALUES, name=YESNO_NAME, create_type=False)
+
+ op.add_column('ps_endpoints', sa.Column('asymmetric_rtp_codec', yesno_values))
+
+
+def downgrade():
+ op.drop_column('ps_endpoints', 'asymmetric_rtp_codec')
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 92bdabb..894ea76 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -759,6 +759,8 @@
char *contact_user;
/*! Whether to response SDP offer with single most preferred codec. */
unsigned int preferred_codec_only;
+ /*! Do we allow an asymmetric RTP codec? */
+ unsigned int asymmetric_rtp_codec;
};
/*!
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 39c365a..916c464 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -925,6 +925,14 @@
On outbound requests, force the user portion of the Contact header to this value.
</para></description>
</configOption>
+ <configOption name="asymmetric_rtp_codec" default="no">
+ <synopsis>Allow the sending and receiving RTP codec to differ</synopsis>
+ <description><para>
+ When set to "yes" the codec in use for sending will be allowed to differ from
+ that of the received one. PJSIP will not automatically switch the sending one
+ to the receiving one.
+ </para></description>
+ </configOption>
</configObject>
<configObject name="auth">
<synopsis>Authentication type</synopsis>
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index 1866467..00c2233 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -1938,6 +1938,7 @@
ast_sorcery_object_field_register(sip_sorcery, "endpoint", "subscribe_context", "", OPT_CHAR_ARRAY_T, 0, CHARFLDSET(struct ast_sip_endpoint, subscription.context));
ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "contact_user", "", contact_user_handler, contact_user_to_str, NULL, 0, 0);
ast_sorcery_object_field_register(sip_sorcery, "endpoint", "preferred_codec_only", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, preferred_codec_only));
+ ast_sorcery_object_field_register(sip_sorcery, "endpoint", "asymmetric_rtp_codec", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, asymmetric_rtp_codec));
if (ast_sip_initialize_sorcery_transport()) {
ast_log(LOG_ERROR, "Failed to register SIP transport support with sorcery\n");
diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index a69aa1a..13a71d4 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -385,6 +385,11 @@
session->dsp = NULL;
}
}
+
+ if (ast_channel_is_bridged(session->channel)) {
+ ast_channel_set_unbridged_nolock(session->channel, 1);
+ }
+
ast_channel_unlock(session->channel);
}
--
To view, visit https://gerrit.asterisk.org/4174
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ec88c6e3912f52c334f1a26983ccb8f267020dc
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
More information about the asterisk-code-review
mailing list