[Asterisk-code-review] res pjsip: Add option to use g.726 for AAL2 packing when neg... (asterisk[13])

Kevin Harwell asteriskteam at digium.com
Fri Jun 12 17:03:58 CDT 2015


Kevin Harwell has uploaded a new change for review.

  https://gerrit.asterisk.org/651

Change subject: res_pjsip: Add option to use g.726 for AAL2 packing when negotiating g.726
......................................................................

res_pjsip: Add option to use g.726 for AAL2 packing when negotiating g.726

Some phones send g.726 audio packed for AAL2, which differs from what is
recommended by RFC 3351. If Asterisk receives audio formatted as such when
negotiating g.726 then it sounds a bit distorted. Added an option to
res_pjsip_endpoint that allows g.726 negotiated audio to be treated as g.726
for AAL2.

ASTERISK-25158 #close
Reported by: Steve Pitts

Change-Id: Ie7e21f75493d7fe53e75e12c971e72f5afa33615
---
M CHANGES
M configs/samples/pjsip.conf.sample
A contrib/ast-db-manage/config/versions/28b8e71e541f_add_g726_non_standard.py
M include/asterisk/res_pjsip.h
M res/res_pjsip.c
M res/res_pjsip/pjsip_configuration.c
M res/res_pjsip_sdp_rtp.c
7 files changed, 64 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/51/651/1

diff --git a/CHANGES b/CHANGES
index bd79dba..3413d83 100644
--- a/CHANGES
+++ b/CHANGES
@@ -17,6 +17,12 @@
  * A new ContactStatus event has been added that reflects res_pjsip contact
    lifecycle changes:  Created, Removed, Reachable, Unreachable, Unknown.
 
+res_pjsip
+------------------
+* A new 'g726_non_standard' endpoint option has been added that, when set to
+  'yes' and g.726 audio is negotiated, changes the packing order for the g726
+  codec to g726 for AAL2.
+
 ------------------------------------------------------------------------------
 --- Functionality changes from Asterisk 13.3.0 to Asterisk 13.4.0 ------------
 ------------------------------------------------------------------------------
diff --git a/configs/samples/pjsip.conf.sample b/configs/samples/pjsip.conf.sample
index ef25a57..239efda 100644
--- a/configs/samples/pjsip.conf.sample
+++ b/configs/samples/pjsip.conf.sample
@@ -656,6 +656,11 @@
                         ; "no")
 ;media_encryption_optimistic=no ; Use encryption if possible but don't fail the call
 								; if not possible.
+;g726_non_standard=no   ; When set to "yes" and an endpoint negotiates g.726
+                        ; audio then g.726 for AAL2 packing order is used contrary
+                        ; to what is recommended in RFC3551. Note, 'g726aal2' also
+                        ; needs to be specified in the codec allow list
+                        ; (default: "no")
 ;inband_progress=no     ; Determines whether chan_pjsip will indicate ringing
                         ; using inband progress (default: "no")
 ;call_group=    ; The numeric pickup groups for a channel (default: "")
