openbsc[master]: nat: Add jitter buffer on the uplink receiver

2018-04-16 Thread Harald Welte 

Patch Set 3: Code-Review+2

-- 
To view, visit https://gerrit.osmocom.org/7793
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf3932adc07442fb5e9c7a06404853f9d0a20959
Gerrit-PatchSet: 3
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: daniel 
Gerrit-HasComments: No

[MERGED] openbsc[master]: nat: Add jitter buffer on the uplink receiver

2018-04-16 Thread Harald Welte 
Harald Welte has submitted this change and it was merged.

Change subject: nat: Add jitter buffer on the uplink receiver
..


nat: Add jitter buffer on the uplink receiver

Default usage values are defined in mgcp node, and can be per-BSC
overriden on each bsc node.

Change-Id: Ibf3932adc07442fb5e9c7a06404853f9d0a20959
---
M openbsc/include/openbsc/bsc_nat.h
M openbsc/include/openbsc/mgcp.h
M openbsc/include/openbsc/mgcp_internal.h
M openbsc/src/libmgcp/mgcp_network.c
M openbsc/src/libmgcp/mgcp_protocol.c
M openbsc/src/libmgcp/mgcp_vty.c
M openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c
M openbsc/src/osmo-bsc_nat/bsc_nat_vty.c
8 files changed, 253 insertions(+), 3 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/openbsc/include/openbsc/bsc_nat.h 
b/openbsc/include/openbsc/bsc_nat.h
index fad3804..5171c3e 100644
--- a/openbsc/include/openbsc/bsc_nat.h
+++ b/openbsc/include/openbsc/bsc_nat.h
@@ -175,6 +175,16 @@
 
/* Osmux is enabled/disabled per BSC */
int osmux;
+
+   /* Use a jitterbuffer on the bts-side receiver */
+   bool bts_use_jibuf;
+   /* Minimum and maximum buffer size for the jitter buffer, in ms */
+   uint32_t bts_jitter_delay_min;
+   uint32_t bts_jitter_delay_max;
+   /* Enabled if explicitly configured through VTY: */
+   bool bts_use_jibuf_override;
+   bool bts_jitter_delay_min_override;
+   bool bts_jitter_delay_max_override;
 };
 
 struct bsc_lac_entry {
diff --git a/openbsc/include/openbsc/mgcp.h b/openbsc/include/openbsc/mgcp.h
index b2262bc..4d1c5ef 100644
--- a/openbsc/include/openbsc/mgcp.h
+++ b/openbsc/include/openbsc/mgcp.h
@@ -236,6 +236,12 @@
 * message.
 */
uint16_t osmux_dummy;
+
+   /* Use a jitterbuffer on the bts-side receiver */
+   bool bts_use_jibuf;
+   /* Minimum and maximum buffer size for the jitter buffer, in ms */
+   uint32_t bts_jitter_delay_min;
+   uint32_t bts_jitter_delay_max;
 };
 
 /* config management */
diff --git a/openbsc/include/openbsc/mgcp_internal.h 
b/openbsc/include/openbsc/mgcp_internal.h
index 7c89d10..cd6365f 100644
--- a/openbsc/include/openbsc/mgcp_internal.h
+++ b/openbsc/include/openbsc/mgcp_internal.h
@@ -25,6 +25,7 @@
 #include 
 
 #include 
+#include 
 
 #define CI_UNUSED 0
 
@@ -205,6 +206,14 @@
uint32_t octets;
} stats;
} osmux;
+
+   /* Jitter buffer */
+   struct osmo_jibuf* bts_jb;
+   /* Use a jitterbuffer on the bts-side receiver */
+   bool bts_use_jibuf;
+   /* Minimum and maximum buffer size for the jitter buffer, in ms */
+   uint32_t bts_jitter_delay_min;
+   uint32_t bts_jitter_delay_max;
 };
 
 #define for_each_line(line, save)  \
@@ -340,3 +349,8 @@
return endp->cfg->bts_ports.bind_addr;
return endp->cfg->source_addr;
 }
+
+/**
+ * Internal jitter buffer related
+ */
+void mgcp_dejitter_udp_send(struct msgb *msg, void *data);
diff --git a/openbsc/src/libmgcp/mgcp_network.c 
b/openbsc/src/libmgcp/mgcp_network.c
index abce6e4..799d998 100644
--- a/openbsc/src/libmgcp/mgcp_network.c
+++ b/openbsc/src/libmgcp/mgcp_network.c
@@ -580,6 +580,36 @@
return rc;
 }
 
