diff mbox

libosmocore[master]: Add helper functions for AMR codec

Message ID gerrit.1464193103196.Ia217679a07d3fbc970f435e20f6eac33d34bd597@gerrit.osmocom.org
State New
Headers show

Commit Message

gerrit-no-reply@lists.osmocom.org May 25, 2016, 4:18 p.m. UTC
Review at  https://gerrit.osmocom.org/118

Add helper functions for AMR codec

* add functions to encode/decode various codec paramters from RTP payload with
  AMR frame according to RFC 4867
* those functions are extended version based on code from osmo-bts'
  amr.c by Andreas Eversberg
* add corresponding enum types and strings for logging
* add regression tests

It's useful both to replace manual parsing in osmo-bts with fuctions
covered by test suite and as a debugging helpers for issues related to
AMR.

Change-Id: Ia217679a07d3fbc970f435e20f6eac33d34bd597
Related: OS#1562
---
M .gitignore
M include/osmocom/codec/codec.h
M src/codec/gsm690.c
M tests/Makefile.am
A tests/codec/codec_test.c
A tests/codec/codec_test.ok
M tests/testsuite.at
7 files changed, 204 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/18/118/1

Comments

gerrit-no-reply@lists.osmocom.org May 30, 2016, 9:26 p.m. UTC | #1
Patch Set 4: Code-Review-1

(1 comment)

https://gerrit.osmocom.org/#/c/118/4/tests/codec/codec_test.c
File tests/codec/codec_test.c:

Line 35: 	int rc = osmo_amr_rtp_dec(t, len, &cmr, &cmi, &ft, &bfi, &sti);
What I really like are roud-trip tests. Decode, encode and check if they are the same. Could you add one here?
gerrit-no-reply@lists.osmocom.org May 31, 2016, 9:29 a.m. UTC | #2
Patch Set 5:

(1 comment)

https://gerrit.osmocom.org/#/c/118/5/tests/codec/codec_test.c
File tests/codec/codec_test.c:

Line 53: 			enum osmo_amr_quality _bfi)
Okay, that is one valid approach for a round-trip test. Have you considered doing decode, encode and then memcmp with the sid_update/sid_first?
gerrit-no-reply@lists.osmocom.org May 31, 2016, 10:11 a.m. UTC | #3
Patch Set 6: Code-Review+2

(1 comment)

https://gerrit.osmocom.org/#/c/118/6/tests/codec/codec_test.c
File tests/codec/codec_test.c:

Line 54: 	printf("[%d] encode [%d]\n", rc, memcmp(tmp, t, SID_LEN));
Okay. I will accept this to not have you spend more time on it. One thing both Jacob and me learned the hardway is:

* Capturing textual output is good to catch accidental changes
* But it is not replacing OSMO_ASSERT. So e.g. in future tests use OSMO_ASSERT on that memcmp.

I am accepting this change now.
gerrit-no-reply@lists.osmocom.org May 31, 2016, 10:19 a.m. UTC | #4
Patch Set 7:

Could you clarify when OSMO_ASSERT would catch the failure but textual output wouldn't?
gerrit-no-reply@lists.osmocom.org May 31, 2016, 10:21 a.m. UTC | #5
Patch Set 7:

Of course textual output would catch a change in return value of memcmp. The issue we had is that as part of a bigger change so much output was changed that something that should have been catched by an OSMO_ASSERT was not.

So my point is:
 * Use OSMO_ASSERT for a strong post-condition/result check
 * Have the output as a safety net for side-effects and accidental changes
diff mbox

Patch

diff --git a/.gitignore b/.gitignore
index 03ce379..70e2016 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,6 +80,7 @@ 
 tests/ringbuf/ringbuf_test
 tests/strrb/strrb_test
 tests/vty/vty_test
+tests/codec/codec_test
 tests/gb/gprs_bssgp_test
 tests/smscb/gsm0341_test
 tests/bitvec/bitvec_test
diff --git a/include/osmocom/codec/codec.h b/include/osmocom/codec/codec.h
index d126e0f..b7bcc78 100644
--- a/include/osmocom/codec/codec.h
+++ b/include/osmocom/codec/codec.h
@@ -2,6 +2,8 @@ 
 
 #include <stdint.h>
 
