laforge submitted this change.

View Change

Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved
amr: Fix length check in bwe<->iuup converters

The check was wrong for format types containing extra bits not aligned
to byte boundaries, such as FT7 (AMR Code 12.20, 244 bits, 31 bytes).

if the source has 1-6 extra bits, they can be fit with one less byte
when shifting 10 bits to the left.

Change-Id: I0552d727585886d25f613e64ca815fb6dcd53f25
---
M src/amr.c
M tests/amr/amr_test.c
M tests/amr/amr_test.ok
3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/src/amr.c b/src/amr.c
index 2df6967..7bc24f5 100644
--- a/src/amr.c
+++ b/src/amr.c
@@ -228,8 +228,8 @@
int osmo_amr_bwe_to_iuup(uint8_t *payload, unsigned int payload_len)
{
/* The header is only valid after shifting first two bytes to OA mode */
- unsigned int i;
- unsigned int amr_speech_len;
+ unsigned int i, required_len_bits;
+ unsigned int amr_speech_len_bytes, amr_speech_len_bits;
uint8_t ft;

if (payload_len < 2)
@@ -240,16 +240,19 @@
if (!osmo_amr_ft_valid(ft))
return -1;

- amr_speech_len = osmo_amr_bytes(ft);
- if (payload_len < amr_speech_len + 2)
+ amr_speech_len_bits = osmo_amr_bits(ft);
+ amr_speech_len_bytes = osmo_amr_bytes(ft);
+
+ required_len_bits = amr_speech_len_bits + 10; /* shift of 10 bits */
+ if (payload_len < (required_len_bits + 7)/8)
return -1;

- for (i = 0; i < amr_speech_len; i++) {
+ for (i = 0; i < amr_speech_len_bytes; i++) {
/* we have to shift the payload by 10 bits to get only the Class A, B, C bits */
payload[i] = (payload[i + 1] << 2) | ((payload[i + 2]) >> 6);
}

- return amr_speech_len;
+ return amr_speech_len_bytes;
}

/*! Convert an AMR frame from IuuP/IuFP payload to bandwith-efficient mode.
@@ -263,9 +266,15 @@
unsigned int payload_maxlen)
{
/* shift all bits by 10 */
- int i;
+ unsigned int i, required_len_bits, required_len_bytes;

- if (payload_maxlen < payload_len + 2)
+ int ft = osmo_amr_bytes_to_ft(payload_len);
+ if (ft < 0)
+ return ft;
+
+ required_len_bits = osmo_amr_bits(ft) + 10;
+ required_len_bytes = (required_len_bits + 7)/8;
+ if (payload_maxlen < required_len_bytes)
return -1;

i = payload_len + 1;
@@ -276,5 +285,5 @@
}
payload[i] = (payload[i - 1] >> 2);
payload[0] = 0;
- return payload_len + 2;
+ return required_len_bytes;
}
diff --git a/tests/amr/amr_test.c b/tests/amr/amr_test.c
index 802fbee..3d0d917 100644
--- a/tests/amr/amr_test.c
+++ b/tests/amr/amr_test.c
@@ -227,7 +227,7 @@

rc = osmo_amr_iuup_to_bwe(buf, len, sizeof(buf));
OSMO_ASSERT(rc > 0);
- OSMO_ASSERT(rc == len + 2);
+ OSMO_ASSERT((rc == len + 1) || (rc == len + 2));
printf(" BE: %d bytes (%s),", rc, osmo_hexdump(buf, rc));

buf[0] = (ft >> 1) & 0x07;
diff --git a/tests/amr/amr_test.ok b/tests/amr/amr_test.ok
index 7a762d9..c9cde1f 100644
--- a/tests/amr/amr_test.ok
+++ b/tests/amr/amr_test.ok
@@ -235,8 +235,8 @@


Testing conversion from IuUP to bw-efficient and inverse:
-Sample No.: 0... AMR mode: 7, IuUP: 31 bytes, BE: 33 bytes (00 1e 9a 1e e2 ca 6e 80 80 4f 4f 2e 18 e9 6b d0 81 a1 a5 86 3e d4 cd b5 41 25 a3 5d 50 59 8c 84 00 ), IuUP: 31 bytes
-Sample No.: 1... AMR mode: 7, IuUP: 31 bytes, BE: 33 bytes (00 11 20 a0 0e 5d f7 06 20 2d 5a aa c2 5d ca 29 ae ee d2 29 5f 8e bf f9 04 49 60 e2 97 39 97 f0 00 ), IuUP: 31 bytes
+Sample No.: 0... AMR mode: 7, IuUP: 31 bytes, BE: 32 bytes (00 1e 9a 1e e2 ca 6e 80 80 4f 4f 2e 18 e9 6b d0 81 a1 a5 86 3e d4 cd b5 41 25 a3 5d 50 59 8c 84 ), IuUP: 31 bytes
+Sample No.: 1... AMR mode: 7, IuUP: 31 bytes, BE: 32 bytes (00 11 20 a0 0e 5d f7 06 20 2d 5a aa c2 5d ca 29 ae ee d2 29 5f 8e bf f9 04 49 60 e2 97 39 97 f0 ), IuUP: 31 bytes
Sample No.: 2... AMR mode: 8, IuUP: 5 bytes, BE: 7 bytes (00 09 a0 f0 74 9b 80 ), IuUP: 5 bytes



To view, visit change 26788. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I0552d727585886d25f613e64ca815fb6dcd53f25
Gerrit-Change-Number: 26788
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-Reviewer: lynxis lazus <lynxis@fe80.eu>
Gerrit-MessageType: merged