laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/26788 ) Change subject: amr: Fix length check in bwe<->iuup converters ...................................................................... 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(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved 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 https://gerrit.osmocom.org/c/libosmo-netif/+/26788 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0552d727585886d25f613e64ca815fb6dcd53f25 Gerrit-Change-Number: 26788 Gerrit-PatchSet: 2 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: lynxis lazus &lt;lynxis(a)fe80.eu&gt; Gerrit-MessageType: merged