osmux: Fix AMR F,Q,CMR fields not properly encoded in osmux header

The value of the first RTP packet in the batch was being encoded,
instead of the last one (as documented in the Osmux protocol
specification).

Related: SYS#5987
Change-Id: I06f3d07a05181d938c22bbd0da7172b5dad6659a

3 files changed, 115 insertions, 8 deletions

diff --git a/src/osmux.c b/src/osmux.c
index 07d2b2e..419645c 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -364,20 +364,16 @@ struct osmux_input_state {
 static int osmux_batch_put(struct osmux_batch *batch,
 			   struct osmux_input_state *state)
 {
-	struct osmux_hdr *osmuxh;
-
 	if (state->add_osmux_hdr) {
-		osmuxh = (struct osmux_hdr *)state->out_msg->tail;
+		struct osmux_hdr *osmuxh;
+		osmuxh = (struct osmux_hdr *)msgb_put(state->out_msg,
+						      sizeof(struct osmux_hdr));
 		osmuxh->ft = OSMUX_FT_VOICE_AMR;
 		osmuxh->ctr = 0;
 		osmuxh->rtp_m = osmuxh->rtp_m || state->rtph->marker;
-		osmuxh->amr_f = state->amrh->f;
-		osmuxh->amr_q= state->amrh->q;
 		osmuxh->seq = batch->seq++;
 		osmuxh->circuit_id = state->ccid;
-		osmuxh->amr_cmr = state->amrh->cmr;
 		osmuxh->amr_ft = state->amrh->ft;
-		msgb_put(state->out_msg, sizeof(struct osmux_hdr));
 
 		/* annotate current osmux header */
 		batch->osmuxh = osmuxh;
@@ -390,6 +386,10 @@ static int osmux_batch_put(struct osmux_batch *batch,
 		}
 		batch->osmuxh->ctr++;
 	}
+	/* For fields below, we only use the last included in batch and ignore any previous: */
+	batch->osmuxh->amr_cmr = state->amrh->cmr;
+	batch->osmuxh->amr_f = state->amrh->f;
+	batch->osmuxh->amr_q = state->amrh->q;
 
 	memcpy(state->out_msg->tail, osmo_amr_get_payload(state->amrh),
 	       state->amr_payload_len);
diff --git a/tests/osmux/osmux_input_test.c b/tests/osmux/osmux_input_test.c
index 3b7774c..ff5379a 100644
--- a/tests/osmux/osmux_input_test.c
+++ b/tests/osmux/osmux_input_test.c
@@ -85,7 +85,7 @@ static struct msgb *rtp_next(void)
 	return rtp_new(rtp_next_seq, rtp_next_ts, 0);
 }
 
-static void rtp_append_amr(struct msgb *msg, uint8_t ft)
+static struct amr_hdr *rtp_append_amr(struct msgb *msg, uint8_t ft)
 {
 	struct amr_hdr *amrh;
 	struct rtp_hdr *rtph = (struct rtp_hdr *)msg->data;
@@ -98,6 +98,7 @@ static void rtp_append_amr(struct msgb *msg, uint8_t ft)
 	amrh->f = 0;
 	amrh->ft = ft;
 	msgb_put(msg, osmo_amr_bytes(amrh->ft));
+	return amrh;
 }
 
 static void sigalarm_handler(int foo)
@@ -251,6 +252,100 @@ static void test_amr_ft_change_middle_batch(void)
 	osmux_xfrm_input_fini(&h_input);
 }
 
