[Asterisk-code-review] ari/resource channels: Add 'formats' to channel create/orig... (asterisk[master])

Joshua Colp asteriskteam at digium.com
Wed Jun 8 05:13:39 CDT 2016


Joshua Colp has submitted this change and it was merged.

Change subject: ari/resource_channels:  Add 'formats' to channel create/originate
......................................................................


ari/resource_channels:  Add 'formats' to channel create/originate

If you create a local channel and don't specify an originator channel
to take capabilities from, we automatically add all audio formats to
the new channel's capabilities. When we try to make the channel
compatible with another, the "best format" functions pick the best
format available, which in this case will be slin192.  While this is
great for preserving quality, it's the worst for performance and
overkill for the vast majority of applications.

In the absense of any other information, adding all formats is the
correct thing to do and it's not always possible to supply an
originator so a new parameter 'formats' has been added to the channel
create/originate functions. It's just a comma separated list of formats
to make availalble for the channel. Example: "ulaw,slin,slin16".
'formats' and 'originator' are mutually exclusive.

To facilitate determination of format names, the format name has been
added to "core show codecs".

ASTERISK-26070 #close

Change-Id: I091b23ecd41c1b4128d85028209772ee139f604b
---
M CHANGES
M include/asterisk/codec.h
M main/codec.c
M main/codec_builtin.c
M res/ari/resource_channels.c
M res/ari/resource_channels.h
M res/res_ari_channels.c
M res/res_ari_recordings.c
M rest-api/api-docs/channels.json
9 files changed, 162 insertions(+), 8 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/CHANGES b/CHANGES
index 281cff3..789ef7d 100644
--- a/CHANGES
+++ b/CHANGES
@@ -362,6 +362,18 @@
    server installations via alternate means (DUNDI for example). By default
    this feature is not used.
 
+Codecs
+------------------
+ * Added the associated format name to 'core show codecs'.
+
+res_ari_channels
+------------------
+ * Added 'formats' to channel create/originate to allow setting the allowed
+   formats for a channel when no originator channel is available.  Especially
+   useful for Local channel creation where no other format information is
+   available.  'core show codecs' can now be used to look up suitable format
+   names.
+
 ------------------------------------------------------------------------------
 --- Functionality changes from Asterisk 13.8.0 to Asterisk 13.9.0 ------------
 ------------------------------------------------------------------------------
diff --git a/include/asterisk/codec.h b/include/asterisk/codec.h
index 3873324..4ea94f9 100644
--- a/include/asterisk/codec.h
+++ b/include/asterisk/codec.h
@@ -77,6 +77,8 @@
 	unsigned int smooth;
 	/*! \brief The module that registered this codec */
 	struct ast_module *mod;