+void mgcp_dejitter_udp_send(struct msgb *msg, void *data)
+{
+   struct mgcp_rtp_end *rtp_end = (struct mgcp_rtp_end *) data;
+
+   int rc = mgcp_udp_send(rtp_end->rtp.fd, _end->addr,
+  rtp_end->rtp_port, (char*) msg->data, msg->len);
+   if (rc != msg->len)
+   LOGP(DMGCP, LOGL_ERROR,
+   "Failed to send data after jitter buffer: %d\n", rc);
+   msgb_free(msg);
+}
+
+static int enqueue_dejitter(struct osmo_jibuf *jb, struct mgcp_rtp_end 
*rtp_end, char *buf, int len)
+{
+   struct msgb *msg;
+   msg = msgb_alloc(len, "mgcp-jibuf");
+   if (!msg)
+   return -1;
+
+   memcpy(msg->data, buf, len);
+   msgb_put(msg, len);
+
+   if (osmo_jibuf_enqueue(jb, msg) < 0) {
+   rtp_end->dropped_packets += 1;
+   msgb_free(msg);
+   }
+
+   return len;
+}
+
 int mgcp_send(struct mgcp_endpoint *endp, int dest, int is_rtp,
  struct sockaddr_in *addr, char *buf, int rc)
 {
@@ -587,6 +617,7 @@
struct mgcp_rtp_end *rtp_end;
struct mgcp_rtp_state *rtp_state;
int tap_idx;
+   struct osmo_jibuf *jb;
 
/* For loop toggle the destination and then dispatch. */
if (tcfg->audio_loop)
@@ -600,10 +631,12 @@
rtp_end = >net_end;
rtp_state = >bts_state;
tap_idx = MGCP_TAP_NET_OUT;
+   jb = endp->bts_jb;
} else {
rtp_end = >bts_end;
rtp_state = >net_state;
tap_idx = MGCP_TAP_BTS_OUT;
+   jb = NULL;
}
 
if

[PATCH] openbsc[master]: nat: Add jitter buffer on the uplink receiver

2018-04-16 Thread Pau Espin Pedrol 
Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7793

to look at the new patch set (#3).

nat: Add jitter buffer on the uplink receiver

Default usage values are defined in mgcp node, and can be per-BSC
overriden on each bsc node.

Change-Id: Ibf3932adc07442fb5e9c7a06404853f9d0a20959
---
M openbsc/include/openbsc/bsc_nat.h
M openbsc/include/openbsc/mgcp.h
M openbsc/include/openbsc/mgcp_internal.h
M openbsc/src/libmgcp/mgcp_network.c
M openbsc/src/libmgcp/mgcp_protocol.c
M openbsc/src/libmgcp/mgcp_vty.c
M openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c
M openbsc/src/osmo-bsc_nat/bsc_nat_vty.c
8 files changed, 253 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/93/7793/3

diff --git a/openbsc/include/openbsc/bsc_nat.h 
b/openbsc/include/openbsc/bsc_nat.h
index fad3804..5171c3e 100644
--- a/openbsc/include/openbsc/bsc_nat.h
+++ b/openbsc/include/openbsc/bsc_nat.h
@@ -175,6 +175,16 @@
 
/* Osmux is enabled/disabled per BSC */
int osmux;
+
+   /* Use a jitterbuffer on the bts-side receiver */
+   bool bts_use_jibuf;
+   /* Minimum and maximum buffer size for the jitter buffer, in ms */
+   uint32_t bts_jitter_delay_min;
+   uint32_t bts_jitter_delay_max;
+   /* Enabled if explicitly configured through VTY: */
+   bool bts_use_jibuf_override;
+   bool bts_jitter_delay_min_override;
+   bool bts_jitter_delay_max_override;
 };
 
 struct bsc_lac_entry {
diff --git a/openbsc/include/openbsc/mgcp.h b/openbsc/include/openbsc/mgcp.h
index b2262bc..4d1c5ef 100644
--- a/openbsc/include/openbsc/mgcp.h
+++ b/openbsc/include/openbsc/mgcp.h
@@ -236,6 +236,12 @@
 * message.
 */
uint16_t osmux_dummy;
+
+   /* Use a jitterbuffer on the bts-side receiver */
+   bool bts_use_jibuf;
+   /* Minimum and maximum buffer size for the jitter buffer, in ms */
+   uint32_t bts_jitter_delay_min;
+   uint32_t bts_jitter_delay_max;
 };
 
 /* config management */
