[PATCH] libosmo-netif[master]: jibuf: Take RTP marker into account

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Fri Apr 13 14:19:07 UTC 2018


Hello Jenkins Builder,

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

    https://gerrit.osmocom.org/7778

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

jibuf: Take RTP marker into account

Change-Id: Ie142acfb45650e0af775f58226fd191beaf8178e
---
M src/jibuf.c
M tests/jibuf/jibuf_test.c
M tests/jibuf/jibuf_test.ok
M tests/jibuf/jibuf_tool.c
4 files changed, 89 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/78/7778/2

diff --git a/src/jibuf.c b/src/jibuf.c
index c3d6bad..dfeb12e 100644
--- a/src/jibuf.c
+++ b/src/jibuf.c
@@ -115,6 +115,14 @@
 
 }
 
+static bool msg_get_marker(struct msgb *msg)
+{
+	/* TODO: make it more generic as a callback so that different types of
+	 * pkts can be used ? */
+	struct rtp_hdr *rtph = osmo_rtp_get_hdr(msg);
+	return rtph->marker;
+}
+
 static uint16_t msg_get_sequence(struct msgb *msg)
 {
 	struct rtp_hdr *rtph = osmo_rtp_get_hdr(msg);
@@ -275,7 +283,11 @@
 
 	clock_gettime_timeval(CLOCK_MONOTONIC, &jb->last_enqueue_time);
 
-	if (!jb->started) {
+	/* If packet contains a mark -> start of talkspurt.
+	 * A lot of packets may have been suppressed by the sender before it,
+	 * so let's take it as a reference
+	 */
+	if (!jb->started || msg_get_marker(msg)) {
 		jb->started = true;
 		msg_set_as_reference(jb, msg);
 		rel_delay = 0;
@@ -310,10 +322,10 @@
 	timeradd(&jb->last_enqueue_time, &delay_ts, &sched_ts);
 
 	LOGP(DLJIBUF, LOGL_DEBUG, "enqueuing packet seq=%"PRIu16" rel=%d delay=%d" \
-		" thres=%d {%lu.%06lu -> %lu.%06lu}\n",
+		" thres=%d {%lu.%06lu -> %lu.%06lu} %s\n",
 		msg_get_sequence(msg), rel_delay, delay, jb->threshold_delay,
 		jb->last_enqueue_time.tv_sec, jb->last_enqueue_time.tv_usec,
-		sched_ts.tv_sec, sched_ts.tv_usec);
+		sched_ts.tv_sec, sched_ts.tv_usec, msg_get_marker(msg)? "M" : "");
 
 	/* Add scheduled dequeue time in msg->cb so we can check it later */
 	unsigned long *old_cb = talloc_memdup(jb->talloc_ctx, msg->cb, sizeof(msg->cb));
diff --git a/tests/jibuf/jibuf_test.c b/tests/jibuf/jibuf_test.c
index ba21906..9965387 100644
--- a/tests/jibuf/jibuf_test.c
+++ b/tests/jibuf/jibuf_test.c
@@ -568,6 +568,56 @@
 	osmo_jibuf_delete(jb);
 }
 
+static void test_rtp_marker(void)
+{
+	int min_delay = 60;
+	struct msgb *msg;
+	struct rtp_hdr *rtph;
+
+	printf("===test_rtp_marker===\n");
+
+	clock_override_enable(true);
+	clock_override_set(0, 0);
+	rtp_init(32, 400);
+	jb = osmo_jibuf_alloc(NULL);
+	osmo_jibuf_set_dequeue_cb(jb, dequeue_cb, NULL);
+	osmo_jibuf_set_min_delay(jb, min_delay);
+	osmo_jibuf_set_max_delay(jb, 200);
+
+	/* First rtp at t=0, should be scheduled in min_delay time */
+	clock_debug("enqueue 1st packet");
+	ENQUEUE_NEXT(jb);
+	clock_override_add(0, TIME_RTP_PKT_MS*1000);
+	clock_debug("enqueue 2nd packet");
+	ENQUEUE_NEXT(jb);
+	clock_override_add(0, min_delay*1000);
+	clock_debug("2 packets dequeued");
+	osmo_select_main(0);
+
+	clock_override_add(0, 40*1000);
+	 /* We are at t=120, next non-marked (consecutive seq) packet arriving at
+	  * this time should be dropped, but since marker establishes new ref,
+	  * it will be accepted as well an ext paket */
+	clock_debug("enqueue late pkt with marker=1, will be enqueued");
+	msg = rtp_next();
+	rtph = osmo_rtp_get_hdr(msg);
+	rtph->marker = 1;
+	OSMO_ASSERT(osmo_jibuf_enqueue(jb, msg) == 0);
+
+	clock_debug("enqueue late pkt after pkt with marker=1, will be enqueued");
+	clock_override_add(0, TIME_RTP_PKT_MS*1000);
+	ENQUEUE_NEXT(jb);
+
+	clock_debug("2 packets dequeued");
+	clock_override_add(0, min_delay*1000);
+	osmo_select_main(0);
+
+	/* t=120, 4 enqueued, 4 dequeued.*/
+	OSMO_ASSERT(osmo_jibuf_empty(jb));
+
+	osmo_jibuf_delete(jb);
+}
+
 int main(int argc, char **argv)
 {
 
@@ -592,6 +642,7 @@
 	test_buffer_threshold_change();
 	test_seq_wraparound();
 	test_timestamp_wraparound();
+	test_rtp_marker();
 
 	fprintf(stdout, "OK: Test passed\n");
 	return EXIT_SUCCESS;
diff --git a/tests/jibuf/jibuf_test.ok b/tests/jibuf/jibuf_test.ok
index 5a0f121..3103781 100644
--- a/tests/jibuf/jibuf_test.ok
+++ b/tests/jibuf/jibuf_test.ok
@@ -348,4 +348,21 @@
 sys={0.160000}, mono={0.160000}: clock_override_add
 sys={0.160000}, mono={0.160000}: dequeue 5th packet (ts=334)
 sys={0.160000}, mono={0.160000}: dequeue: seq=37 ts=334 LATEST
+===test_rtp_marker===
+sys={0.000000}, mono={0.000000}: clock_override_set
+sys={0.000000}, mono={0.000000}: enqueue 1st packet
+sys={0.020000}, mono={0.020000}: clock_override_add
+sys={0.020000}, mono={0.020000}: enqueue 2nd packet
+sys={0.080000}, mono={0.080000}: clock_override_add
+sys={0.080000}, mono={0.080000}: 2 packets dequeued
+sys={0.080000}, mono={0.080000}: dequeue: seq=33 ts=560 INTERMEDIATE
+sys={0.080000}, mono={0.080000}: dequeue: seq=34 ts=720 LATEST
+sys={0.120000}, mono={0.120000}: clock_override_add
+sys={0.120000}, mono={0.120000}: enqueue late pkt with marker=1, will be enqueued
+sys={0.120000}, mono={0.120000}: enqueue late pkt after pkt with marker=1, will be enqueued
+sys={0.140000}, mono={0.140000}: clock_override_add
+sys={0.140000}, mono={0.140000}: 2 packets dequeued
+sys={0.200000}, mono={0.200000}: clock_override_add
+sys={0.200000}, mono={0.200000}: dequeue: seq=35 ts=880 INTERMEDIATE
+sys={0.200000}, mono={0.200000}: dequeue: seq=36 ts=1040 LATEST
 OK: Test passed
diff --git a/tests/jibuf/jibuf_tool.c b/tests/jibuf/jibuf_tool.c
index d24086c..00c6f7e 100644
--- a/tests/jibuf/jibuf_tool.c
+++ b/tests/jibuf/jibuf_tool.c
@@ -218,7 +218,9 @@
 	gettimeofday(&pinfo->postqueue.ts, NULL);
 	pinfo->postqueue.timestamp = htonl(rtph->timestamp);
 
-	if (postqueue_started) {
+	/* If pkt->marker -> init of talkspurt, there may be missing packets before,
+	 * better to start calculating the jitter from here */
+	if (postqueue_started && !rtph->marker) {
 		/* In random test mode we now the sender time, so we get real
 		 * jitter results using it */
 		if(opt_test_rand) {
@@ -269,7 +271,9 @@
 	gettimeofday(&pinfo->prequeue.ts, NULL);
 	pinfo->prequeue.timestamp = htonl(rtph->timestamp);
 
-	if (prequeue_started) {
+	/* If pkt->marker -> init of talkspurt, there may be missing packets before,
+	 * better to start calculating the jitter from here */
+	if (prequeue_started && !rtph->marker) {
 		/* In random test mode we now the sender time, so we get real
 		 * jitter results using it */
 		if(opt_test_rand) {

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie142acfb45650e0af775f58226fd191beaf8178e
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder



More information about the gerrit-log mailing list