+static void test_last_amr_cmr_f_q_used_osmux_deliver_cb(struct msgb *batch_msg, void *data)
+{
+	struct osmux_hdr *osmuxh;
+	char buf[2048];
+	bool *osmux_transmitted = (bool *)data;
+
+	osmux_snprintf(buf, sizeof(buf), batch_msg);
+	clock_debug("OSMUX message (len=%d): %s\n", batch_msg->len, buf);
+
+	/* We expect 1 batch: */
+	osmuxh = osmux_xfrm_output_pull(batch_msg);
+	OSMO_ASSERT(osmuxh->ft == OSMUX_FT_VOICE_AMR);
+	/* Check CMR and Q values are the ones from the last message: */
+	OSMO_ASSERT(osmuxh->amr_f == 0);
+	OSMO_ASSERT(osmuxh->amr_q == 0);
+	OSMO_ASSERT(osmuxh->amr_cmr == 2);
+
+	osmuxh = osmux_xfrm_output_pull(batch_msg);
+	OSMO_ASSERT(osmuxh == NULL);
+
+	msgb_free(batch_msg);
+
+	*osmux_transmitted = true;
+}
+/* Test that fields CMR, F and Q of the last RTP packet in the batch are the
+ * ones set in the osmux batch header. */
+static void test_last_amr_cmr_f_q_used(void)
+{
+	struct msgb *msg;
+	int rc;
+	const uint8_t cid = 32;
+	bool osmux_transmitted = false;
+	struct amr_hdr *amrh;
+
+	printf("===%s===\n", __func__);
+
+
+
+	clock_override_enable(true);
+	clock_override_set(0, 0);
+	rtp_init(0, 0);
+
+	struct osmux_in_handle h_input = {
+	.osmux_seq	= 0, /* sequence number to start OSmux message from */
+	.batch_factor	= 3, /* batch up to 3 RTP messages */
+	.deliver	= test_last_amr_cmr_f_q_used_osmux_deliver_cb,
+	.data		= &osmux_transmitted,
+	};
+
+	osmux_xfrm_input_init(&h_input);
+	osmux_xfrm_input_open_circuit(&h_input, cid, false);
+
+	/* First RTP frame at t=0 */
+	msg = rtp_next();
+	amrh = rtp_append_amr(msg, AMR_FT_2);
+	amrh->f = 1;
+	amrh->q = 1;
+	amrh->cmr = 0;
+	rc = osmux_xfrm_input(&h_input, msg, cid);
+	OSMO_ASSERT(rc == 0);
+
+	/* Second RTP frame at t=20, CMR changes 0->1 */
+	clock_debug("Submit 2nd RTP packet, CMR changes");
+	clock_override_add(0, TIME_RTP_PKT_MS*1000);
+	msg = rtp_next();
+	amrh = rtp_append_amr(msg, AMR_FT_2);
+	amrh->f = 1;
+	amrh->q = 1;
+	amrh->cmr = 1;
+	rc = osmux_xfrm_input(&h_input, msg, cid);
+	OSMO_ASSERT(rc == 0);
+
+	/* Third RTP frame at t=40, q changes 1->0, CMR changes 1->2: */
+	clock_debug("Submit 3rd RTP packet with Q and CMR changes");
+	clock_override_add(0, TIME_RTP_PKT_MS*1000);
+	msg = rtp_next();
+	amrh = rtp_append_amr(msg, AMR_FT_2);
+	amrh->f = 0;
+	amrh->q = 0;
+	amrh->cmr = 2;
+	rc = osmux_xfrm_input(&h_input, msg, cid);
+	OSMO_ASSERT(rc == 0);
+
+	/* t=60, osmux batch is scheduled to be transmitted: */
+	clock_override_add(0, TIME_RTP_PKT_MS*1000);
+	clock_debug("Osmux frame should now be transmitted");
+	osmo_select_main(0);
+	OSMO_ASSERT(osmux_transmitted == true);
+
+	clock_debug("Closing circuit");
+	osmux_xfrm_input_close_circuit(&h_input, cid);
+	osmux_xfrm_input_fini(&h_input);
+}
+
 int main(int argc, char **argv)
 {
 
@@ -269,6 +364,7 @@ int main(int argc, char **argv)
 	alarm(10);
 
 	test_amr_ft_change_middle_batch();
+	test_last_amr_cmr_f_q_used();
 
 	fprintf(stdout, "OK: Test passed\n");
 	return EXIT_SUCCESS;
diff --git a/tests/osmux/osmux_input_test.ok b/tests/osmux/osmux_input_test.ok
index 9aca52e..0464fd4 100644
--- a/tests/osmux/osmux_input_test.ok
+++ b/tests/osmux/osmux_input_test.ok
@@ -10,4 +10,15 @@ sys={0.080000}, mono={0.080000}: Osmux frame should now be transmitted
 sys={0.080000}, mono={0.080000}: OSMUX message (len=81): OSMUX seq=000 ccid=030 ft=1 rtp_m=0 ctr=1 amr_f=0 amr_q=1 amr_ft=02 amr_cmr=00 [ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ], OSMUX seq=001 ccid=030 ft=1 rtp_m=0 ctr=0 amr_f=0 amr_q=1 amr_ft=06 amr_cmr=00 [ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ], OSMUX seq=002 ccid=030 ft=1 rtp_m=0 ctr=0 amr_f=0 amr_q=1 amr_ft=01 amr_cmr=00 [ 00 00 00 00 00 00 00 00 00 00 00 00 00 ]
 
 sys={0.080000}, mono={0.080000}: Closing circuit
+===test_last_amr_cmr_f_q_used===
+sys={0.000000}, mono={0.000000}: clock_override_set
+sys={0.000000}, mono={0.000000}: Submit 2nd RTP packet, CMR changes
+sys={0.020000}, mono={0.020000}: clock_override_add
+sys={0.020000}, mono={0.020000}: Submit 3rd RTP packet with Q and CMR changes
+sys={0.040000}, mono={0.040000}: clock_override_add
+sys={0.060000}, mono={0.060000}: clock_override_add
+sys={0.060000}, mono={0.060000}: Osmux frame should now be transmitted
+sys={0.060000}, mono={0.060000}: OSMUX message (len=49): OSMUX seq=000 ccid=032 ft=1 rtp_m=0 ctr=2 amr_f=0 amr_q=0 amr_ft=02 amr_cmr=02 [ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ]
+
+sys={0.060000}, mono={0.060000}: Closing circuit
 OK: Test passed