+	/*! \brief A format name for a default sane format using this codec */
+	const char *format_name;
 };
 
 /*!
diff --git a/main/codec.c b/main/codec.c
index 9c4169a..a762645 100644
--- a/main/codec.c
+++ b/main/codec.c
@@ -135,8 +135,8 @@
 				"\tIt does not indicate anything about your configuration.\n");
 	}
 
-	ast_cli(a->fd, "%8s %5s %8s %s\n","ID","TYPE","NAME","DESCRIPTION");
-	ast_cli(a->fd, "-----------------------------------------------------------------------------------\n");
+	ast_cli(a->fd, "%8s %-5s %-12s %-16s %s\n","ID","TYPE","NAME","FORMAT","DESCRIPTION");
+	ast_cli(a->fd, "------------------------------------------------------------------------------------------------\n");
 
 	ao2_rdlock(codecs);
 	i = ao2_iterator_init(codecs, AO2_ITERATOR_DONTLOCK);
@@ -164,10 +164,11 @@
 			}
 		}
 
-		ast_cli(a->fd, "%8u %5s %8s (%s)\n",
+		ast_cli(a->fd, "%8u %-5s %-12s %-16s (%s)\n",
 			codec->id,
 			ast_codec_media_type2str(codec->type),
 			codec->name,
+			S_OR(codec->format_name, "no cached format"),
 			codec->description);
 	}
 
@@ -216,7 +217,8 @@
 		return CLI_SUCCESS;
 	}
 
-	ast_cli(a->fd, "%11u %s\n", (unsigned int) codec->id, codec->description);
+	ast_cli(a->fd, "%11u %s (%s)\n", (unsigned int) codec->id, codec->description,
+		S_OR(codec->format_name, "no format"));
 
 	ao2_ref(codec, -1);
 
diff --git a/main/codec_builtin.c b/main/codec_builtin.c
index f86a8df..fdaa018 100644
--- a/main/codec_builtin.c
+++ b/main/codec_builtin.c
@@ -774,6 +774,7 @@
 		int __res_ ## __LINE__ = 0; \
 		struct ast_format *__fmt_ ## __LINE__; \
 		struct ast_codec *__codec_ ## __LINE__; \
+		codec.format_name = (codec).name; \
 		res |= __ast_codec_register(&(codec), NULL); \
 		__codec_ ## __LINE__ = ast_codec_get((codec).name, (codec).type, (codec).sample_rate); \
 		__fmt_ ## __LINE__ = __codec_ ## __LINE__ ? ast_format_create(__codec_ ## __LINE__) : NULL; \
@@ -783,14 +784,15 @@
 		__res_ ## __LINE__; \
 	})
 
-#define CODEC_REGISTER_AND_CACHE_NAMED(format_name, codec) \
+#define CODEC_REGISTER_AND_CACHE_NAMED(fmt_name, codec) \
 	({ \
 		int __res_ ## __LINE__ = 0; \
 		struct ast_format *__fmt_ ## __LINE__; \
 		struct ast_codec *__codec_ ## __LINE__; \
+		codec.format_name = fmt_name; \
 		res |= __ast_codec_register(&(codec), NULL); \
 		__codec_ ## __LINE__ = ast_codec_get((codec).name, (codec).type, (codec).sample_rate); \
-		__fmt_ ## __LINE__ = ast_format_create_named((format_name), __codec_ ## __LINE__); \
+		__fmt_ ## __LINE__ = ast_format_create_named((fmt_name), __codec_ ## __LINE__); \
 		res |= ast_format_cache_set(__fmt_ ## __LINE__); \
 		ao2_ref(__fmt_ ## __LINE__, -1); \
 		ao2_ref(__codec_ ## __LINE__, -1); \
diff --git a/res/ari/resource_channels.c b/res/ari/resource_channels.c
index b42581c..7c43bdd 100644
--- a/res/ari/resource_channels.c
+++ b/res/ari/resource_channels.c
@@ -915,6 +915,7 @@
 	const char *args_channel_id,
 	const char *args_other_channel_id,
 	const char *args_originator,
+	const char *args_formats,
 	struct ast_ari_response *response)
 {
 	char *dialtech;
@@ -933,6 +934,7 @@
 	};
 	struct ari_origination *origination;
 	pthread_t thread;
+	struct ast_format_cap *format_cap = NULL;
 
 	if ((assignedids.uniqueid && AST_MAX_PUBLIC_UNIQUEID < strlen(assignedids.uniqueid))
 		|| (assignedids.uniqueid2 && AST_MAX_PUBLIC_UNIQUEID < strlen(assignedids.uniqueid2))) {
@@ -944,6 +946,12 @@
 	if (ast_strlen_zero(args_endpoint)) {
 		ast_ari_response_error(response, 400, "Bad Request",
 			"Endpoint must be specified");
+		return;
+	}
+
+	if (!ast_strlen_zero(args_originator) && !ast_strlen_zero(args_formats)) {
+		ast_ari_response_error(response, 400, "Bad Request",
+			"Originator and formats can't both be specified");
 		return;
 	}
 
@@ -1069,7 +1077,41 @@
 		}
 	}
 
-	if (ast_dial_prerun(dial, other, NULL)) {
+	if (!ast_strlen_zero(args_formats)) {
+		char *format_name;
+		char *formats_copy = ast_strdupa(args_formats);
+
+		if (!(format_cap = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT))) {
+			ast_ari_response_alloc_failed(response);
+			ast_dial_destroy(dial);
+			ast_free(origination);
+			ast_channel_cleanup(other);
+			return;
+		}
+
+		while ((format_name = ast_strip(strsep(&formats_copy, ",")))) {
+			struct ast_format *fmt = ast_format_cache_get(format_name);
+
+			if (!fmt || ast_format_cap_append(format_cap, fmt, 0)) {
+				if (!fmt) {
+					ast_ari_response_error(
+						response, 400, "Bad Request",
+						"Provided format (%s) was not found", format_name);
+				} else {
+					ast_ari_response_alloc_failed(response);
+				}
+				ast_dial_destroy(dial);
+				ast_free(origination);
+				ast_channel_cleanup(other);
+				ao2_ref(format_cap, -1);
+				ao2_cleanup(fmt);
+				return;
+			}
+			ao2_ref(fmt, -1);
+		}
+	}
+
+	if (ast_dial_prerun(dial, other, format_cap)) {
 		ast_ari_response_alloc_failed(response);
 		ast_dial_destroy(dial);
 		ast_free(origination);
@@ -1078,6 +1120,7 @@
 	}
 
 	ast_channel_cleanup(other);
+	ao2_cleanup(format_cap);
 
 	chan = ast_dial_get_channel(dial, 0);
 	if (!chan) {
@@ -1218,6 +1261,7 @@
 		args->channel_id,
 		args->other_channel_id,
 		args->originator,
+		args->formats,
 		response);
 	ast_variables_destroy(variables);
 }
@@ -1254,6 +1298,7 @@
 		args->channel_id,
 		args->other_channel_id,
 		args->originator,
+		args->formats,
 		response);
 	ast_variables_destroy(variables);
 }
@@ -1527,6 +1572,12 @@
 		return;
 	}
 
+	if (!ast_strlen_zero(args->originator) && !ast_strlen_zero(args->formats)) {
+		ast_ari_response_error(response, 400, "Bad Request",
+			"Originator and formats can't both be specified");
+		return;
+	}
+
 	chan_data->stasis_stuff = ast_str_create(32);
 	if (!chan_data->stasis_stuff) {
 		ast_ari_response_alloc_failed(response);
@@ -1548,8 +1599,41 @@
 	originator = ast_channel_get_by_name(args->originator);
 	if (originator) {
 		request_cap = ao2_bump(ast_channel_nativeformats(originator));
+	} else if (!ast_strlen_zero(args->formats)) {
+		char *format_name;
+		char *formats_copy = ast_strdupa(args->formats);
+
+		if (!(request_cap = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT))) {
+			ast_ari_response_alloc_failed(response);
+			chan_data_destroy(chan_data);
+			return;
+		}
+
+		while ((format_name = ast_strip(strsep(&formats_copy, ",")))) {
+			struct ast_format *fmt = ast_format_cache_get(format_name);
+
+			if (!fmt || ast_format_cap_append(request_cap, fmt, 0)) {
+				if (!fmt) {
+					ast_ari_response_error(
+						response, 400, "Bad Request",
+						"Provided format (%s) was not found", format_name);
+				} else {
+					ast_ari_response_alloc_failed(response);
+				}
+				ao2_ref(request_cap, -1);
+				ao2_cleanup(fmt);
+				chan_data_destroy(chan_data);
+				return;
+			}
+			ao2_ref(fmt, -1);
+		}
 	} else {
-		request_cap = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
+		if (!(request_cap = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT))) {
+			ast_ari_response_alloc_failed(response);
+			chan_data_destroy(chan_data);
+			return;
+		}
+
 		ast_format_cap_append_by_type(request_cap, AST_MEDIA_TYPE_AUDIO);
 	}
 
diff --git a/res/ari/resource_channels.h b/res/ari/resource_channels.h
index c690d70..951433f 100644
--- a/res/ari/resource_channels.h
+++ b/res/ari/resource_channels.h
@@ -78,6 +78,8 @@
 	const char *other_channel_id;
 	/*! The unique id of the channel which is originating this one. */
 	const char *originator;