diff --git a/contrib/ast-db-manage/config/versions/28b8e71e541f_add_g726_non_standard.py b/contrib/ast-db-manage/config/versions/28b8e71e541f_add_g726_non_standard.py
new file mode 100644
index 0000000..ad36bd9
--- /dev/null
+++ b/contrib/ast-db-manage/config/versions/28b8e71e541f_add_g726_non_standard.py
@@ -0,0 +1,30 @@
+"""add g726_non_standard
+
+Revision ID: 28b8e71e541f
+Revises: a541e0b5e89
+Create Date: 2015-06-12 16:07:08.609628
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '28b8e71e541f'
+down_revision = 'a541e0b5e89'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects.postgresql import ENUM
+
+YESNO_NAME = 'yesno_values'
+YESNO_VALUES = ['yes', 'no']
+
+def upgrade():
+    ############################# Enums ##############################
+
+    # yesno_values have already been created, so use postgres enum object
+    # type to get around "already created" issue - works okay with mysql
+    yesno_values = ENUM(*YESNO_VALUES, name=YESNO_NAME, create_type=False)
+    op.add_column('ps_endpoints', sa.Column('g726_non_standard', yesno_values))
+
+
+def downgrade():
+    op.drop_column('ps_endpoints', 'g726_non_standard')
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 0447eaa..d4ab164 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -555,6 +555,8 @@
 	unsigned int tos_video;
 	/*! Priority for video streams */
 	unsigned int cos_video;
+	/*! Is g.726 packed in a non standard way */
+	unsigned int g726_non_standard;
 };
 
 /*!
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index ff18a1d..b698165 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -471,6 +471,15 @@
 						set to <literal>sdes</literal> or <literal>dtls</literal>.
 					</para></description>
 				</configOption>
+				<configOption name="g726_non_standard" default="no">
+					<synopsis>Specify to use g.726 for AAL2 packing order when negotiating g.726</synopsis>
+					<description><para>
+                                                When set to "yes" and an endpoint negotiates g.726 audio then use g.726 for AAL2
+                                                packing order instead of what is recommended by RFC3551. Since this essentially
+                                                replaces the underlying 'g726' codec with 'g726aal2' then 'g726aal2' needs to be
+                                                specified in the endpoint's allowed codec list.
+					</para></description>
+				</configOption>
 				<configOption name="inband_progress" default="no">
 					<synopsis>Determines whether chan_pjsip will indicate ringing using inband
 					    progress.</synopsis>
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index 34bb692..9a383a2 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -1922,6 +1922,7 @@
 	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "dtls_fingerprint", "", dtls_handler, dtlsfingerprint_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "srtp_tag_32", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, media.rtp.srtp_tag_32));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "media_encryption_optimistic", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, media.rtp.encryption_optimistic));
+	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "g726_non_standard", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, media.g726_non_standard));
 	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "redirect_method", "user", redirect_handler, NULL, NULL, 0, 0);
 	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "set_var", "", set_var_handler, set_var_to_str, set_var_to_vl, 0, 0);
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "message_context", "", OPT_STRINGFIELD_T, 1, STRFLDSET(struct ast_sip_endpoint, message_context));
diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index 1bd3b0c..fb70dd3 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -155,6 +155,8 @@
 	char name[256];
 	char media[20];
 	char fmt_param[256];
+	enum ast_rtp_options options = session->endpoint->media.g726_non_standard ?
+		AST_RTP_OPT_G726_NONSTANDARD : 0;
 
 	ast_rtp_codecs_payloads_initialize(codecs);
 
@@ -176,9 +178,10 @@
                 if (strcmp(name,"telephone-event") == 0) {
                         tel_event++;
                 }
+
 		ast_copy_pj_str(media, (pj_str_t*)&stream->desc.media, sizeof(media));
 		ast_rtp_codecs_payloads_set_rtpmap_type_rate(codecs, NULL, pj_strtoul(&stream->desc.fmt[i]),
-							     media, name, 0, rtpmap->clock_rate);
+							     media, name, options, rtpmap->clock_rate);
 		/* Look for an optional associated fmtp attribute */
 		if (!(attr = pjmedia_sdp_media_find_attr2(stream, "fmtp", &rtpmap->pt))) {
 			continue;
@@ -304,18 +307,20 @@
 	return 0;
 }
 
-static pjmedia_sdp_attr* generate_rtpmap_attr(pjmedia_sdp_media *media, pj_pool_t *pool, int rtp_code,
-					      int asterisk_format, struct ast_format *format, int code)
+static pjmedia_sdp_attr* generate_rtpmap_attr(struct ast_sip_session *session, pjmedia_sdp_media *media, pj_pool_t *pool,
+					      int rtp_code, int asterisk_format, struct ast_format *format, int code)
 {
 	pjmedia_sdp_rtpmap rtpmap;
 	pjmedia_sdp_attr *attr = NULL;
 	char tmp[64];
+	enum ast_rtp_options options = session->endpoint->media.g726_non_standard ?
+		AST_RTP_OPT_G726_NONSTANDARD : 0;
 
 	snprintf(tmp, sizeof(tmp), "%d", rtp_code);
 	pj_strdup2(pool, &media->desc.fmt[media->desc.fmt_count++], tmp);
 	rtpmap.pt = media->desc.fmt[media->desc.fmt_count - 1];
 	rtpmap.clock_rate = ast_rtp_lookup_sample_rate2(asterisk_format, format, code);
-	pj_strdup2(pool, &rtpmap.enc_name, ast_rtp_lookup_mime_subtype2(asterisk_format, format, code, 0));
+	pj_strdup2(pool, &rtpmap.enc_name, ast_rtp_lookup_mime_subtype2(asterisk_format, format, code, options));
 	rtpmap.param.slen = 0;
 	rtpmap.param.ptr = NULL;
 
@@ -1050,7 +1055,7 @@
 			continue;
 		}
 
-		if (!(attr = generate_rtpmap_attr(media, pool, rtp_code, 1, format, 0))) {
+		if (!(attr = generate_rtpmap_attr(session, media, pool, rtp_code, 1, format, 0))) {
 			ao2_ref(format, -1);
 			continue;
 		}
@@ -1075,7 +1080,7 @@
 				continue;
 			}
 
-			if (!(attr = generate_rtpmap_attr(media, pool, rtp_code, 0, NULL, index))) {
+			if (!(attr = generate_rtpmap_attr(session, media, pool, rtp_code, 0, NULL, index))) {
 				continue;
 			}
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie7e21f75493d7fe53e75e12c971e72f5afa33615
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>



More information about the asterisk-code-review mailing list