+#include <osmocom/core/utils.h>
+
 extern const uint16_t gsm610_bitorder[];	/* FR */
 extern const uint16_t gsm620_unvoiced_bitorder[]; /* HR unvoiced */
 extern const uint16_t gsm620_voiced_bitorder[];   /* HR voiced */
@@ -15,3 +17,32 @@ 
 extern const uint16_t gsm690_5_9_bitorder[];	/* AMR  5.9  kbits */
 extern const uint16_t gsm690_5_15_bitorder[];	/* AMR  5.15 kbits */
 extern const uint16_t gsm690_4_75_bitorder[];	/* AMR  4.75 kbits */
+
+extern const struct value_string osmo_amr_type_names[];
+
+enum osmo_amr_type {
+       AMR_4_75 = 0,
+       AMR_5_15 = 1,
+       AMR_5_90 = 2,
+       AMR_6_70 = 3,
+       AMR_7_40 = 4,
+       AMR_7_95 = 5,
+       AMR_10_2 = 6,
+       AMR_12_2 = 7,
+       AMR_SID = 8,
+       AMR_GSM_EFR_SID = 9,
+       AMR_TDMA_EFR_SID = 10,
+       AMR_PDC_EFR_SID = 11,
+       AMR_NO_DATA = 15,
+};
+
+enum osmo_amr_quality {
+       AMR_BAD = 0,
+       AMR_GOOD = 1
+};
+
+int osmo_amr_rtp_enc(uint8_t *payload, uint8_t cmr, enum osmo_amr_type ft,
+		     enum osmo_amr_quality bfi);
+int osmo_amr_rtp_dec(const uint8_t *payload, int payload_len, uint8_t *cmr,
+		     int8_t *cmi, enum osmo_amr_type *ft,
+		     enum osmo_amr_quality *bfi, int8_t *sti);
diff --git a/src/codec/gsm690.c b/src/codec/gsm690.c
index fdf3302..3a509bd 100644
--- a/src/codec/gsm690.c
+++ b/src/codec/gsm690.c
@@ -22,7 +22,10 @@ 
  */
 
 #include <stdint.h>
+#include <errno.h>
 