+	/*! The format name capability list to use if originator is not specified. Ex. "ulaw,slin16".  Format names an be found with "core show codecs". */
+	const char *formats;
 };
 /*!
  * \brief Body parsing function for /channels.
@@ -114,6 +116,8 @@
 	const char *other_channel_id;
 	/*! Unique ID of the calling channel */
 	const char *originator;
+	/*! The format name capability list to use if originator is not specified. Ex. "ulaw,slin16".  Format names an be found with "core show codecs". */
+	const char *formats;
 };
 /*!
  * \brief Body parsing function for /channels/create.
@@ -175,6 +179,8 @@
 	const char *other_channel_id;
 	/*! The unique id of the channel which is originating this one. */
 	const char *originator;
+	/*! The format name capability list to use if originator is not specified. Ex. "ulaw,slin16".  Format names an be found with "core show codecs". */
+	const char *formats;
 };
 /*!
  * \brief Body parsing function for /channels/{channelId}.
diff --git a/res/res_ari_channels.c b/res/res_ari_channels.c
index 951a547..412c06d 100644
--- a/res/res_ari_channels.c
+++ b/res/res_ari_channels.c
@@ -157,6 +157,10 @@
 	if (field) {
 		args->originator = ast_json_string_get(field);
 	}
+	field = ast_json_object_get(body, "formats");
+	if (field) {
+		args->formats = ast_json_string_get(field);
+	}
 	return 0;
 }
 
@@ -216,6 +220,9 @@
 		} else
 		if (strcmp(i->name, "originator") == 0) {
 			args.originator = (i->value);
+		} else
+		if (strcmp(i->name, "formats") == 0) {
+			args.formats = (i->value);
 		} else
 		{}
 	}
@@ -298,6 +305,10 @@
 	if (field) {
 		args->originator = ast_json_string_get(field);
 	}
+	field = ast_json_object_get(body, "formats");
+	if (field) {
+		args->formats = ast_json_string_get(field);
+	}
 	return 0;
 }
 
@@ -339,6 +350,9 @@
 		} else
 		if (strcmp(i->name, "originator") == 0) {
 			args.originator = (i->value);
+		} else
+		if (strcmp(i->name, "formats") == 0) {
+			args.formats = (i->value);
 		} else
 		{}
 	}
@@ -502,6 +516,10 @@
 	if (field) {
 		args->originator = ast_json_string_get(field);
 	}
+	field = ast_json_object_get(body, "formats");
+	if (field) {
+		args->formats = ast_json_string_get(field);
+	}
 	return 0;
 }
 
@@ -559,6 +577,9 @@
 		if (strcmp(i->name, "originator") == 0) {
 			args.originator = (i->value);
 		} else
+		if (strcmp(i->name, "formats") == 0) {
+			args.formats = (i->value);
+		} else
 		{}
 	}
 	for (i = path_vars; i; i = i->next) {
diff --git a/res/res_ari_recordings.c b/res/res_ari_recordings.c
index abc264d..a219435 100644
--- a/res/res_ari_recordings.c
+++ b/res/res_ari_recordings.c
@@ -257,6 +257,7 @@
 		break;
 	case 500: /* Internal Server Error */
 	case 501: /* Not Implemented */
