[Asterisk-code-review] main/format: Add an API call for retrieving format attributes (asterisk[13])

Matt Jordan asteriskteam at digium.com
Tue Aug 11 11:59:39 CDT 2015


Matt Jordan has submitted this change and it was merged.

Change subject: main/format: Add an API call for retrieving format attributes
......................................................................


main/format: Add an API call for retrieving format attributes

Some codecs that may be a third party library to Asterisk need to have
knowledge of the format attributes that were negotiated. Unfortunately,
when the great format migration of Asterisk 13 occurred, that ability
was lost.

This patch adds an API call, ast_format_attribute_get, to the core
format API, along with updates to the unit test to check the new API
call. A new callback is also now available for format attribute modules,
such that they can provide the format attribute values they manage.

Note that the API returns a void *. This is done as the format attribute
modules themselves may store format attributes in any particular manner
they like. Care should be taken by consumers of the API to check the
return value before casting and dereferencing. Consumers will obviously
need to have a priori knowledge of the type of the format attribute as
well.

Change-Id: Ieec76883dfb46ecd7aff3dc81a52c81f4dc1b9e3
---
M include/asterisk/format.h
M main/format.c
M tests/test_core_format.c
3 files changed, 139 insertions(+), 0 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Matt Jordan: Looks good to me, approved
  Ashley Sanders: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/include/asterisk/format.h b/include/asterisk/format.h
index 3da2d82..2ce1b97 100644
--- a/include/asterisk/format.h
+++ b/include/asterisk/format.h
@@ -120,6 +120,18 @@
 	 */
 	void (* const format_generate_sdp_fmtp)(const struct ast_format *format, unsigned int payload,
 		struct ast_str **str);
+
+	/*!
+	 * \since 13.6.0
+	 * \brief Retrieve a particular format attribute setting
+	 *
+	 * \param format The format containing attributes
+	 * \param name The name of the attribute to retrieve
+	 *
+	 * \retval NULL if the parameter is not set on the format
+	 * \retval non-NULL the format attribute value
+	 */
+	const void *(* const format_attribute_get)(const struct ast_format *format, const char *name);
 };
 
 /*!
@@ -204,6 +216,17 @@
 	const char *value);
 
 /*!
+ * \since 13.6.0
+ *
+ * \param format The format to retrieve the attribute from
+ * \param name Attribute name
+ *
+ * \retval non-NULL the attribute value
+ * \retval NULL the attribute does not exist or is unset
+ */
+const void *ast_format_attribute_get(const struct ast_format *format, const char *name);
+
+/*!
  * \brief This function is used to have a media format aware module parse and interpret
  * SDP attribute information. Once interpreted this information is stored on the format
  * itself using Asterisk format attributes.
diff --git a/main/format.c b/main/format.c
index 148c77f..8ac82f0 100644
--- a/main/format.c
+++ b/main/format.c
@@ -298,6 +298,17 @@
 	return interface->format_attribute_set(format, name, value);
 }
 
+const void *ast_format_attribute_get(const struct ast_format *format, const char *name)
+{
+	const struct ast_format_interface *interface = format->interface;
+
+	if (!interface || !interface->format_attribute_get) {
+		return NULL;
+	}
+
+	return interface->format_attribute_get(format, name);
+}
+
 struct ast_format *ast_format_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
 {
 	const struct ast_format_interface *interface = format->interface;
diff --git a/tests/test_core_format.c b/tests/test_core_format.c
index a6614d6..a3819c6 100644
--- a/tests/test_core_format.c
+++ b/tests/test_core_format.c
@@ -45,6 +45,7 @@
 static enum ast_format_cmp_res test_core_format_cmp(const struct ast_format *format1, const struct ast_format *format2);
 static struct ast_format *test_core_format_get_joint(const struct ast_format *format1, const struct ast_format *format2);
 static struct ast_format *test_core_format_attribute_set(const struct ast_format *format, const char *name, const char *value);
+static const void *test_core_format_attribute_get(const struct ast_format *format, const char *name);
 static struct ast_format *test_core_format_parse_sdp_fmtp(const struct ast_format *format, const char *attributes);
 static void test_core_format_generate_sdp_fmtp(const struct ast_format *format, unsigned int payload, struct ast_str **str);
 
@@ -55,6 +56,7 @@
 	.format_cmp = &test_core_format_cmp,
 	.format_get_joint = &test_core_format_get_joint,
 	.format_attribute_set = &test_core_format_attribute_set,
+	.format_attribute_get = &test_core_format_attribute_get,
 	.format_parse_sdp_fmtp = &test_core_format_parse_sdp_fmtp,
 	.format_generate_sdp_fmtp = &test_core_format_generate_sdp_fmtp,
 };
@@ -202,6 +204,19 @@
 	return clone;
 }
 
+/*! \brief Format attribute callback for retrieving an attribute */
+static const void *test_core_format_attribute_get(const struct ast_format *format, const char *name)
+{
+	struct test_core_format_pvt *pvt = ast_format_get_attribute_data(format);
+
+	if (!strcmp(name, "one")) {
+		return &pvt->field_one;
+	} else if (!strcmp(name, "two")) {
+		return &pvt->field_two;
+	}
+	return NULL;
+}
+
 /*! \brief Format attribute callback to construct a format from an SDP fmtp line */
 static struct ast_format *test_core_format_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
 {
@@ -329,6 +344,55 @@
 
 	ast_test_validate(test, test_callbacks_called.format_attribute_set == 1);
 	ast_test_validate(test, test_callbacks_called.format_cmp == 1);
+
+	return AST_TEST_PASS;
+}
+
+AST_TEST_DEFINE(format_retrieve_attr)
+{
+	RAII_VAR(struct ast_codec *, codec, NULL, ao2_cleanup);
+	RAII_VAR(struct ast_format *, format, NULL, ao2_cleanup);
+	RAII_VAR(struct ast_format *, format_w_attr, NULL, ao2_cleanup);
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = __PRETTY_FUNCTION__;
+		info->category = TEST_CATEGORY;
+		info->summary = "Format attribute retrieval unit test";
+		info->description =
+			"Test retrieval of format attributes";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	codec = ast_codec_get("test_core_format_codec", AST_MEDIA_TYPE_AUDIO, 8000);
+	if (!codec) {
+		ast_test_status_update(test, "Could not retrieve test_core_format_codec codec\n");
+		return AST_TEST_FAIL;
+	}
+
+	format = ast_format_create(codec);
+	if (!format) {
+		ast_test_status_update(test, "Could not create format using test_core_format_codec codec\n");
+		return AST_TEST_FAIL;
+	}
+
+	format_w_attr = ast_format_attribute_set(format, "one", "1");
+	if (!format_w_attr) {
+		ast_test_status_update(test, "Could not create format with attributes using test_core_format_codec codec\n");
+		return AST_TEST_FAIL;
+	}
+
+	if (*((int *)ast_format_attribute_get(format_w_attr, "one")) != 1) {
+		ast_test_status_update(test, "Could not retrieve valid format attribute\n");
+		return AST_TEST_FAIL;
+	}
+
+	if (ast_format_attribute_get(format_w_attr, "foo") != NULL) {
+		ast_test_status_update(test, "Retrieved invalid format attribute\n");
+		return AST_TEST_FAIL;
+	}
 
 	return AST_TEST_PASS;
 }
