Skip to content

Commit

Permalink
res_pjsip: Adjust outgoing offer call pref.
Browse files Browse the repository at this point in the history 
This changes the outgoing offer call preference
default option to match the behavior of previous
versions of Asterisk.

The additional advanced codec negotiation options
have also been removed from the sample configuration
and marked as reserved for future functionality in
XML documentation.

The codec preference options have also been fixed to
enforce local codec configuration.

ASTERISK-29109

Change-Id: Iad19347bd5f3d89900c15ecddfebf5e20950a1c2
  • Loading branch information
@jcolp
jcolp committed Oct 13, 2020
1 parent fa023cb commit dcd2ed6
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 77 deletions.
66 changes: 7 additions & 59 deletions configs/samples/pjsip.conf.sample
View file
Original file line number Diff line number Diff line change
Expand Up @@ -828,69 +828,17 @@
; local - Include all codecs in the local list that
; are also in the remote list preserving the local
; order.
; local_merge - Include all codecs in BOTH lists
; preserving the local list order. Codes in the
; remote list not in the local list will be placed
; at the end of the joint list.
; local_merge - Include all codecs in the local list
; preserving the local order.
; local_first - Include only the first codec in the
; local list.
; remote - Include all codecs in the remote list that
; are also in the local list preserving remote list
; order. (default)
; remote_merge - Include all codecs in BOTH lists
; preserving the remote list order. Codes in the
; local list not in the remote list will be placed
; at the end of the joint list.
; remote_first - Include only the first codec in
; the remote list.
;codec_prefs_incoming_offer=; This is a string that describes how the codecs
; specified on an incoming SDP offer (pending) are
; reconciled with the codecs specified on an endpoint
; (configured) before being sent to the Asterisk core.
; The string actually specifies 4 name:value pair
; parameters separated by commas. Whitespace is
; ignored and they may be specified in any order.
; prefer: <pending | configured>,
; operation: <intersect | only_preferred
; | only_nonpreferred>,
; keep: <first | all>,
; transcode: <allow | prevent>
;codec_prefs_outgoing_offer=; This is a string that describes how the codecs
; specified in the topology that comes from the
; Asterisk core (pending) are reconciled with the
; codecs specified on an endpoint (configured)
; when sending an SDP offer.
; The string actually specifies 4 name:value pair
; parameters separated by commas. Whitespace is
; ignored and they may be specified in any order.
; prefer: <pending | configured>,
; operation: <intersect | union
; | only_preferred | only_nonpreferred>,
; keep: <first | all>,
; transcode: <allow | prevent>
;codec_prefs_incoming_answer=; This is a string that describes how the codecs
; specified in an incoming SDP answer (pending)
; are reconciled with the codecs specified on an
; endpoint (configured) when receiving an SDP
; answer.
; The string actually specifies 4 name:value pair
; parameters separated by commas. Whitespace is
; ignored and they may be specified in any order.
; prefer: <pending | configured>,
; operation: <intersect | union
; | only_preferred | only_nonpreferred>,
; keep: <first | all>
;codec_prefs_outgoing_answer=; This is a string that describes how the codecs
; that come from the core (pending) are reconciled
; with the codecs specified on an endpoint
; (configured) when sending an SDP answer.
; The string actually specifies 4 name:value pair
; parameters separated by commas. Whitespace is
; ignored and they may be specified in any order.
; prefer: <pending | configured>,
; operation: <intersect | union
; | only_preferred | only_nonpreferred>,
; keep: <first | all>
; order.
; remote_merge - Include all codecs in the local list
; preserving the remote list order. (default)
; remote_first - Include only the first codec in the
; remote list that is also in the local list.
;preferred_codec_only=no ; Respond to a SIP invite with the single most
; preferred codec rather than advertising all joint
; codec capabilities. This limits the other side's
Expand Down
20 changes: 11 additions & 9 deletions res/res_pjsip.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
on an endpoint (configured) before being sent to the Asterisk core.
The string actually specifies 4 <literal>name:value</literal> pair parameters
separated by commas. Whitespace is ignored and they may be specified in any order.
Note that this option is reserved for future functionality.
</para>
<para>
Expand Down Expand Up @@ -171,6 +172,7 @@
endpoint (configured) when sending an SDP offer.
The string actually specifies 4 <literal>name:value</literal> pair parameters
separated by commas. Whitespace is ignored and they may be specified in any order.
Note that this option is reserved for future functionality.
</para>
<para>
Expand Down Expand Up @@ -232,6 +234,8 @@
when receiving an SDP answer.
The string actually specifies 4 <literal>name:value</literal> pair parameters
separated by commas. Whitespace is ignored and they may be specified in any order.
Note that this option is reserved for future functionality.
</para>
<para>
Parameters:
Expand Down Expand Up @@ -288,6 +292,8 @@
when sending an SDP answer.
The string actually specifies 4 <literal>name:value</literal> pair parameters
separated by commas. Whitespace is ignored and they may be specified in any order.
Note that this option is reserved for future functionality.
</para>
<para>
Parameters:
Expand Down Expand Up @@ -1214,7 +1220,7 @@
</enumlist>
</description>
</configOption>
<configOption name="outgoing_call_offer_pref" default="local">
<configOption name="outgoing_call_offer_pref" default="remote_merge">
<synopsis>Preferences for selecting codecs for an outgoing call.</synopsis>
<description>
<para>Based on this setting, a joint list of preferred codecs between
Expand All @@ -1227,24 +1233,20 @@
preserving the local order.
</para></enum>
<enum name="local_merge"><para>
Include all codecs in BOTH lists preserving the local order.
Remote codecs not in the local list will be placed at the end
of the joint list.
Include all codecs in the local list preserving the local order.
</para></enum>
<enum name="local_first"><para>
Include only the first codec in the local list.
</para></enum>
<enum name="remote"><para>
Include all codecs in the remote list that are also in the local list
preserving the remote order. (default)
preserving the remote order.
</para></enum>
<enum name="remote_merge"><para>
Include all codecs in BOTH lists preserving the remote order.
Local codecs not in the remote list will be placed at the end
of the joint list.
Include all codecs in the local list preserving the remote order. (default)
</para></enum>
<enum name="remote_first"><para>
Include only the first codec in the remote list.
Include only the first codec in the remote list that is also in the local list.
</para></enum>
</enumlist>
</description>
Expand Down
2 changes: 1 addition & 1 deletion res/res_pjsip/pjsip_configuration.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2138,7 +2138,7 @@ int ast_res_pjsip_initialize_configuration(void)
ast_sorcery_object_field_register(sip_sorcery, "endpoint", "ignore_183_without_sdp", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, ignore_183_without_sdp));
ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "incoming_call_offer_pref", "local",
call_offer_pref_handler, incoming_call_offer_pref_to_str, NULL, 0, 0);
ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "outgoing_call_offer_pref", "remote",
ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "outgoing_call_offer_pref", "remote_merge",
call_offer_pref_handler, outgoing_call_offer_pref_to_str, NULL, 0, 0);
ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "codec_prefs_incoming_offer",
"prefer: pending, operation: intersect, keep: all, transcode: allow",
Expand Down
19 changes: 14 additions & 5 deletions res/res_pjsip_session/pjsip_session_caps.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,34 +68,43 @@ struct ast_format_cap *ast_sip_create_joint_call_cap(const struct ast_format_cap
{
struct ast_format_cap *joint = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
struct ast_format_cap *local_filtered = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
struct ast_format_cap *remote_filtered = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);

if (!joint || !local_filtered) {
if (!joint || !local_filtered || !remote_filtered) {
ast_log(LOG_ERROR, "Failed to allocate %s call offer capabilities\n",
ast_codec_media_type2str(media_type));
ao2_cleanup(joint);
ao2_cleanup(local_filtered);
ao2_cleanup(remote_filtered);
return NULL;
}

ast_format_cap_append_from_cap(local_filtered, local, media_type);

/* Remote should always be a subset of local, as local is what defines the underlying
* permitted formats.
*/
ast_format_cap_get_compatible(remote, local_filtered, remote_filtered);

if (ast_sip_call_codec_pref_test(codec_pref, LOCAL)) {
if (ast_sip_call_codec_pref_test(codec_pref, INTERSECT)) {
ast_format_cap_get_compatible(local_filtered, remote, joint); /* Get common, prefer local */
ast_format_cap_get_compatible(local_filtered, remote_filtered, joint); /* Get common, prefer local */
} else {
ast_format_cap_append_from_cap(joint, local_filtered, media_type); /* Add local */
ast_format_cap_append_from_cap(joint, remote, media_type); /* Then remote */
ast_format_cap_append_from_cap(joint, remote_filtered, media_type); /* Then remote */
}
} else {
if (ast_sip_call_codec_pref_test(codec_pref, INTERSECT)) {
ast_format_cap_get_compatible(remote, local_filtered, joint); /* Get common, prefer remote */
joint = remote_filtered; /* Get common, prefer remote - as was done when filtering initially */
remote_filtered = NULL;
} else {
ast_format_cap_append_from_cap(joint, remote, media_type); /* Add remote */
ast_format_cap_append_from_cap(joint, remote_filtered, media_type); /* Add remote */
ast_format_cap_append_from_cap(joint, local_filtered, media_type); /* Then local */
}
}

ao2_ref(local_filtered, -1);
ao2_cleanup(remote_filtered);

if (ast_format_cap_empty(joint)) {
return joint;
Expand Down
6 changes: 3 additions & 3 deletions tests/test_res_pjsip_session_caps.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ AST_TEST_DEFINE(low_level)
ast_test_status_update(test, "Testing outgoing expected pass\n");
RUN_CREATE_JOINT("ulaw,alaw,g722", "g722,g729,alaw", "local", 1, "alaw,g722", AST_TEST_PASS);
RUN_CREATE_JOINT("ulaw,alaw,g722", "g722,g729,alaw", "local_first", 1, "alaw", AST_TEST_PASS);
RUN_CREATE_JOINT("ulaw,alaw,g722", "g722,g729,alaw", "local_merge", 1, "ulaw,alaw,g722,g729", AST_TEST_PASS);
RUN_CREATE_JOINT("ulaw,alaw,g722", "g722,g729,alaw", "local_merge", 1, "ulaw,alaw,g722", AST_TEST_PASS);
RUN_CREATE_JOINT("ulaw,alaw,g722", "g722,g729,alaw", "remote", 1, "g722,alaw", AST_TEST_PASS);
RUN_CREATE_JOINT("ulaw,alaw,g722", "g722,g729,alaw", "remote_first", 1, "g722", AST_TEST_PASS);
RUN_CREATE_JOINT("ulaw,alaw,g722", "g722,g729,alaw", "remote_merge", 1, "g722,g729,alaw,ulaw", AST_TEST_PASS);
RUN_CREATE_JOINT("!all", "g722,g729,alaw", "remote_merge", 1, "g722,g729,alaw", AST_TEST_PASS);
RUN_CREATE_JOINT("ulaw,alaw,g722", "g722,g729,alaw", "remote_merge", 1, "g722,alaw,ulaw", AST_TEST_PASS);
RUN_CREATE_JOINT("!all", "g722,g729,alaw", "remote_merge", 1, "nothing", AST_TEST_PASS);

return rc >= 1 ? AST_TEST_FAIL : AST_TEST_PASS;
}
Expand Down

0 comments on commit dcd2ed6

Please sign in to comment.