+#include <osmocom/core/utils.h>
+#include <osmocom/codec/codec.h>
 /*
  * These table map between the raw encoder parameter output and
  * the format used before channel coding. Both in GSM and in various
@@ -208,3 +211,104 @@ 
 	 88,  90,  91,  34,  55,  68,  89,  37,  58,  71,
 	 92,  31,  52,  65,  86,
 };
+
+static const uint8_t amr_len_by_ft[16] = {
+	12, 13, 15, 17, 19, 20, 26, 31, 0,  0,  0,  0,  0,  0,  0,  0
+};
+
+const struct value_string osmo_amr_type_names[] = {
+	{ AMR_4_75,		"AMR 4,75 kbits/s" },
+	{ AMR_5_15,		"AMR 5,15 kbit/s" },
+	{ AMR_5_90,		"AMR 5,90 kbit/s" },
+	{ AMR_6_70,		"AMR 6,70 kbit/s (PDC-EFR)" },
+	{ AMR_7_40,		"AMR 7,40 kbit/s (TDMA-EFR)" },
+	{ AMR_7_95,		"AMR 7,95 kbit/s" },
+	{ AMR_10_2,		"AMR 10,2 kbit/s" },
+	{ AMR_12_2,		"AMR 12,2 kbit/s (GSM-EFR)" },
+	{ AMR_SID,		"AMR SID" },
+	{ AMR_GSM_EFR_SID,	"GSM-EFR SID" },
+	{ AMR_TDMA_EFR_SID,	"TDMA-EFR SID" },
+	{ AMR_PDC_EFR_SID,	"PDC-EFR SID" },
+	{ AMR_NO_DATA,		"No Data/NA" },
+	{ 0,			NULL },
+};
+
+/*! \brief Decode various AMR parameters from RTP payload (RFC 4867) acording to
+ *         3GPP TS 26.101
+ *  \param[in] rtppayload Payload from RTP packet
+ *  \param[in] payload_len length of rtppayload
+ *  \param[out] cmr AMR Codec Mode Request, not filled if NULL
+ *  \param[out] cmi AMR Codec Mode Indicator, -1 if not applicable for this type,
+ *              not filled if NULL
+ *  \param[out] ft AMR Frame Type, not filled if NULL
+ *  \param[out] bfi AMR Bad Frame Indicator, not filled if NULL
+ *  \param[out] sti AMR SID Type Indicator, -1 if not applicable for this type,
+ *              not filled if NULL
+ *  \returns length of AMR data or negative value on error
+ */
+int osmo_amr_rtp_dec(const uint8_t *rtppayload, int payload_len, uint8_t *cmr,
+		     int8_t *cmi, enum osmo_amr_type *ft,
+		     enum osmo_amr_quality *bfi, int8_t *sti)
+{
+	/* RFC 4867 § 4.4.2 ToC - compound payloads are not supported: F = 0 */
+	uint8_t type = (rtppayload[1] >> 3) & 0xf;
+
+	/* compound payloads are not supported */
+	if (rtppayload[1] >> 7)
+		return -ENOTSUP;
+
+	if (payload_len - 2 < amr_len_by_ft[type])
+		return -ENOTSUP;
+
+	if (payload_len < 2)
+		return -EINVAL;
+
+	if (ft)
+		*ft = type;
+
+	if (cmr)
+		*cmr = rtppayload[0] >> 4;
+
+	if (bfi)
+		*bfi = (rtppayload[1] >> 2) & 1;
+
+	/* Table 6 in 3GPP TS 26.101 */
+	if (cmi)
+		*cmi = (type == AMR_SID) ? ((rtppayload[6] >> 1) & 7) : -1;
+
+	if (sti)
+		*sti = (type == AMR_SID) ? (rtppayload[6] & 0x10) : -1;
+
+	return 2 + amr_len_by_ft[type];
+}
+
+/*! \brief Encode various AMR parameters from RTP payload (RFC 4867)
+ *  \param[out] payload Payload for RTP packet, contains speech data (if any)
+ *              except for have 2 first bytes where header will be built
+ *  \param[in] cmr AMR codec Mode Request
+ *  \param[in] ft AMR Frame Type
+ *  \param[in] bfi AMR Bad Frame Indicator
+ *  \returns length of AMR data (header + ToC + speech data) or negative value
+ *           on error
+ *
+ *  Note: only octet-aligned mode is supported so the header occupies 2 full
+ *  bytes. Optional interleaving header is not supported.
+ */
+int osmo_amr_rtp_enc(uint8_t *payload, uint8_t cmr, enum osmo_amr_type ft,
+		     enum osmo_amr_quality bfi)
+{
+	if (cmr > 15)
+		return -EINVAL;
+
+	if (ft > 15)
+		return -EMEDIUMTYPE;
+
+	/* RFC 4867 § 4.3.1 payload header */
+	payload[0] = cmr << 4;
+
+	/* RFC 4867 § 4.4.2 ToC - compound payloads are not supported: F = 0 */
+	payload[1] = (((uint8_t)ft) << 3) | (((uint8_t)bfi) << 2);
+
+	/* speech data */
+	return 2 + amr_len_by_ft[ft];
+}
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6c9929b..55aaa07 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -8,7 +8,7 @@ 
                  gsm0808/gsm0808_test gsm0408/gsm0408_test		\
 		 gb/bssgp_fc_test gb/gprs_bssgp_test gb/gprs_ns_test	\
 		 gprs/gprs_test	kasumi/kasumi_test			\
-		 logging/logging_test fr/fr_test			\
+		 logging/logging_test fr/fr_test codec/codec_test	\
 		 loggingrb/loggingrb_test strrb/strrb_test              \
 		 vty/vty_test comp128/comp128_test utils/utils_test	\
 		 smscb/gsm0341_test stats/stats_test			\
@@ -97,6 +97,9 @@ 
 fr_fr_test_SOURCES = fr/fr_test.c
 fr_fr_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gb/libosmogb.la $(LIBRARY_DL)
 
+codec_codec_test_SOURCES = codec/codec_test.c
+codec_codec_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/codec/libosmocodec.la
+
 loggingrb_loggingrb_test_SOURCES = loggingrb/loggingrb_test.c
 loggingrb_loggingrb_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/vty/libosmovty.la
 
