Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/18265 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 7 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: ipse Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Sat, 16 May 2020 20:21:42 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
laforge has submitted this change. (
https://gerrit.osmocom.org/c/libosmo-netif/+/18265 )
Change subject: amr: Fix OA<->BWE conversion.
..
amr: Fix OA<->BWE conversion.
Size of a single AMR frame doesn't always shrink by a byte when
converted from octet-aligned to bandwidth-efficient mode. It does
shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for
AMR modes 0, 1, 5, and SID frames because we only remove 6 bits.
So old code generated truncated AMR packets for those AMR modes.
This patch fixes the length calculation by properly counting bits.
Proper bit counting is also bringing us one small step closer to
properly handlig multi-frame AMR packets.
Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70
---
M include/osmocom/netif/amr.h
M src/amr.c
M tests/amr/amr_test.c
M tests/amr/amr_test.ok
4 files changed, 164 insertions(+), 83 deletions(-)
Approvals:
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/include/osmocom/netif/amr.h b/include/osmocom/netif/amr.h
index 6e37c99..c5a8e28 100644
--- a/include/osmocom/netif/amr.h
+++ b/include/osmocom/netif/amr.h
@@ -78,20 +78,34 @@
#define AMR_FT_SID 8 /* SID */
#define AMR_FT_MAX 9
-/* AMR voice frame length (in bytes, rounded),
- * See also RFC 3267, chapter 3.6 */
-#define AMR_FT_0_LEN 12 /* 4.75 */
-#define AMR_FT_1_LEN 13 /* 5.15 */
-#define AMR_FT_2_LEN 15 /* 5.90 */
-#define AMR_FT_3_LEN 17 /* 6.70 */
-#define AMR_FT_4_LEN 19 /* 7.40 */
-#define AMR_FT_5_LEN 20 /* 7.95 */
-#define AMR_FT_6_LEN 26 /* 10.2 */
-#define AMR_FT_7_LEN 31 /* 12.2 */
-#define AMR_FT_SID_LEN 5 /* SID */
-
-/* NOTE: the above constant refers to the length of one AMR speech frame-block,
+/* AMR voice frame length (in bits).
+ * See also RFC 3267, chapter 3.6.
+ *
+ * NOTE: These constants refer to the length of one AMR speech frame-block,
* not counting CMR, TOC. */
+#define AMR_FT_0_LEN_BITS 95 /* 4.75 */
+#define AMR_FT_1_LEN_BITS 103 /* 5.15 */
+#define AMR_FT_2_LEN_BITS 118 /* 5.90 */
+#define AMR_FT_3_LEN_BITS 134 /* 6.70 */
+#define AMR_FT_4_LEN_BITS 148 /* 7.40 */
+#define AMR_FT_5_LEN_BITS 159 /* 7.95 */
+#define AMR_FT_6_LEN_BITS 204 /* 10.2 */
+#define AMR_FT_7_LEN_BITS 244 /* 12.2 */
+#define AMR_FT_SID_LEN_BITS39 /* SID */
+
+/* AMR voice frame length (in bytes, rounded).
+ *
+ * NOTE: These constants refer to the length of one AMR speech frame-block,
+ * not counting CMR, TOC. */
+#define AMR_FT_0_LEN ((AMR_FT_0_LEN_BITS+7)/8) /* 4.75 */
+#define AMR_FT_1_LEN ((AMR_FT_1_LEN_BITS+7)/8) /* 5.15 */
+#define AMR_FT_2_LEN ((AMR_FT_2_LEN_BITS+7)/8) /* 5.90 */
+#define AMR_FT_3_LEN ((AMR_FT_3_LEN_BITS+7)/8) /* 6.70 */
+#define AMR_FT_4_LEN ((AMR_FT_4_LEN_BITS+7)/8) /* 7.40 */
+#define AMR_FT_5_LEN ((AMR_FT_5_LEN_BITS+7)/8) /* 7.95 */
+#define AMR_FT_6_LEN ((AMR_FT_6_LEN_BITS+7)/8) /* 10.2 */
+#define AMR_FT_7_LEN ((AMR_FT_7_LEN_BITS+7)/8) /* 12.2 */
+#define AMR_FT_SID_LEN ((AMR_FT_SID_LEN_BITS+7)/8) /* SID */
int osmo_amr_ft_valid(uint8_t amr_ft);
size_t osmo_amr_bytes(uint8_t amr_cmr);
diff --git a/src/amr.c b/src/amr.c
index 5609c46..e4c7bb5 100644
--- a/src/amr.c
+++ b/src/amr.c
@@ -29,6 +29,18 @@
* 7 12.20 24431
*/
+static size_t amr_ft_to_bits[AMR_FT_MAX] = {
+ [AMR_FT_0] = AMR_FT_0_LEN_BITS,
+ [AMR_FT_1] = AMR_FT_1_LEN_BITS,
+ [AMR_FT_2] = AMR_FT_2_LEN_BITS,
+ [AMR_FT_3] = AMR_FT_3_LEN_BITS,
+ [AMR_FT_4] = AMR_FT_4_LEN_BITS,
+ [AMR_FT_5] = AMR_FT_5_LEN_BITS,
+ [AMR_FT_6] = AMR_FT_6_LEN_BITS,
+ [AMR_FT_7] = AMR_FT_7_LEN_BITS,
+ [AMR_FT_SID]= AMR_FT_SID_LEN_BITS,
+};
+
static size_t amr_ft_to_bytes[AMR_FT_MAX] = {
[AMR_FT_0] = AMR_FT_0_LEN,
[AMR_FT_1] = AMR_FT_1_LEN,
@@ -41,6 +53,11 @@
[AMR_FT_SID]= AMR_FT_SID_LEN,
};
+size_t osmo_amr_bits(uint8_t amr_ft)
+{
+ return amr_ft_to_bits[amr_ft];
+}
+
size_t osmo_amr_bytes(uint8_t amr_ft)
{
return amr_ft_to_bytes[amr_ft];
@@ -119,8 +136,10 @@
int osmo_amr_oa_to_bwe(uint8_t *payload, unsigned int payload_len)
{
struct amr_hdr *oa_hdr = (struct amr_hdr *)payload;
+ unsigned int ft = oa_hdr->ft;
unsigned int frame_len = payload_len - sizeof(struct amr_hdr);
unsigned int i;
+ int bwe_payload_len;
/* This implementation is not capable to handle multi-frame
* packets, so we need to make sure that the frame we operate on
@@ -128,6 +147,10 @@
if (oa_hdr->f != 0)
return -1;
+ /* Check for valid FT (AMR mode) value */
+ if (!osmo_amr_ft_valid(oa_hdr->ft))
+
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/18265 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 7 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: ipse Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Fri, 15 May 2020 12:34:53 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
ipse has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/18265 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 7: > Patch Set 6: Code-Review+1 > > SO the outcome is: Please build everything with --enable-sanitize when > testing stuff :) Yes, good point. Meantime, I've updated the test to add an invalid FT case in the other direction of the conversion. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 7 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: ipse Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Fri, 15 May 2020 12:32:43 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
Hello pespin, neels, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 to look at the new patch set (#7). Change subject: amr: Fix OA<->BWE conversion. .. amr: Fix OA<->BWE conversion. Size of a single AMR frame doesn't always shrink by a byte when converted from octet-aligned to bandwidth-efficient mode. It does shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for AMR modes 0, 1, 5, and SID frames because we only remove 6 bits. So old code generated truncated AMR packets for those AMR modes. This patch fixes the length calculation by properly counting bits. Proper bit counting is also bringing us one small step closer to properly handlig multi-frame AMR packets. Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 --- M include/osmocom/netif/amr.h M src/amr.c M tests/amr/amr_test.c M tests/amr/amr_test.ok 4 files changed, 164 insertions(+), 83 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/65/18265/7 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 7 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: ipse Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-MessageType: newpatchset
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/18265 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 6: Code-Review+1 SO the outcome is: Please build everything with --enable-sanitize when testing stuff :) -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 6 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: ipse Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Fri, 15 May 2020 11:19:27 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
ipse has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/18265 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 6: Apparently, there is a function to check FT validity - I've updated the patch to use it instead of the manually coding the condition. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 6 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: ipse Gerrit-Reviewer: neels Gerrit-Comment-Date: Fri, 15 May 2020 08:31:28 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
Hello neels, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 to look at the new patch set (#6). Change subject: amr: Fix OA<->BWE conversion. .. amr: Fix OA<->BWE conversion. Size of a single AMR frame doesn't always shrink by a byte when converted from octet-aligned to bandwidth-efficient mode. It does shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for AMR modes 0, 1, 5, and SID frames because we only remove 6 bits. So old code generated truncated AMR packets for those AMR modes. This patch fixes the length calculation by properly counting bits. Proper bit counting is also bringing us one small step closer to properly handlig multi-frame AMR packets. Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 --- M include/osmocom/netif/amr.h M src/amr.c M tests/amr/amr_test.c M tests/amr/amr_test.ok 4 files changed, 151 insertions(+), 75 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/65/18265/6 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 6 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-MessageType: newpatchset
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/18265 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 5: Code-Review+1 I reverted the previous version of this patch because it had a bug, now it has a few changes, and I don't feel solid enough to +2 right away. Can another reviewer give it another look plz... -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 5 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Comment-Date: Fri, 15 May 2020 00:13:19 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 to look at the new patch set (#5). Change subject: amr: Fix OA<->BWE conversion. .. amr: Fix OA<->BWE conversion. Size of a single AMR frame doesn't always shrink by a byte when converted from octet-aligned to bandwidth-efficient mode. It does shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for AMR modes 0, 1, 5, and SID frames because we only remove 6 bits. So old code generated truncated AMR packets for those AMR modes. This patch fixes the length calculation by properly counting bits. Proper bit counting is also bringing us one small step closer to properly handlig multi-frame AMR packets. Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 --- M include/osmocom/netif/amr.h M src/amr.c M tests/amr/amr_test.c M tests/amr/amr_test.ok 4 files changed, 151 insertions(+), 75 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/65/18265/5 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 5 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 to look at the new patch set (#3). Change subject: amr: Fix OA<->BWE conversion. .. amr: Fix OA<->BWE conversion. Size of a single AMR frame doesn't always shrink by a byte when converted from octet-aligned to bandwidth-efficient mode. It does shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for AMR modes 0, 1, 5, and SID frames because we only remove 6 bits. So old code generated truncated AMR packets for those AMR modes. This patch fixes the length calculation by properly counting bits. Proper bit counting is also bringing us one small step closer to properly handlig multi-frame AMR packets. Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 --- M include/osmocom/netif/amr.h M src/amr.c M tests/amr/amr_test.c M tests/amr/amr_test.ok 4 files changed, 151 insertions(+), 75 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/65/18265/3 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 3 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
neels has uploaded a new patch set (#4) to the change originally created by ipse. ( https://gerrit.osmocom.org/c/libosmo-netif/+/18265 ) Change subject: amr: Fix OA<->BWE conversion. .. amr: Fix OA<->BWE conversion. Size of a single AMR frame doesn't always shrink by a byte when converted from octet-aligned to bandwidth-efficient mode. It does shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for AMR modes 0, 1, 5, and SID frames because we only remove 6 bits. So old code generated truncated AMR packets for those AMR modes. This patch fixes the length calculation by properly counting bits. Proper bit counting is also bringing us one small step closer to properly handlig multi-frame AMR packets. Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 --- M include/osmocom/netif/amr.h M src/amr.c M tests/amr/amr_test.c M tests/amr/amr_test.ok 4 files changed, 147 insertions(+), 75 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/65/18265/4 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/18265 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70 Gerrit-Change-Number: 18265 Gerrit-PatchSet: 4 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
ipse has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmo-netif/+/18265 )
Change subject: amr: Fix OA<->BWE conversion.
..
amr: Fix OA<->BWE conversion.
Size of a single AMR frame doesn't always shrink by a byte when
converted from octet-aligned to bandwidth-efficient mode. It does
shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for
AMR modes 0, 1, 5, and SID frames because we only remove 6 bits.
So old code generated truncated AMR packets for those AMR modes.
This patch fixes the length calculation by properly counting bits.
Proper bit counting is also bringing us one small step closer to
properly handlig multi-frame AMR packets.
Change-Id: I0462e054a0adc9080456f3eeea9cab7c229cdb70
---
M include/osmocom/netif/amr.h
M src/amr.c
M tests/amr/amr_test.c
M tests/amr/amr_test.ok
4 files changed, 147 insertions(+), 75 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/65/18265/1
diff --git a/include/osmocom/netif/amr.h b/include/osmocom/netif/amr.h
index 6e37c99..c5a8e28 100644
--- a/include/osmocom/netif/amr.h
+++ b/include/osmocom/netif/amr.h
@@ -78,20 +78,34 @@
#define AMR_FT_SID 8 /* SID */
#define AMR_FT_MAX 9
-/* AMR voice frame length (in bytes, rounded),
- * See also RFC 3267, chapter 3.6 */
-#define AMR_FT_0_LEN 12 /* 4.75 */
-#define AMR_FT_1_LEN 13 /* 5.15 */
-#define AMR_FT_2_LEN 15 /* 5.90 */
-#define AMR_FT_3_LEN 17 /* 6.70 */
-#define AMR_FT_4_LEN 19 /* 7.40 */
-#define AMR_FT_5_LEN 20 /* 7.95 */
-#define AMR_FT_6_LEN 26 /* 10.2 */
-#define AMR_FT_7_LEN 31 /* 12.2 */
-#define AMR_FT_SID_LEN 5 /* SID */
-
-/* NOTE: the above constant refers to the length of one AMR speech frame-block,
+/* AMR voice frame length (in bits).
+ * See also RFC 3267, chapter 3.6.
+ *
+ * NOTE: These constants refer to the length of one AMR speech frame-block,
* not counting CMR, TOC. */
+#define AMR_FT_0_LEN_BITS 95 /* 4.75 */
+#define AMR_FT_1_LEN_BITS 103 /* 5.15 */
+#define AMR_FT_2_LEN_BITS 118 /* 5.90 */
+#define AMR_FT_3_LEN_BITS 134 /* 6.70 */
+#define AMR_FT_4_LEN_BITS 148 /* 7.40 */
+#define AMR_FT_5_LEN_BITS 159 /* 7.95 */
+#define AMR_FT_6_LEN_BITS 204 /* 10.2 */
+#define AMR_FT_7_LEN_BITS 244 /* 12.2 */
+#define AMR_FT_SID_LEN_BITS39 /* SID */
+
+/* AMR voice frame length (in bytes, rounded).
+ *
+ * NOTE: These constants refer to the length of one AMR speech frame-block,
+ * not counting CMR, TOC. */
+#define AMR_FT_0_LEN ((AMR_FT_0_LEN_BITS+7)/8) /* 4.75 */
+#define AMR_FT_1_LEN ((AMR_FT_1_LEN_BITS+7)/8) /* 5.15 */
+#define AMR_FT_2_LEN ((AMR_FT_2_LEN_BITS+7)/8) /* 5.90 */
+#define AMR_FT_3_LEN ((AMR_FT_3_LEN_BITS+7)/8) /* 6.70 */
+#define AMR_FT_4_LEN ((AMR_FT_4_LEN_BITS+7)/8) /* 7.40 */
+#define AMR_FT_5_LEN ((AMR_FT_5_LEN_BITS+7)/8) /* 7.95 */
+#define AMR_FT_6_LEN ((AMR_FT_6_LEN_BITS+7)/8) /* 10.2 */
+#define AMR_FT_7_LEN ((AMR_FT_7_LEN_BITS+7)/8) /* 12.2 */
+#define AMR_FT_SID_LEN ((AMR_FT_SID_LEN_BITS+7)/8) /* SID */
int osmo_amr_ft_valid(uint8_t amr_ft);
size_t osmo_amr_bytes(uint8_t amr_cmr);
diff --git a/src/amr.c b/src/amr.c
index 5609c46..838915d 100644
--- a/src/amr.c
+++ b/src/amr.c
@@ -29,6 +29,18 @@
* 7 12.20 24431
*/
+static size_t amr_ft_to_bits[AMR_FT_MAX] = {
+ [AMR_FT_0] = AMR_FT_0_LEN_BITS,
+ [AMR_FT_1] = AMR_FT_1_LEN_BITS,
+ [AMR_FT_2] = AMR_FT_2_LEN_BITS,
+ [AMR_FT_3] = AMR_FT_3_LEN_BITS,
+ [AMR_FT_4] = AMR_FT_4_LEN_BITS,
+ [AMR_FT_5] = AMR_FT_5_LEN_BITS,
+ [AMR_FT_6] = AMR_FT_6_LEN_BITS,
+ [AMR_FT_7] = AMR_FT_7_LEN_BITS,
+ [AMR_FT_SID]= AMR_FT_SID_LEN_BITS,
+};
+
static size_t amr_ft_to_bytes[AMR_FT_MAX] = {
[AMR_FT_0] = AMR_FT_0_LEN,
[AMR_FT_1] = AMR_FT_1_LEN,
@@ -41,6 +53,11 @@
[AMR_FT_SID]= AMR_FT_SID_LEN,
};
+size_t osmo_amr_bits(uint8_t amr_ft)
+{
+ return amr_ft_to_bits[amr_ft];
+}
+
size_t osmo_amr_bytes(uint8_t amr_ft)
{
return amr_ft_to_bytes[amr_ft];
@@ -119,8 +136,10 @@
int osmo_amr_oa_to_bwe(uint8_t *payload, unsigned int payload_len)
{
struct amr_hdr *oa_hdr = (struct amr_hdr *)payload;
+ unsigned int ft = oa_hdr->ft;
unsigned int frame_len = payload_len - sizeof(struct amr_hdr);
unsigned int i;
+ int bwe_payload_len;
/* This implementation is not capable to handle multi-frame
* packets, so we need to make sure that the frame we operate on
@@ -137,8 +156,10 @@
payload[i + 2] = payload[i + 2] << 6;
}
- /* The overall saving is one byte! */
- return payload_len - 1;
+ /* Calculate new payload length */
+ bwe_payload_len
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
neels has created a revert of this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/17997 ) Change subject: amr: Fix OA<->BWE conversion. .. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced Gerrit-Change-Number: 17997 Gerrit-PatchSet: 8 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: ipse Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-MessageType: revert
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
laforge has submitted this change. (
https://gerrit.osmocom.org/c/libosmo-netif/+/17997 )
Change subject: amr: Fix OA<->BWE conversion.
..
amr: Fix OA<->BWE conversion.
Size of a single AMR frame doesn't always shrink by a byte when
converted from octet-aligned to bandwidth-efficient mode. It does
shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for
AMR modes 0, 1, 5, and SID frames because we only remove 6 bits.
So old code generated truncated AMR packets for those AMR modes.
This patch fixes the length calculation by properly counting bits.
Proper bit counting is also bringing us one small step closer to
properly handlig multi-frame AMR packets.
Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced
---
M include/osmocom/netif/amr.h
M src/amr.c
M tests/amr/amr_test.c
M tests/amr/amr_test.ok
4 files changed, 145 insertions(+), 75 deletions(-)
Approvals:
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/include/osmocom/netif/amr.h b/include/osmocom/netif/amr.h
index 6e37c99..c5a8e28 100644
--- a/include/osmocom/netif/amr.h
+++ b/include/osmocom/netif/amr.h
@@ -78,20 +78,34 @@
#define AMR_FT_SID 8 /* SID */
#define AMR_FT_MAX 9
-/* AMR voice frame length (in bytes, rounded),
- * See also RFC 3267, chapter 3.6 */
-#define AMR_FT_0_LEN 12 /* 4.75 */
-#define AMR_FT_1_LEN 13 /* 5.15 */
-#define AMR_FT_2_LEN 15 /* 5.90 */
-#define AMR_FT_3_LEN 17 /* 6.70 */
-#define AMR_FT_4_LEN 19 /* 7.40 */
-#define AMR_FT_5_LEN 20 /* 7.95 */
-#define AMR_FT_6_LEN 26 /* 10.2 */
-#define AMR_FT_7_LEN 31 /* 12.2 */
-#define AMR_FT_SID_LEN 5 /* SID */
-
-/* NOTE: the above constant refers to the length of one AMR speech frame-block,
+/* AMR voice frame length (in bits).
+ * See also RFC 3267, chapter 3.6.
+ *
+ * NOTE: These constants refer to the length of one AMR speech frame-block,
* not counting CMR, TOC. */
+#define AMR_FT_0_LEN_BITS 95 /* 4.75 */
+#define AMR_FT_1_LEN_BITS 103 /* 5.15 */
+#define AMR_FT_2_LEN_BITS 118 /* 5.90 */
+#define AMR_FT_3_LEN_BITS 134 /* 6.70 */
+#define AMR_FT_4_LEN_BITS 148 /* 7.40 */
+#define AMR_FT_5_LEN_BITS 159 /* 7.95 */
+#define AMR_FT_6_LEN_BITS 204 /* 10.2 */
+#define AMR_FT_7_LEN_BITS 244 /* 12.2 */
+#define AMR_FT_SID_LEN_BITS39 /* SID */
+
+/* AMR voice frame length (in bytes, rounded).
+ *
+ * NOTE: These constants refer to the length of one AMR speech frame-block,
+ * not counting CMR, TOC. */
+#define AMR_FT_0_LEN ((AMR_FT_0_LEN_BITS+7)/8) /* 4.75 */
+#define AMR_FT_1_LEN ((AMR_FT_1_LEN_BITS+7)/8) /* 5.15 */
+#define AMR_FT_2_LEN ((AMR_FT_2_LEN_BITS+7)/8) /* 5.90 */
+#define AMR_FT_3_LEN ((AMR_FT_3_LEN_BITS+7)/8) /* 6.70 */
+#define AMR_FT_4_LEN ((AMR_FT_4_LEN_BITS+7)/8) /* 7.40 */
+#define AMR_FT_5_LEN ((AMR_FT_5_LEN_BITS+7)/8) /* 7.95 */
+#define AMR_FT_6_LEN ((AMR_FT_6_LEN_BITS+7)/8) /* 10.2 */
+#define AMR_FT_7_LEN ((AMR_FT_7_LEN_BITS+7)/8) /* 12.2 */
+#define AMR_FT_SID_LEN ((AMR_FT_SID_LEN_BITS+7)/8) /* SID */
int osmo_amr_ft_valid(uint8_t amr_ft);
size_t osmo_amr_bytes(uint8_t amr_cmr);
diff --git a/src/amr.c b/src/amr.c
index 5609c46..a6b8361 100644
--- a/src/amr.c
+++ b/src/amr.c
@@ -29,6 +29,18 @@
* 7 12.20 24431
*/
+static size_t amr_ft_to_bits[AMR_FT_MAX] = {
+ [AMR_FT_0] = AMR_FT_0_LEN_BITS,
+ [AMR_FT_1] = AMR_FT_1_LEN_BITS,
+ [AMR_FT_2] = AMR_FT_2_LEN_BITS,
+ [AMR_FT_3] = AMR_FT_3_LEN_BITS,
+ [AMR_FT_4] = AMR_FT_4_LEN_BITS,
+ [AMR_FT_5] = AMR_FT_5_LEN_BITS,
+ [AMR_FT_6] = AMR_FT_6_LEN_BITS,
+ [AMR_FT_7] = AMR_FT_7_LEN_BITS,
+ [AMR_FT_SID]= AMR_FT_SID_LEN_BITS,
+};
+
static size_t amr_ft_to_bytes[AMR_FT_MAX] = {
[AMR_FT_0] = AMR_FT_0_LEN,
[AMR_FT_1] = AMR_FT_1_LEN,
@@ -41,6 +53,11 @@
[AMR_FT_SID]= AMR_FT_SID_LEN,
};
+size_t osmo_amr_bits(uint8_t amr_ft)
+{
+ return amr_ft_to_bits[amr_ft];
+}
+
size_t osmo_amr_bytes(uint8_t amr_ft)
{
return amr_ft_to_bytes[amr_ft];
@@ -119,8 +136,10 @@
int osmo_amr_oa_to_bwe(uint8_t *payload, unsigned int payload_len)
{
struct amr_hdr *oa_hdr = (struct amr_hdr *)payload;
+ unsigned int ft = oa_hdr->ft;
unsigned int frame_len = payload_len - sizeof(struct amr_hdr);
unsigned int i;
+ int bwe_payload_len;
/* This implementation is not capable to handle multi-frame
* packets, so we need to make sure that the frame we operate on
@@ -137,8 +156,10 @@
payload[i + 2] = payload[i + 2] << 6;
}
- /* The overall saving is one byte! */
- return payload_len - 1;
+ /* Calculate
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/17997 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced Gerrit-Change-Number: 17997 Gerrit-PatchSet: 7 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: ipse Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Comment-Date: Thu, 14 May 2020 12:03:40 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/17997 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced Gerrit-Change-Number: 17997 Gerrit-PatchSet: 7 Gerrit-Owner: ipse Gerrit-Assignee: dexter Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: ipse Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 12 May 2020 18:04:40 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/17997 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced Gerrit-Change-Number: 17997 Gerrit-PatchSet: 7 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: ipse Gerrit-Reviewer: pespin Gerrit-Comment-Date: Sun, 03 May 2020 14:58:45 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
ipse has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/17997 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 7: (1 comment) https://gerrit.osmocom.org/c/libosmo-netif/+/17997/6/tests/amr/amr_test.c File tests/amr/amr_test.c: https://gerrit.osmocom.org/c/libosmo-netif/+/17997/6/tests/amr/amr_test.c@127 PS6, Line 127: len = strlen(bwe_amr_samples[i]); > just spotted this line has not use at all. Fixed -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced Gerrit-Change-Number: 17997 Gerrit-PatchSet: 7 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: ipse Gerrit-Reviewer: pespin Gerrit-Comment-Date: Sat, 02 May 2020 23:19:31 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
Hello dexter, pespin, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 to look at the new patch set (#7). Change subject: amr: Fix OA<->BWE conversion. .. amr: Fix OA<->BWE conversion. Size of a single AMR frame doesn't always shrink by a byte when converted from octet-aligned to bandwidth-efficient mode. It does shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for AMR modes 0, 1, 5, and SID frames because we only remove 6 bits. So old code generated truncated AMR packets for those AMR modes. This patch fixes the length calculation by properly counting bits. Proper bit counting is also bringing us one small step closer to properly handlig multi-frame AMR packets. Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced --- M include/osmocom/netif/amr.h M src/amr.c M tests/amr/amr_test.c M tests/amr/amr_test.ok 4 files changed, 145 insertions(+), 75 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/97/17997/7 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced Gerrit-Change-Number: 17997 Gerrit-PatchSet: 7 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: ipse Gerrit-Reviewer: pespin Gerrit-MessageType: newpatchset
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/17997 ) Change subject: amr: Fix OA<->BWE conversion. .. Patch Set 6: Code-Review+1 (1 comment) https://gerrit.osmocom.org/c/libosmo-netif/+/17997/6/tests/amr/amr_test.c File tests/amr/amr_test.c: https://gerrit.osmocom.org/c/libosmo-netif/+/17997/6/tests/amr/amr_test.c@127 PS6, Line 127: len = strlen(bwe_amr_samples[i]); just spotted this line has not use at all. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced Gerrit-Change-Number: 17997 Gerrit-PatchSet: 6 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: ipse Gerrit-Reviewer: pespin Gerrit-Comment-Date: Sat, 02 May 2020 22:57:38 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
Hello dexter, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 to look at the new patch set (#6). Change subject: amr: Fix OA<->BWE conversion. .. amr: Fix OA<->BWE conversion. Size of a single AMR frame doesn't always shrink by a byte when converted from octet-aligned to bandwidth-efficient mode. It does shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for AMR modes 0, 1, 5, and SID frames because we only remove 6 bits. So old code generated truncated AMR packets for those AMR modes. This patch fixes the length calculation by properly counting bits. Proper bit counting is also bringing us one small step closer to properly handlig multi-frame AMR packets. Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced --- M include/osmocom/netif/amr.h M src/amr.c M tests/amr/amr_test.c M tests/amr/amr_test.ok 4 files changed, 145 insertions(+), 73 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/97/17997/6 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced Gerrit-Change-Number: 17997 Gerrit-PatchSet: 6 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: ipse Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in libosmo-netif[master]: amr: Fix OA<->BWE conversion.
Hello dexter, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 to look at the new patch set (#5). Change subject: amr: Fix OA<->BWE conversion. .. amr: Fix OA<->BWE conversion. Size of a single AMR frame doesn't always shrink by a byte when converted from octet-aligned to bandwidth-efficient mode. It does shrink for AMR modes 2, 3, 4, 6, and 7 but doesn't shrink for AMR modes 0, 1, 5, and SID frames because we only remove 6 bits. So old code generated truncated SID frames for those AMR modes which are 1 byte short. This patch fixes the length calculation by properly counting bits. Proper bit counting is also bringing us one small step closer to properly handlig multi-frame AMR packets. Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced --- M include/osmocom/netif/amr.h M src/amr.c M tests/amr/amr_test.c M tests/amr/amr_test.ok 4 files changed, 145 insertions(+), 73 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/97/17997/5 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/17997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9fc5fb92e9bada22a47a82fcfb0925e892e50ced Gerrit-Change-Number: 17997 Gerrit-PatchSet: 5 Gerrit-Owner: ipse Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: ipse Gerrit-CC: pespin Gerrit-MessageType: newpatchset
