mgcp: Pick a different encoding when we need to handle Marker frames

Encode a list of packets differently when we are dealing with a
marker bit. We now have 7 more bits to store information, we might
also want to add a mark to stop using marks.

3 files changed, 233 insertions, 101 deletions

diff --git a/openbsc/include/openbsc/mgcp_internal.h b/openbsc/include/openbsc/mgcp_internal.h
index c1b2bb338..138cc091d 100644
--- a/openbsc/include/openbsc/mgcp_internal.h
+++ b/openbsc/include/openbsc/mgcp_internal.h
@@ -175,8 +175,8 @@ struct mgcp_trunk_config *mgcp_trunk_num(struct mgcp_config *cfg, int index);
 
 int rtp_compress(struct mgcp_rtp_compr_state *state, struct msgb *msg,
 		 int endpoint, struct llist_head *rtp_packets);
-struct llist_head rtp_decompress(struct mgcp_rtp_compr_state *state,
-				  struct msgb *msg);
+int rtp_decompress(struct mgcp_rtp_compr_state *state, struct llist_head *list,
+		   struct msgb *msg);
 
 
 #endif
diff --git a/openbsc/src/libmgcp/rtp_helper.c b/openbsc/src/libmgcp/rtp_helper.c
index 66687b19b..2fc583f5e 100644
--- a/openbsc/src/libmgcp/rtp_helper.c
+++ b/openbsc/src/libmgcp/rtp_helper.c
@@ -23,9 +23,16 @@
 #include <openbsc/mgcp_internal.h>
 
 #include <osmocom/core/talloc.h>
+#include <osmocom/gsm/tlv.h>
 
 #include <string.h>
 