@@ -141,7 +144,7 @@ 
              gsm0808/gsm0808_test.ok gb/bssgp_fc_tests.err		\
              gb/bssgp_fc_tests.ok gb/bssgp_fc_tests.sh			\
              gb/gprs_bssgp_test.ok gb/gprs_ns_test.ok			\
-             gprs/gprs_test.ok kasumi/kasumi_test.ok			\
+             gprs/gprs_test.ok kasumi/kasumi_test.ok codec/codec_test.ok \
              msgfile/msgfile_test.ok msgfile/msgconfig.cfg		\
              logging/logging_test.ok logging/logging_test.err		\
              fr/fr_test.ok loggingrb/logging_test.ok			\
diff --git a/tests/codec/codec_test.c b/tests/codec/codec_test.c
new file mode 100644
index 0000000..854717b
--- /dev/null
+++ b/tests/codec/codec_test.c
@@ -0,0 +1,54 @@ 
+/*
+ * (C) 2016 by Sysmocom s.f.m.c. GmbH
+ * All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <osmocom/core/utils.h>
+#include <osmocom/codec/codec.h>
+
+const uint8_t sid_update[] = {0x20, 0x44, 0x29, 0xc2, 0x92, 0x91, 0xf4};
+const uint8_t sid_first[] = {0x20, 0x44, 0x00, 0x00, 0x00, 0x00, 0x04};
+
+static void test_sid_dec(const uint8_t *t, size_t len)
+{
+	uint8_t cmr;
+	enum osmo_amr_type ft;
+	enum osmo_amr_quality bfi;
+	int8_t sti, cmi;
+	int rc = osmo_amr_rtp_dec(t, len, &cmr, &cmi, &ft, &bfi, &sti);
+	printf("[%d] decode RTP %s%s: FT %s, CMR %s, CMI is %d, SID type %s\n",
+	       rc, osmo_hexdump(t, len), bfi == AMR_GOOD ? "OK" : "BAD",
+	       get_value_string(osmo_amr_type_names, ft),
+	       get_value_string(osmo_amr_type_names, cmr),
+	       cmi, sti ? "UPDATE" : "FIRST");
+	if (sti == -1)
+		printf("FAIL: incompatible STI for SID\n");
+}
+
+int main(int argc, char **argv)
+{
+	printf("AMR RTP payload decoder test:\n");
+	test_sid_dec(sid_first, 7);
+	test_sid_dec(sid_update, 7);
+
+	return 0;
+}
+
+
diff --git a/tests/codec/codec_test.ok b/tests/codec/codec_test.ok
new file mode 100644
index 0000000..f3da636
--- /dev/null
+++ b/tests/codec/codec_test.ok
@@ -0,0 +1,3 @@ 
+AMR RTP payload decoder test:
+[2] decode RTP 20 44 00 00 00 00 04 OK: FT AMR SID, CMR AMR 5,90 kbit/s, CMI is 2, SID type FIRST
+[2] decode RTP 20 44 29 c2 92 91 f4 OK: FT AMR SID, CMR AMR 5,90 kbit/s, CMI is 2, SID type UPDATE
diff --git a/tests/testsuite.at b/tests/testsuite.at
index aa269af..d49f7ff 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -115,12 +115,17 @@ 
 AT_CHECK([$abs_top_builddir/tests/logging/logging_test], [0], [expout], [experr])
 AT_CLEANUP
 
+AT_SETUP([codec])
+AT_KEYWORDS([codec])
+cat $abs_srcdir/codec/codec_test.ok > expout
+AT_CHECK([$abs_top_builddir/tests/codec/codec_test], [0], [expout], [ignore])
+AT_CLEANUP
+
 AT_SETUP([fr])
 AT_KEYWORDS([fr])
 cat $abs_srcdir/fr/fr_test.ok > expout
 cat $abs_srcdir/fr/fr_test.err > experr
 AT_CHECK([$abs_top_builddir/tests/fr/fr_test], [0], [expout], [experr])
-
 AT_CLEANUP
 
 AT_SETUP([loggingrb])