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

diff --git a/CHANGES b/CHANGES
index 8e7d66c81fc79bae8e94e536d3c586cd651e8039..b0b988525f33b489801f5b11053acdf7b04df144 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -21,6 +21,13 @@ res_pjsip
    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 13.11.0 to Asterisk 13.12.0 ----------
 ------------------------------------------------------------------------------

diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 23545112ec5d761d1bee32f44c13305755bfc19a..90553cb6330776f951a86111b970b83cfc46d83e 100644 (file)
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -219,9 +219,7 @@ static enum ast_rtp_glue_result chan_pjsip_get_vrtp_peer(struct ast_channel *cha
 /*! \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,15 +702,28 @@ static struct ast_frame *chan_pjsip_read(struct ast_channel *ast)
 
        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;
        }
 
+       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) {
                int dsp_features;
 

diff --git a/configs/samples/pjsip.conf.sample b/configs/samples/pjsip.conf.sample
index c9b5a8c07d1a5f2e5da85d7f2833ed602d86fb4d..2ef8933845a108220c6ec52aa08b8675d508c6f2 100644 (file)
--- a/configs/samples/pjsip.conf.sample
+++ b/configs/samples/pjsip.conf.sample
@@ -753,6 +753,8 @@
                    ; "0" or not enabled)
 ;contact_user= ; On outgoing requests, force the user portion of the Contact
                ; header to this value (default: "")
+;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 (file)
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 28ecf7f1d8902fdda63372ddf61b494d6fb0c2f7..4ad6607274149a21df30c33c55da3f1b40a826d6 100644 (file)
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -753,6 +753,8 @@ struct ast_sip_endpoint {
        unsigned int faxdetect_timeout;
        /*! Override the user on the outgoing Contact header with this value. */
        char *contact_user;
+       /*! 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 4927ea36a7873b17483e1e669b9087ff77664018..153352f9facb2ee25a93dbcd51da90d6002c871b 100644 (file)
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -919,6 +919,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 478e5c7d74e62285326bf612bdbe9204b10e148c..84dfa22641e05e0de745e45609b5882f0e342ae7 100644 (file)
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -1939,6 +1939,7 @@ int ast_res_pjsip_initialize_configuration(const struct ast_module_info *ast_mod
        ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "contact_acl", "", endpoint_acl_handler, contact_acl_to_str, NULL, 0, 0);
        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", "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 aaedde423c78d35ed754a1523ae3f20ded42924a..9e9815591f4065f7d49f2276c972ab30ee1fad77 100644 (file)
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -370,6 +370,11 @@ static int set_caps(struct ast_sip_session *session, struct ast_sip_session_medi
                                session->dsp = NULL;
                        }
                }
+
+               if (ast_channel_is_bridged(session->channel)) {
+                       ast_channel_set_unbridged_nolock(session->channel, 1);
+               }
+
                ast_channel_unlock(session->channel);
        }