+	case 403: /* The recording file could not be opened */
 	case 404: /* Recording not found */
 		is_valid = 1;
 		break;
diff --git a/rest-api/api-docs/channels.json b/rest-api/api-docs/channels.json
index aafd231..9e8c7c3 100644
--- a/rest-api/api-docs/channels.json
+++ b/rest-api/api-docs/channels.json
@@ -128,6 +128,14 @@
 							"required": false,
 							"allowMultiple": false,
 							"dataType": "string"
+						},
+						{
+							"name": "formats",
+							"description": "The format name capability list to use if originator is not specified. Ex. \"ulaw,slin16\".  Format names can be found with \"core show codecs\".",
+							"paramType": "query",
+							"required": false,
+							"allowMultiple": false,
+							"dataType": "string"
 						}
 					],
 					"errorResponses": [
@@ -192,6 +200,14 @@
 						{
 							"name": "originator",
 							"description": "Unique ID of the calling channel",
+							"paramType": "query",
+							"required": false,
+							"allowMultiple": false,
+							"dataType": "string"
+						},
+						{
+							"name": "formats",
+							"description": "The format name capability list to use if originator is not specified. Ex. \"ulaw,slin16\".  Format names can be found with \"core show codecs\".",
 							"paramType": "query",
 							"required": false,
 							"allowMultiple": false,
@@ -338,6 +354,14 @@
 							"required": false,
 							"allowMultiple": false,
 							"dataType": "string"
+						},
+						{
+							"name": "formats",
+							"description": "The format name capability list to use if originator is not specified. Ex. \"ulaw,slin16\".  Format names can be found with \"core show codecs\".",
+							"paramType": "query",
+							"required": false,
+							"allowMultiple": false,
+							"dataType": "string"
 						}
 					],
 					"errorResponses": [

-- 
To view, visit https://gerrit.asterisk.org/2918
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I091b23ecd41c1b4128d85028209772ee139f604b
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-code-review mailing list