+enum RTP_EXTRA_FRAME {
+	/** Set a marker bit on the RFC */
+	RTP_EXTRA_MARKER = 0x1,
+
+	/** Maybe add a bit to mention no further extension bits? */
+};
 
 struct reduced_rtp_hdr {
 	/** I will need to be inflated to the SSRC */
@@ -65,7 +72,125 @@ static void fill_rtp_state(struct rtp_hdr *hdr,
 	hdr->timestamp = htonl(state->timestamp);
 	state->timestamp += 160;
 }
-			   
+
+static void write_compressed_big(struct reduced_rtp_hdr *reduced_hdr,
+				 struct msgb *msg,
+				 struct llist_head *rtp_packets)
+{
+	struct msgb *rtp, *tmp;
+
+	reduced_hdr->type = 1;
+
+	llist_for_each_entry_safe(rtp, tmp, rtp_packets, list) {
+		struct rtp_hdr *hdr = (struct rtp_hdr *) rtp->l2h;
+		uint32_t len = msgb_l2len(rtp) - sizeof(*hdr);
+		uint8_t *data;
+
+		msgb_v_put(msg, hdr->marker ? RTP_EXTRA_MARKER : 0);
+		data = msgb_put(msg, len);
+		memcpy(data, hdr->data, len);
+
+		llist_del(&rtp->list);
+		talloc_free(rtp);
+	}
+}
+
+
+/**
+ * I write a simple 3 byte header followed by the payloads of the
+ * single RTP packets. This is assumed to be AMR.
+ */
+static void write_compressed_slim(struct reduced_rtp_hdr *reduced_hdr,
+				  struct msgb *msg,
+				  struct llist_head *rtp_packets)
+{
+	struct msgb *rtp, *tmp;
+
+	reduced_hdr->type = 0;
+
+	llist_for_each_entry_safe(rtp, tmp, rtp_packets, list) {
+		struct rtp_hdr *hdr = (struct rtp_hdr *) rtp->l2h;
+		uint32_t len = msgb_l2len(rtp) - sizeof(*hdr);
+		uint8_t *data = msgb_put(msg, len);
+		memcpy(data, hdr->data, len);
+
+		llist_del(&rtp->list);
+		talloc_free(rtp);
+	}
+}
+
+static int read_compressed_big(struct msgb *msg,
+			       struct reduced_rtp_hdr *rhdr,
+			       struct llist_head *list,
+			       struct mgcp_rtp_compr_state *state)
+{
+	int i;
+
+	if (msgb_l2len(msg) < sizeof(*rhdr) + rhdr->payloads * 18) {
+		LOGP(DMGCP, LOGL_ERROR,
+		     "Payloads do not fit. %d\n", rhdr->payloads);
+		return -3;
+	}
+
+	for (i = 0; i < rhdr->payloads; ++i) {
+		struct rtp_hdr *hdr;
+		struct msgb *out = msgb_alloc_headroom(4096, 128, "RTP decompr");
+		if (!out) {
+			LOGP(DMGCP, LOGL_ERROR, "Failed to allocate: %d\n", i);
+			continue;
+		}
+
+		out->l2h = msgb_put(out, 0);
+		hdr = msgb_put_struct(out, struct rtp_hdr);
+		fill_rtp_hdr(hdr);
+		fill_rtp_state(hdr, state);
+
+		/* re-apply the marker bit */
+		if (rhdr->data[i * 18] & RTP_EXTRA_MARKER)
+			hdr->marker = 1;
+
+		out->l3h = msgb_put(out, 17);
+		memcpy(out->l3h, &rhdr->data[i * 18], 17);
+		msgb_enqueue(list, out);
+	}
+
+	return 0;
+}
+
+static int read_compressed_slim(struct msgb *msg,
+				struct reduced_rtp_hdr *rhdr,
+				struct llist_head *list,
+				struct mgcp_rtp_compr_state *state)
+{
+	int i;
+
+	if (msgb_l2len(msg) < sizeof(*rhdr) + rhdr->payloads * 17) {
+		LOGP(DMGCP, LOGL_ERROR,
+		     "Payloads do not fit. %d\n", rhdr->payloads);
+		return -3;
+	}
+
+	for (i = 0; i < rhdr->payloads; ++i) {
+		struct rtp_hdr *hdr;
+		struct msgb *out = msgb_alloc_headroom(4096, 128, "RTP decompr");
+		if (!out) {
+			LOGP(DMGCP, LOGL_ERROR, "Failed to allocate: %d\n", i);
+			continue;
+		}
+
+		out->l2h = msgb_put(out, 0);
+		hdr = msgb_put_struct(out, struct rtp_hdr);
+		fill_rtp_hdr(hdr);
+		fill_rtp_state(hdr, state);
+
+		out->l3h = msgb_put(out, 17);
+		memcpy(out->l3h, &rhdr->data[i * 17], 17);
+		msgb_enqueue(list, out);
+	}
+
+	return 0;
+}
+
 
 /**
  * I try to compress these packets into one single stream. I have various
@@ -87,11 +212,11 @@ int rtp_compress(struct mgcp_rtp_compr_state *state, struct msgb *msg,
 	struct msgb *rtp, *tmp;
 	struct reduced_rtp_hdr *reduced_hdr;
 	uint16_t last_sequence = 0;
-	int count = 0;
-	uint8_t *data;
+	int count = 0, marker = 0;
 
 	/*
-	 * sanity check if everything is a RTP packet
+	 * sanity check if everything is a RTP packet, or if we need to do
+	 * something special.
 	 */
 	llist_for_each_entry_safe(rtp, tmp, rtp_packets, list) {
 		struct rtp_hdr *hdr;
@@ -124,6 +249,9 @@ int rtp_compress(struct mgcp_rtp_compr_state *state, struct msgb *msg,
 			continue;
 		}
 
+		if (hdr->marker)
+			marker = 1;
+
 		last_sequence = sequence;
 		count += 1;
 	}
@@ -135,66 +263,35 @@ int rtp_compress(struct mgcp_rtp_compr_state *state, struct msgb *msg,
 	reduced_hdr = msgb_put_struct(msg, struct reduced_rtp_hdr);
 	reduced_hdr->endp = endp;
 	reduced_hdr->sequence_no = ++state->last_ts % UCHAR_MAX;
-	reduced_hdr->type = 0;
 	reduced_hdr->payloads = count;
 
-
-	llist_for_each_entry_safe(rtp, tmp, rtp_packets, list) {
-		struct rtp_hdr *hdr = (struct rtp_hdr *) rtp->l2h;
-		uint32_t len = msgb_l2len(rtp) - sizeof(*hdr);
-		data = msgb_put(msg, len);
-		memcpy(data, hdr->data, len);
-
-		llist_del(&rtp->list);
-		talloc_free(rtp);
-	}
+	if (marker)
+		write_compressed_big(reduced_hdr, msg, rtp_packets);
+	else
+		write_compressed_slim(reduced_hdr, msg, rtp_packets);
 
 	return count;
 }
 
-struct llist_head rtp_decompress(struct mgcp_rtp_compr_state *state,
-				  struct msgb *msg)
+int rtp_decompress(struct mgcp_rtp_compr_state *state,
+		   struct llist_head *list,
+		   struct msgb *msg)
 {
 	struct reduced_rtp_hdr *rhdr;
-	int i;
-	struct llist_head list;
-	INIT_LLIST_HEAD(&list);
 
 	if (msgb_l2len(msg) < sizeof(*rhdr)) {
 		LOGP(DMGCP, LOGL_ERROR, "Compressed header does not fit.\n");
-		return list;
+		return -1;
 	}
 
 	rhdr = (struct reduced_rtp_hdr *) msg->l2h;
-	if (rhdr->type != 0) {
+	if (rhdr->type == 0)
+		return read_compressed_slim(msg, rhdr, list, state);
+	else if (rhdr->type == 1)
+		return read_compressed_big(msg, rhdr, list, state);
+	else {
 		LOGP(DMGCP, LOGL_ERROR,
 		     "Type %d is not known.\n", rhdr->type);
-		return list;
+		return -2;
 	}
-
-	if (msgb_l2len(msg) < sizeof(*rhdr) + rhdr->payloads * 17) {
-		LOGP(DMGCP, LOGL_ERROR,
-		     "Payloads do not fit. %d\n", rhdr->payloads);
-		return list;
-	}
-
-	for (i = 0; i < rhdr->payloads; ++i) {
-		struct rtp_hdr *hdr;
-		struct msgb *out = msgb_alloc_headroom(4096, 128, "RTP decompr");
-		if (!out) {
-			LOGP(DMGCP, LOGL_ERROR, "Failed to allocate: %d\n", i);
-			continue;
-		}
-
-		out->l2h = msgb_put(out, 0);
-		hdr = msgb_put_struct(out, struct rtp_hdr);
-		fill_rtp_hdr(hdr);
-		fill_rtp_state(hdr, state);
-
-		out->l3h = msgb_put(out, 17);
-		memcpy(out->l3h, &rhdr->data[i * 17], 17);
-		msgb_enqueue(&list, out);
-	}
-
-	return list;
 }
diff --git a/openbsc/tests/mgcp/mgcp_test.c b/openbsc/tests/mgcp/mgcp_test.c
index 30bc625c0..f07db2f79 100644
--- a/openbsc/tests/mgcp/mgcp_test.c
+++ b/openbsc/tests/mgcp/mgcp_test.c
@@ -116,43 +116,75 @@ static struct msgb *from(const uint8_t *data, uint16_t len)
 	return msg;
 }
 
-#define FROM(array) \
-	from(array, sizeof(array))
+struct pdata {
+	const uint8_t *data;
+	uint16_t len;
+};
 
-void test_compress()
+static const struct pdata marker_normal[] = {
+	{ .data = packet_1, .len = sizeof(packet_1), },
+	{ .data = packet_2, .len = sizeof(packet_2), },
+	{ .data = packet_3, .len = sizeof(packet_3), },
+	{ .data = packet_4, .len = sizeof(packet_4), },
+};
+
+struct estate {
+	const struct pdata *data;
+	int plen;
+
+	const struct mgcp_rtp_compr_state state;
+	int output_size;
+};
+
+static const struct estate test_scenarious[] = {
+	{ .data = marker_normal, .plen = ARRAY_SIZE(marker_normal),
+	  .output_size = 17 *4 + 3 + 4 *1,
+	  .state = {
+		.last_ts = UCHAR_MAX,
+		.generated_ssrc = 0x6f0fb1da,
+		.sequence = 52103,
+		.timestamp = 4157323848u,
+	  },
+	},
+	{ .data = &marker_normal[1], .plen = ARRAY_SIZE(marker_normal) - 1,
+	  .output_size = 17 * 3 + 3,
+	  .state = {
+		.last_ts = UCHAR_MAX,
+		.generated_ssrc = 0x6f0fb1da,
+		.sequence = 52104,
+		.timestamp = 4157324008u,
+	  },
+	},
+};
+
+static void test_compress_one(const struct estate *edata, char *t)
 {
+	const struct pdata *data = edata->data;
+	const int len = edata->plen;
+
 	int i = 0;
 	struct msgb *msg;
-
-	struct msgb *msg1 = FROM(packet_1);
-	struct msgb *msg2 = FROM(packet_2);
-	struct msgb *msg3 = FROM(packet_3);
-	struct msgb *msg4 = FROM(packet_4);
-
+	struct msgb *msgs[len];
 	struct llist_head list;
+
 	INIT_LLIST_HEAD(&list);
-	msgb_enqueue(&list, msg1);
-	msgb_enqueue(&list, msg2);
-	msgb_enqueue(&list, msg3);
-	msgb_enqueue(&list, msg4);
-
-	struct mgcp_rtp_compr_state state;
-	memset(&state, 0, sizeof(state));
-	state.last_ts = UCHAR_MAX;
-	state.generated_ssrc = 0x6f0fb1da;
-	state.sequence = 52103;
-	state.timestamp = 4157323848u;
+
+	for (i = 0; i < len; ++i) {
+		msgs[i] = from(data[i].data, data[i].len);
+		msgb_enqueue(&list, msgs[i]);
+	}
 
 	struct msgb *out = msgb_alloc_headroom(4096, 128, "out");
 	out->l2h = msgb_put(out, 0);
 
+	struct mgcp_rtp_compr_state state = edata->state;
 	int rc = rtp_compress(&state, out, 23, &list);
-	if (rc != 4) {
-		fprintf(stderr, "Result is not 4: %d\n", rc);
+	if (rc != len) {
+		fprintf(stderr, "Result is not %d: %d\n", len, rc);
 		abort();
 	}
 
-	if (msgb_l2len(out) != (17*4 + 3)) {
+	if (msgb_l2len(out) != edata->output_size) {
 		fprintf(stderr, "Result is wrong size: %d\n", msgb_l2len(out));
 		abort();
 	}
@@ -160,49 +192,52 @@ void test_compress()
 	printf("output is: %s\n", osmo_hexdump(out->l2h, msgb_l2len(out)));
 
 
-	list = rtp_decompress(&state, out);
+	INIT_LLIST_HEAD(&list);
+	if (rtp_decompress(&state, &list, out) != 0) {
+		fprintf(stderr, "Failed to decompress the code.\n");
+		abort();
+	}
 
+	i = 0;
 	llist_for_each_entry(msg, &list, list) {
-		const uint8_t *data;
-		int len;
-
-		switch (++i) {
-		case 1:
-			data = packet_1;
-			len = sizeof(packet_1);
-			break;
-		case 2:
-			data = packet_2;
-			len = sizeof(packet_2);
-			break;
-		case 3:
-			data = packet_3;
-			len = sizeof(packet_3);
-			break;
-		case 4:
-			data = packet_4;
-			len = sizeof(packet_4);
-			break;
-		default:
-			fprintf(stderr, "Should not be reached.\n");
-			abort();
-		}
+		const struct pdata *got_data = &data[i];
 
-		if (msgb_l2len(msg) != len) {
+		if (msgb_l2len(msg) != got_data->len) {
 			fprintf(stderr, "Wrong len for %d %d\n", i, msgb_l2len(msg));
 			abort();
 		}
 
-		if (memcmp(msg->l2h, data, len) != 0) {
+		if (memcmp(msg->l2h, got_data->data, len) != 0) {
 			fprintf(stderr, "Wrong data for %d, '%s'\n",
 				i, osmo_hexdump(msg->l2h, msgb_l2len(msg)));
 			abort();
 		}
+
+		i += 1;
+	}
+
+	if (len != i) {
+		fprintf(stderr, "Failed to decode all packets: %d vs. %d\n", len, i);
 	}
 
 	msgb_free(out);
 }
 
+void test_compress()
+{
+	/**
+	 * This tests that we correctly encode/decode a marker bit
+	 */
+	test_compress_one(&test_scenarious[0], "Test marker");
+
+
+	/**
+	 * This tests the code path that will go through a smaller encoding
+	 */
+	test_compress_one(&test_scenarious[1], "No marker");
+
+}
+
 int main(int argc, char **argv)
 {
 	osmo_init_logging(&log_info);