[Asterisk-code-review] res_pjsip: Fix codec preference defaults. (asterisk[18])

[Joshua Colp]

Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/14752 )

Change subject: res_pjsip: Fix codec preference defaults.
......................................................................

res_pjsip: Fix codec preference defaults.

When reading in a codec preference configuration option
the value would be set on the respective option before
applying any default adjustments, resulting in the
configuration not being as expected.

This was exposed by the REST API push configuration as
it used the configuration returned by Asterisk to then do
a modification. In the case of codec preferences one of
the options had a transcode value of "unspecified" when the
defaults should have ensured it would be "allow" instead.

This also renames the options in other places that were
missed.

Change-Id: I4ad42e74fdf181be2e17bc75901c62591d403964
---
M res/res_pjsip/pjsip_configuration.c
1 file changed, 23 insertions(+), 12 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, approved; Approved for Submit
  Sean Bright: Looks good to me, but someone else must approve
  Benjamin Keith Ford: Looks good to me, but someone else must approve



diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index 20acdcf..b23de7e 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -1170,6 +1170,7 @@
 	struct ast_variable *var, void *obj)
 {
 	struct ast_sip_endpoint *endpoint = obj;
+	struct ast_stream_codec_negotiation_prefs *option_prefs;
 	struct ast_stream_codec_negotiation_prefs prefs;
 	struct ast_str *error_message = ast_str_create(128);
 	enum ast_stream_codec_negotiation_prefs_prefer_values default_prefer;
@@ -1185,7 +1186,7 @@
 	}
 	ast_free(error_message);
 
-	if (strcmp(var->name, "incoming_offer_codec_prefs") == 0) {
+	if (strcmp(var->name, "codec_prefs_incoming_offer") == 0) {
 		if (prefs.operation == CODEC_NEGOTIATION_OPERATION_UNION) {
 			ast_log(LOG_ERROR, "Endpoint '%s': Codec preference '%s' has invalid value '%s' for option: '%s'",
 				ast_sorcery_object_get_id(endpoint),
@@ -1194,21 +1195,26 @@
 				var->name);
 			return -1;
 		}
-		endpoint->media.codec_prefs_incoming_offer = prefs;
+		option_prefs = &endpoint->media.codec_prefs_incoming_offer;
 		default_prefer = CODEC_NEGOTIATION_PREFER_PENDING;
 		default_operation = CODEC_NEGOTIATION_OPERATION_INTERSECT;
-	} else if (strcmp(var->name, "outgoing_offer_codec_prefs") == 0) {
-		endpoint->media.codec_prefs_outgoing_offer = prefs;
+	} else if (strcmp(var->name, "codec_prefs_outgoing_offer") == 0) {
+		option_prefs = &endpoint->media.codec_prefs_outgoing_offer;
 		default_prefer = CODEC_NEGOTIATION_PREFER_PENDING;
 		default_operation = CODEC_NEGOTIATION_OPERATION_UNION;
-	} else if (strcmp(var->name, "incoming_answer_codec_prefs") == 0) {
-		endpoint->media.codec_prefs_incoming_answer = prefs;
+	} else if (strcmp(var->name, "codec_prefs_incoming_answer") == 0) {
+		option_prefs = &endpoint->media.codec_prefs_incoming_answer;
 		default_prefer = CODEC_NEGOTIATION_PREFER_PENDING;
 		default_operation = CODEC_NEGOTIATION_OPERATION_INTERSECT;
-	} else if (strcmp(var->name, "outgoing_answer_codec_prefs") == 0) {
-		endpoint->media.codec_prefs_outgoing_answer = prefs;
+	} else if (strcmp(var->name, "codec_prefs_outgoing_answer") == 0) {
+		option_prefs = &endpoint->media.codec_prefs_outgoing_answer;
 		default_prefer = CODEC_NEGOTIATION_PREFER_PENDING;
 		default_operation = CODEC_NEGOTIATION_OPERATION_INTERSECT;
+	} else {
+		ast_log(LOG_ERROR, "Endpoint '%s': Unsupported option '%s'\n",
+			ast_sorcery_object_get_id(endpoint),
+			var->name);
+		return -1;
 	}
 
 	if (prefs.prefer == CODEC_NEGOTIATION_PREFER_UNSPECIFIED) {
@@ -1227,6 +1233,11 @@
 		prefs.transcode = CODEC_NEGOTIATION_TRANSCODE_ALLOW;
 	}
 
+	/* Now that defaults have been applied as needed we apply the full codec
+	 * preference configuration to the option.
+	 */
+	*option_prefs = prefs;
+
 	return 0;
 }
 
@@ -2128,16 +2139,16 @@
 		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",
 		call_offer_pref_handler, outgoing_call_offer_pref_to_str, NULL, 0, 0);
-	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "incoming_offer_codec_prefs",
+	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "codec_prefs_incoming_offer",
 		"prefer: pending, operation: intersect, keep: all, transcode: allow",
 		codec_prefs_handler, incoming_offer_codec_prefs_to_str, NULL, 0, 0);
-	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "outgoing_offer_codec_prefs",
+	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "codec_prefs_outgoing_offer",
 		"prefer: pending, operation: union, keep: all, transcode: allow",
 		codec_prefs_handler, outgoing_offer_codec_prefs_to_str, NULL, 0, 0);
-	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "incoming_answer_codec_prefs",
+	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "codec_prefs_incoming_answer",
 		"prefer: pending, operation: intersect, keep: all",
 		codec_prefs_handler, incoming_answer_codec_prefs_to_str, NULL, 0, 0);
-	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "outgoing_answer_codec_prefs",
+	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "codec_prefs_outgoing_answer",
 		"prefer: pending, operation: intersect, keep: all",
 		codec_prefs_handler, outgoing_answer_codec_prefs_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "stir_shaken", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, stir_shaken));

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14752
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: I4ad42e74fdf181be2e17bc75901c62591d403964
Gerrit-Change-Number: 14752
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200811/868ef7f8/attachment-0001.html>