@@ -829,6 +893,43 @@
 	return AST_TEST_PASS;
 }
 
+AST_TEST_DEFINE(format_attribute_get_without_interface)
+{
+	RAII_VAR(struct ast_codec *, codec, NULL, ao2_cleanup);
+	RAII_VAR(struct ast_format *, format, NULL, ao2_cleanup);
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = __PRETTY_FUNCTION__;
+		info->category = TEST_CATEGORY;
+		info->summary = "Format attribute retrieval unit test";
+		info->description =
+			"Test that attribute retrieval on a format without an interface fails";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	codec = ast_codec_get("ulaw", AST_MEDIA_TYPE_AUDIO, 8000);
+	if (!codec) {
+		ast_test_status_update(test, "Could not retrieve built-in ulaw codec\n");
+		return AST_TEST_FAIL;
+	}
+
+	format = ast_format_create(codec);
+	if (!format) {
+		ast_test_status_update(test, "Could not create format using built-in codec\n");
+		return AST_TEST_FAIL;
+	}
+
+	if (ast_format_attribute_get(format, "bees") != NULL) {
+		ast_test_status_update(test, "Successfully retrieved an attribute on a format without an interface\n");
+		return AST_TEST_FAIL;
+	}
+
+	return AST_TEST_PASS;
+}
+
 AST_TEST_DEFINE(format_parse_sdp_fmtp_without_interface)
 {
 	RAII_VAR(struct ast_codec *, codec, NULL, ao2_cleanup);
@@ -925,6 +1026,7 @@
 {
 	AST_TEST_UNREGISTER(format_create);
 	AST_TEST_UNREGISTER(format_create_attr);
+	AST_TEST_UNREGISTER(format_retrieve_attr);
 	AST_TEST_UNREGISTER(format_clone);
 	AST_TEST_UNREGISTER(format_cmp_same_codec);
 	AST_TEST_UNREGISTER(format_attr_cmp_same_codec);
@@ -934,6 +1036,7 @@
 	AST_TEST_UNREGISTER(format_joint_different_codec);
 	AST_TEST_UNREGISTER(format_copy);
 	AST_TEST_UNREGISTER(format_attribute_set_without_interface);
+	AST_TEST_UNREGISTER(format_attribute_get_without_interface);
 	AST_TEST_UNREGISTER(format_parse_sdp_fmtp_without_interface);
 	AST_TEST_UNREGISTER(format_parse_and_generate_sdp_fmtp);
 
@@ -955,6 +1058,7 @@
 
 	AST_TEST_REGISTER(format_create);
 	AST_TEST_REGISTER(format_create_attr);
+	AST_TEST_REGISTER(format_retrieve_attr);
 	AST_TEST_REGISTER(format_clone);
 	AST_TEST_REGISTER(format_cmp_same_codec);
 	AST_TEST_REGISTER(format_attr_cmp_same_codec);
@@ -964,6 +1068,7 @@
 	AST_TEST_REGISTER(format_joint_different_codec);
 	AST_TEST_REGISTER(format_copy);
 	AST_TEST_REGISTER(format_attribute_set_without_interface);
+	AST_TEST_REGISTER(format_attribute_get_without_interface);
 	AST_TEST_REGISTER(format_parse_sdp_fmtp_without_interface);
 	AST_TEST_REGISTER(format_parse_and_generate_sdp_fmtp);
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ieec76883dfb46ecd7aff3dc81a52c81f4dc1b9e3
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>



More information about the asterisk-code-review mailing list