diff --git a/openbsc/include/openbsc/mgcp_internal.h 
b/openbsc/include/openbsc/mgcp_internal.h
index 7c89d10..cd6365f 100644
--- a/openbsc/include/openbsc/mgcp_internal.h
+++ b/openbsc/include/openbsc/mgcp_internal.h
@@ -25,6 +25,7 @@
 #include 
 
 #include 
+#include 
 
 #define CI_UNUSED 0
 
@@ -205,6 +206,14 @@
uint32_t octets;
} stats;
} osmux;
+
+   /* Jitter buffer */
+   struct osmo_jibuf* bts_jb;
+   /* Use a jitterbuffer on the bts-side receiver */
+   bool bts_use_jibuf;
+   /* Minimum and maximum buffer size for the jitter buffer, in ms */
+   uint32_t bts_jitter_delay_min;
+   uint32_t bts_jitter_delay_max;
 };
 
 #define for_each_line(line, save)  \
@@ -340,3 +349,8 @@
return endp->cfg->bts_ports.bind_addr;
return endp->cfg->source_addr;
 }
+
+/**
+ * Internal jitter buffer related
+ */
+void mgcp_dejitter_udp_send(struct msgb *msg, void *data);
diff --git a/openbsc/src/libmgcp/mgcp_network.c 
b/openbsc/src/libmgcp/mgcp_network.c
index abce6e4..799d998 100644
--- a/openbsc/src/libmgcp/mgcp_network.c
+++ b/openbsc/src/libmgcp/mgcp_network.c
@@ -580,6 +580,36 @@
return rc;
 }
 
+void mgcp_dejitter_udp_send(struct msgb *msg, void *data)
+{
+   struct mgcp_rtp_end *rtp_end = (struct mgcp_rtp_end *) data;
+
+   int rc = mgcp_udp_send(rtp_end->rtp.fd, _end->addr,
+  rtp_end->rtp_port, (char*) msg->data, msg->len);
+   if (rc != msg->len)
+   LOGP(DMGCP, LOGL_ERROR,
+   "Failed to send data after jitter buffer: %d\n", rc);
+   msgb_free(msg);
+}
+
+static int enqueue_dejitter(struct osmo_jibuf *jb, struct mgcp_rtp_end 
*rtp_end, char *buf, int len)
+{
+   struct msgb *msg;
+   msg = msgb_alloc(len, "mgcp-jibuf");
+   if (!msg)
+   return -1;
+
+   memcpy(msg->data, buf, len);
+   msgb_put(msg, len);
+
+   if (osmo_jibuf_enqueue(jb, msg) < 0) {
+   rtp_end->dropped_packets += 1;
+   msgb_free(msg);
+   }
+
+   return len;
+}
+
 int mgcp_send(struct mgcp_endpoint *endp, int dest, int is_rtp,
  struct sockaddr_in *addr, char *buf, int rc)
 {
@@ -587,6 +617,7 @@
struct mgcp_rtp_end *rtp_end;
struct mgcp_rtp_state *rtp_state;
int tap_idx;
+   struct osmo_jibuf *jb;
 
/* For loop toggle the destination and then dispatch. */
if (tcfg->audio_loop)
@@ -600,10 +631,12 @@
rtp_end = >net_end;
rtp_state = >bts_state;
tap_idx = MGCP_TAP_NET_OUT;
+   jb = endp->bts_jb;
} else {
rtp_end = >bts_end;
rtp_state = >net_state;
tap_idx = MGCP_TAP_BTS_OUT;
+   jb = NULL;
}
 
if (!rtp_end->output_enabled)
@@ -621,9 +654,12 @@

openbsc[master]: nat: Add jitter buffer on the uplink receiver

2018-04-16 Thread Harald Welte 

Patch Set 2: Code-Review-1

(7 comments)

https://gerrit.osmocom.org/#/c/7793/2/openbsc/include/openbsc/mgcp_internal.h
File openbsc/include/openbsc/mgcp_internal.h:

Line 215:   uint32_t bts_jitter_delay_min;
what units is this? ms? us? bytes? codec-frames?  If the varioable name doesn't 
indicate this (like delay_ms_min) the comment should at least state it.


https://gerrit.osmocom.org/#/c/7793/2/openbsc/src/libmgcp/mgcp_protocol.c
File openbsc/src/libmgcp/mgcp_protocol.c:

Line 872:   if(endp->bts_use_jibuf) {
"if" is not a function (space needed)


Line 887:   if(endp->bts_use_jibuf) {
same here


Line 1357:  if(endp->bts_jb)
and again?


https://gerrit.osmocom.org/#/c/7793/2/openbsc/src/libmgcp/mgcp_vty.c
File openbsc/src/libmgcp/mgcp_vty.c:

Line 1371:   DEJITTER_STR " Minimum Delay\n" "Minimum Delay\n")
see my other comment at the struct definition.  The VTY user has no idea about 
the unit of the value he's supposed to enter here.  the "?" help should make 
that clear.


https://gerrit.osmocom.org/#/c/7793/2/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c
File openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c:

Line 589:   if(bsc_endp->bsc->cfg->bts_use_jibuf_override)
if is not a function (below two more instances)


https://gerrit.osmocom.org/#/c/7793/2/openbsc/src/osmo-bsc_nat/bsc_nat_vty.c
File openbsc/src/osmo-bsc_nat/bsc_nat_vty.c:

Line 1266:   "bts-jitter-buffer-delay-min <1-65535>",
same comment related to "unit" of the value


-- 
To view, visit https://gerrit.osmocom.org/7793
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf3932adc07442fb5e9c7a06404853f9d0a20959
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: daniel 
Gerrit-HasComments: Yes

openbsc[master]: nat: Add jitter buffer on the uplink receiver

2018-04-13 Thread Pau Espin Pedrol 

Patch Set 1:

After I receive feedback for this one, I may add support to enable a 
jitterbuffer in the downstream too in a follow up commit.

-- 
To view, visit https://gerrit.osmocom.org/7793
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf3932adc07442fb5e9c7a06404853f9d0a20959
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No

openbsc[master]: nat: Add jitter buffer on the uplink receiver

2018-04-13 Thread Pau Espin Pedrol 

Patch Set 1:

Depends on https://gerrit.osmocom.org/#/c/7773 for correct build.

-- 
To view, visit https://gerrit.osmocom.org/7793
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf3932adc07442fb5e9c7a06404853f9d0a20959
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No

[PATCH] openbsc[master]: nat: Add jitter buffer on the uplink receiver

2018-04-13 Thread Pau Espin Pedrol 

nat: Add jitter buffer on the uplink receiver

Default usage values are defined in mgcp node, and can be per-BSC
overriden on each bsc node.

Change-Id: Ibf3932adc07442fb5e9c7a06404853f9d0a20959
---
M openbsc/include/openbsc/bsc_nat.h
M openbsc/include/openbsc/mgcp.h
M openbsc/include/openbsc/mgcp_internal.h
M openbsc/src/libmgcp/mgcp_network.c
M openbsc/src/libmgcp/mgcp_protocol.c
M openbsc/src/libmgcp/mgcp_vty.c
M openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c
M openbsc/src/osmo-bsc_nat/bsc_nat_vty.c
8 files changed, 253 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/93/7793/2

diff --git a/openbsc/include/openbsc/bsc_nat.h 
b/openbsc/include/openbsc/bsc_nat.h
index fad3804..48cd034 100644
--- a/openbsc/include/openbsc/bsc_nat.h
+++ b/openbsc/include/openbsc/bsc_nat.h
@@ -175,6 +175,16 @@
 
/* Osmux is enabled/disabled per BSC */
int osmux;
+
+   /* Use a jitterbuffer on the bts-side receiver */
+   bool bts_use_jibuf;
+   /* Minimum and maximum buffer size for the jitter buffer */
+   uint32_t bts_jitter_delay_min;
+   uint32_t bts_jitter_delay_max;
+   /* Enabled if explicitly configured through VTY: */
+   bool bts_use_jibuf_override;
+   bool bts_jitter_delay_min_override;
+   bool bts_jitter_delay_max_override;
 };
 
 struct bsc_lac_entry {
diff --git a/openbsc/include/openbsc/mgcp.h b/openbsc/include/openbsc/mgcp.h
index b2262bc..8dc3e76 100644
--- a/openbsc/include/openbsc/mgcp.h
+++ b/openbsc/include/openbsc/mgcp.h
@@ -236,6 +236,12 @@
 * message.
 */
uint16_t osmux_dummy;
+
+   /* Use a jitterbuffer on the bts-side receiver */
+   bool bts_use_jibuf;
+   /* Minimum and maximum buffer size for the jitter buffer */
+   uint32_t bts_jitter_delay_min;
+   uint32_t bts_jitter_delay_max;
 };
 
 /* config management */
