[Asterisk-code-review] res format attr g729: Add annexb=no format parameter to SDPs (asterisk[13])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: res_format_attr_g729: Add annexb=no format parameter to SDPs
......................................................................


res_format_attr_g729: Add annexb=no format parameter to SDPs

Historically, Asterisk has always specified annexb=no for the g729 format.
However, when using res_pjsip no format attribute was specified. This patch
makes it so the SDP now contains a format attribute line with annexb=no.

Note, that this means only g729a is negotiated. Even for pass through support.
According to rfc7261 the type of annex used (a or b) is dependent upon the
answerer. However, Asterisk being a back to back user agent makes this tricky
to support at this time, thus we only allow annex 'a' for now.

ASTERISK-26228 #close
patches:
  res_format_attr_g729.c submitted by Jason Parker (license 4993)

Change-Id: I76bc20cc0a01af01536e9915afef319c269c22d0
---
M channels/chan_sip.c
A res/res_format_attr_g729.c
2 files changed, 77 insertions(+), 4 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Matt Jordan: Looks good to me, approved
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index f58c19e..c0aae46 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -12981,10 +12981,7 @@
 
 	framing = ast_format_cap_get_format_framing(p->caps, format);
 
-	if (ast_format_cmp(format, ast_format_g729) == AST_FORMAT_CMP_EQUAL) {
-		/* Indicate that we don't support VAD (G.729 annex B) */
-		ast_str_append(a_buf, 0, "a=fmtp:%d annexb=no\r\n", rtp_code);
-	} else if (ast_format_cmp(format, ast_format_g723) == AST_FORMAT_CMP_EQUAL) {
+	if (ast_format_cmp(format, ast_format_g723) == AST_FORMAT_CMP_EQUAL) {
 		/* Indicate that we don't support VAD (G.723.1 annex A) */
 		ast_str_append(a_buf, 0, "a=fmtp:%d annexa=no\r\n", rtp_code);
 	} else if (ast_format_cmp(format, ast_format_g719) == AST_FORMAT_CMP_EQUAL) {
diff --git a/res/res_format_attr_g729.c b/res/res_format_attr_g729.c
new file mode 100644
index 0000000..5ba4920
--- /dev/null
+++ b/res/res_format_attr_g729.c
@@ -0,0 +1,76 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2016, Digium, Inc.
+ *
+ * Jason Parker <jparker at sangoma.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*** MODULEINFO
+	<support_level>core</support_level>
+ ***/
+
+#include "asterisk.h"
+
+ASTERISK_REGISTER_FILE()
+
+#include "asterisk/module.h"
+#include "asterisk/format.h"
+
+/* Destroy is a required callback and must exist */
+static void g729_destroy(struct ast_format *format)
+{
+}
+
+/* Clone is a required callback and must exist */
+static int g729_clone(const struct ast_format *src, struct ast_format *dst)
+{
+	return 0;
+}
+
+static void g729_generate_sdp_fmtp(const struct ast_format *format, unsigned int payload, struct ast_str **str)
+{
+	/*
+	 * According to the rfc the joint annexb format parameter should be set to 'yes'
+	 * or 'no' based on the answerer (rfc7261 - 3.3). However, Asterisk being a B2BUA
+	 * makes things tricky. So for now Asterisk will set annexb=no.
+	 */
+	ast_str_append(str, 0, "a=fmtp:%u annexb=no\r\n", payload);
+}
+
+static struct ast_format_interface g729_interface = {
+	.format_destroy = g729_destroy,
+	.format_clone = g729_clone,
+	.format_generate_sdp_fmtp = g729_generate_sdp_fmtp,
+};
+
+static int load_module(void)
+{
+	if (ast_format_interface_register("g729", &g729_interface)) {
+		return AST_MODULE_LOAD_DECLINE;
+	}
+
+	return AST_MODULE_LOAD_SUCCESS;
+}
+
+static int unload_module(void)
+{
+	return 0;
+}
+
+AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_LOAD_ORDER, "G.729 Format Attribute Module",
+	.support_level = AST_MODULE_SUPPORT_CORE,
+	.load = load_module,
+	.unload = unload_module,
+	.load_pri = AST_MODPRI_CHANNEL_DEPEND,
+);

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I76bc20cc0a01af01536e9915afef319c269c22d0
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Kevin Harwell <kharwell 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: Matt Jordan <mjordan at digium.com>