diff --git a/openbsc/include/openbsc/mgcp_internal.h 
b/openbsc/include/openbsc/mgcp_internal.h
index 7c89d10..4bc4f43 100644
--- a/openbsc/include/openbsc/mgcp_internal.h
+++ b/openbsc/include/openbsc/mgcp_internal.h
@@ -25,6 +25,7 @@
 #include 
 
 #include 
+#include 
 
 #define CI_UNUSED 0
 
@@ -205,6 +206,14 @@
uint32_t octets;
} stats;
} osmux;
+
+   /* Jitter buffer */
+   struct osmo_jibuf* bts_jb;
+   /* Use a jitterbuffer on the bts-side receiver */
+   bool bts_use_jibuf;
+   /* Minimum and maximum buffer size for the jitter buffer */
+   uint32_t bts_jitter_delay_min;
+   uint32_t bts_jitter_delay_max;
 };
 
 #define for_each_line(line, save)  \
@@ -340,3 +349,8 @@
return endp->cfg->bts_ports.bind_addr;
return endp->cfg->source_addr;
 }
+
+/**
+ * Internal jitter buffer related
+ */
+void mgcp_dejitter_udp_send(struct msgb *msg, void *data);
diff --git a/openbsc/src/libmgcp/mgcp_network.c 
b/openbsc/src/libmgcp/mgcp_network.c
index abce6e4..799d998 100644
--- a/openbsc/src/libmgcp/mgcp_network.c
+++ b/openbsc/src/libmgcp/mgcp_network.c
@@ -580,6 +580,36 @@
return rc;
 }
 
+void mgcp_dejitter_udp_send(struct msgb *msg, void *data)
+{
+   struct mgcp_rtp_end *rtp_end = (struct mgcp_rtp_end *) data;
+
+   int rc = mgcp_udp_send(rtp_end->rtp.fd, _end->addr,
+  rtp_end->rtp_port, (char*) msg->data, msg->len);
+   if (rc != msg->len)
+   LOGP(DMGCP, LOGL_ERROR,
+   "Failed to send data after jitter buffer: %d\n", rc);
+   msgb_free(msg);
+}
+
+static int enqueue_dejitter(struct osmo_jibuf *jb, struct mgcp_rtp_end 
*rtp_end, char *buf, int len)
+{
+   struct msgb *msg;
+   msg = msgb_alloc(len, "mgcp-jibuf");
+   if (!msg)
+   return -1;
+
+   memcpy(msg->data, buf, len);
+   msgb_put(msg, len);
+
+   if (osmo_jibuf_enqueue(jb, msg) < 0) {
+   rtp_end->dropped_packets += 1;
+   msgb_free(msg);
+   }
+
+   return len;
+}
+
 int mgcp_send(struct mgcp_endpoint *endp, int dest, int is_rtp,
  struct sockaddr_in *addr, char *buf, int rc)
 {
@@ -587,6 +617,7 @@
struct mgcp_rtp_end *rtp_end;
struct mgcp_rtp_state *rtp_state;
int tap_idx;
+   struct osmo_jibuf *jb;
 
/* For loop toggle the destination and then dispatch. */
if (tcfg->audio_loop)
@@ -600,10 +631,12 @@
rtp_end = >net_end;
rtp_state = >bts_state;
tap_idx = MGCP_TAP_NET_OUT;
+   jb = endp->bts_jb;
} else {
rtp_end = >bts_end;
rtp_state = >net_state;
tap_idx = MGCP_TAP_BTS_OUT;
+   jb = NULL;
}
 
if (!rtp_end->output_enabled)
@@ -621,9 +654,12 @@
mgcp_patch_and_count(endp, rtp_state, rtp_end, addr, 
buf, len);
forward_data(rtp_end->rtp.fd, >taps[tap_idx],