jbuf: refactor jbuf_alloc min/max delay in ms

baresip · sreimers · Oct 8, 2023 · Oct 9, 2023 · Oct 12, 2023 · Oct 26, 2023
commit ba44d8e332ae685ff330b843a93203fae67d4abf
diff --git a/include/baresip.h b/include/baresip.h
@@ -428,11 +428,13 @@ struct config_avt {
 	bool rtcp_mux;          /**< RTP/RTCP multiplexing          */
 	struct {
 		enum jbuf_type jbtype;  /**< Jitter buffer type     */
-		struct range jbuf_del;  /**< Delay, number of frames*/
+		struct range jbuf_del;  /**< Max./Min. Delay [ms]   */
+		uint32_t jbuf_sz;       /**< Max. buffer  [packets] */
 	} audio;
 	struct {
 		enum jbuf_type jbtype;  /**< Jitter buffer type     */
-		struct range jbuf_del;  /**< Delay, number of frames*/
+		struct range jbuf_del;  /**< Max./Min. Delay [ms]   */
+		uint32_t jbuf_sz;       /**< Max. buffer  [packets] */
 	} video;
 	bool rtp_stats;         /**< Enable RTP statistics          */
 	uint32_t rtp_timeout;   /**< RTP Timeout in seconds (0=off) */
@@ -1556,7 +1558,8 @@ struct jbuf_stat {
 };
 
 
-int  jbuf_alloc(struct jbuf **jbp, uint32_t min, uint32_t max);
+int jbuf_alloc(struct jbuf **jbp, uint32_t mind, uint32_t maxd,
+	       uint32_t maxsz);
 void jbuf_set_srate(struct jbuf *jb, uint32_t srate);
 void jbuf_set_id(struct jbuf *jb, struct pl *id);
 int  jbuf_set_type(struct jbuf *jb, enum jbuf_type jbtype);

diff --git a/src/config.c b/src/config.c
@@ -76,11 +76,13 @@ static struct config core_config = {
 		.rtcp_mux = false,
 		.audio = {
 			JBUF_FIXED,
-			{5, 10},
+			{100, 500},
+			50
 		},
 		.video = {
 			JBUF_FIXED,
-			{5, 50},
+			{100, 500},
+			250
 		},
 		.rtp_stats = false,
 		.rtp_timeout = 0,
@@ -504,31 +506,24 @@ int config_parse_conf(struct config *cfg, const struct conf *conf)
 		cfg->avt.rtp_bw.max *= 1000;
 	}
 
-	if (0 == conf_get(conf, "jitter_buffer_type", &jbtype)) {
-		cfg->avt.video.jbtype = conf_get_jbuf_type(&jbtype);
-		cfg->avt.audio.jbtype = conf_get_jbuf_type(&jbtype);
-		warning("config: jitter_buffer_* config is deprecated, use "
-			"audio_jitter_buffer_* and "
-			"video_jitter_buffer_* options\n");
-	}
-
-	(void)conf_get_range(conf, "jitter_buffer_delay",
-			     &cfg->avt.video.jbuf_del);
-	(void)conf_get_range(conf, "jitter_buffer_delay",
-			     &cfg->avt.audio.jbuf_del);
-
 	if (0 == conf_get(conf, "audio_jitter_buffer_type", &jbtype))
 		cfg->avt.audio.jbtype = conf_get_jbuf_type(&jbtype);
 
-	(void)conf_get_range(conf, "audio_jitter_buffer_delay",
+	(void)conf_get_range(conf, "audio_jitter_buffer_delay_ms",
 			     &cfg->avt.audio.jbuf_del);
 
+	(void)conf_get_u32(conf, "audio_jitter_buffer_size",
+			   &cfg->avt.audio.jbuf_sz);
+
 	if (0 == conf_get(conf, "video_jitter_buffer_type", &jbtype))
 		cfg->avt.video.jbtype = conf_get_jbuf_type(&jbtype);
 
-	(void)conf_get_range(conf, "video_jitter_buffer_delay",
+	(void)conf_get_range(conf, "video_jitter_buffer_delay_ms",
 			     &cfg->avt.video.jbuf_del);
 
+	(void)conf_get_u32(conf, "audio_jitter_buffer_size",
+			   &cfg->avt.video.jbuf_sz);
+
 	(void)conf_get_bool(conf, "rtp_stats", &cfg->avt.rtp_stats);
 	(void)conf_get_u32(conf, "rtp_timeout", &cfg->avt.rtp_timeout);
 
@@ -684,9 +679,11 @@ int config_print(struct re_printf *pf, const struct config *cfg)
 			 "rtp_ports\t\t%H\n"
 			 "rtp_bandwidth\t\t%H\n"
 			 "audio_jitter_buffer_type\t%s\n"
-			 "audio_jitter_buffer_delay\t%H\n"
+			 "audio_jitter_buffer_delay_ms\t%H\n"
+			 "audio_jitter_buffer_size\t%u\n"
 			 "video_jitter_buffer_type\t%s\n"
-			 "video_jitter_buffer_delay\t%H\n"
+			 "video_jitter_buffer_delay_ms\t%H\n"
+			 "video_jitter_buffer_size\t%u\n"
 			 "rtp_stats\t\t%s\n"
 			 "rtp_timeout\t\t%u # in seconds\n"
 			 "avt_bundle\t\t%s\n"
@@ -703,8 +700,10 @@ int config_print(struct re_printf *pf, const struct config *cfg)
 			 range_print, &cfg->avt.rtp_bw,
 			 jbuf_type_str(cfg->avt.audio.jbtype),
 			 range_print, &cfg->avt.audio.jbuf_del,
+			 cfg->avt.audio.jbuf_sz,
 			 jbuf_type_str(cfg->avt.video.jbtype),
 			 range_print, &cfg->avt.video.jbuf_del,
+			 cfg->avt.video.jbuf_sz,
 			 cfg->avt.rtp_stats ? "yes" : "no",
 			 cfg->avt.rtp_timeout,
 			 cfg->avt.bundle ? "yes" : "no",
@@ -934,12 +933,14 @@ static int core_config_template(struct re_printf *pf, const struct config *cfg)
 			  "#rtp_bandwidth\t\t512-1024 # [kbit/s]\n"
 			  "audio_jitter_buffer_type\tfixed\t\t# off, fixed,"
 				" adaptive\n"
-			  "audio_jitter_buffer_delay\t%u-%u\t\t"
-					"# (min. frames)-(max. packets)\n"
+			  "audio_jitter_buffer_delay_ms\t%u-%u\t\t"
+				"# Min. - Max. [ms]\n"
+			  "audio_jitter_buffer_size\t50\t\t# [packets]\n"
 			  "video_jitter_buffer_type\tfixed\t\t# off, fixed,"
 				" adaptive\n"
-			  "video_jitter_buffer_delay\t%u-%u\t\t"
-					"# (min. frames)-(max. packets)\n"
+			  "video_jitter_buffer_delay_ms\t%u-%u\t\t"
+				"# Min. - Max. [ms]\n"
+			  "video_jitter_buffer_size\t250\t\t# [packets]\n"
 			  "rtp_stats\t\tno\n"
 			  "#rtp_timeout\t\t60\n"
 			  "#avt_bundle\t\tno\n"

diff --git a/src/jbuf.c b/src/jbuf.c
@@ -73,9 +73,9 @@ struct jbuf {
 	struct list pooll;   /**< List of free packets in pool               */
 	struct list packetl; /**< List of buffered packets                   */
 	uint32_t n;          /**< [# packets] Current # of packets in buffer */
-	uint32_t min;        /**< Minimum time in [ms] to buffer             */
-	uint32_t max;        /**< [# frames] Maximum # of frames to buffer   */
-	uint32_t wish;       /**< [# frames] Wish size for adaptive mode     */
+	uint32_t mind;       /**< Minimum time in [ms] to delay              */
+	uint32_t maxd;       /**< Maximum time in [ms] to delay              */
+	uint32_t maxsz;      /**< [# packets] Maximum # of packets to buffer */
 	uint16_t seq_put;    /**< Sequence number for last jbuf_put()        */
 	uint16_t seq_get;    /**< Sequence number of last played frame       */
 	uint32_t ssrc;       /**< Previous ssrc                              */
@@ -195,12 +195,13 @@ static void jbuf_destructor(void *data)
  * Allocate a new jitter buffer
  *
  * @param jbp    Pointer to returned jitter buffer
- * @param min    Minimum delay in [ms]
- * @param max    Maximum packets
+ * @param mind   Minimum delay in [ms]
+ * @param maxd   Maximum delay in [ms]
+ * @param maxsz  Maximum size in [packets]
  *
  * @return 0 if success, otherwise errorcode
  */
-int jbuf_alloc(struct jbuf **jbp, uint32_t min, uint32_t max)
+int jbuf_alloc(struct jbuf **jbp, uint32_t mind, uint32_t maxd, uint32_t maxsz)
 {
 	struct jbuf *jb;
 	uint32_t i;
@@ -223,9 +224,9 @@ int jbuf_alloc(struct jbuf **jbp, uint32_t min, uint32_t max)
 	list_init(&jb->packetl);
 
 	jb->jbtype	 = JBUF_FIXED;
-	jb->min		 = min;
-	jb->max		 = max;
-	jb->wish	 = min;
+	jb->mind	 = mind;
+	jb->maxd	 = maxd;
+	jb->maxsz	 = maxsz;
 	jb->next_play_fn = next_play;
 
 	DEBUG_INFO("alloc: delay=%u-%u packets\n", min, max);
@@ -238,7 +239,7 @@ int jbuf_alloc(struct jbuf **jbp, uint32_t min, uint32_t max)
 	mem_destructor(jb, jbuf_destructor);
 
 	/* Allocate all packets now */
-	for (i = 0; i < jb->max; i++) {
+	for (i = 0; i < jb->maxsz; i++) {
 		struct packet *f = mem_zalloc(sizeof(*f), NULL);
 		if (!f) {
 			err = ENOMEM;
@@ -437,7 +438,7 @@ static inline uint32_t offset(struct packet *p)
 
 static uint32_t calc_playout_time(struct jbuf *jb, struct packet *p)
 {
-	uint32_t base_offset = 0;
+	uint32_t jitter_offset = 0;
 	uint32_t play_time_base;
 
 	/* Fragmented frames (like video) have equal playout_time.
@@ -466,18 +467,24 @@ static uint32_t calc_playout_time(struct jbuf *jb, struct packet *p)
 		(void)adjust_due_to_skew(jb, p);
 
 		/* Add Jitter compensation */
-		base_offset += adjust_due_to_jitter(jb, p);
+		jitter_offset += adjust_due_to_jitter(jb, p);
 	}
 
 	/* Check min/max latency requirements */
-	base_offset += (jb->p.srate / 1000) * jb->min;
+	uint32_t min_lat = (jb->p.srate / 1000) * jb->mind;
+	uint32_t max_lat = (jb->p.srate / 1000) * jb->maxd;
+
+	if (jitter_offset < min_lat)
+		jitter_offset = min_lat;
+	else if (jitter_offset > max_lat)
+		jitter_offset = max_lat;
 
 	RE_TRACE_ID_INSTANT_I("jbuf", "recv_delay",
 			      delay_ms(jb->p.last_delay, jb->p.srate), jb->id);
 	RE_TRACE_ID_INSTANT_I("jbuf", "play_delay",
 			      delay_ms(base_offset, jb->p.srate), jb->id);
 
-	return play_time_base + base_offset;
+	return play_time_base + jitter_offset;
 }
 
 
@@ -904,8 +911,8 @@ int jbuf_debug(struct re_printf *pf, const struct jbuf *jb)
 
 	mtx_lock(jb->lock);
 	err |= mbuf_printf(mb, " running=%d", jb->running);
-	err |= mbuf_printf(mb, " min=%u cur=%u max=%u [packets]\n",
-			  jb->min, jb->n, jb->max);
+	err |= mbuf_printf(mb, " min=%ums cur=%u max=%ums [packets]\n",
+			  jb->mind, jb->n, jb->maxd);
 	err |= mbuf_printf(mb, " seq_put=%u\n", jb->seq_put);
 
 #if JBUF_STAT

diff --git a/src/rtprecv.c b/src/rtprecv.c
@@ -729,7 +729,7 @@ int rtprecv_alloc(struct rtp_receiver **rxp,
 	    cfg->audio.jbtype != JBUF_OFF && cfg->audio.jbuf_del.max) {
 
 		err = jbuf_alloc(&rx->jbuf, cfg->audio.jbuf_del.min,
-				 cfg->audio.jbuf_del.max);
+				 cfg->audio.jbuf_del.max, cfg->audio.jbuf_sz);
 		if (err)
 			goto out;
 
@@ -743,7 +743,7 @@ int rtprecv_alloc(struct rtp_receiver **rxp,
 	    cfg->video.jbtype != JBUF_OFF && cfg->video.jbuf_del.max) {
 
 		err = jbuf_alloc(&rx->jbuf, cfg->video.jbuf_del.min,
-				 cfg->video.jbuf_del.max);
+				 cfg->video.jbuf_del.max, cfg->audio.jbuf_sz);
 		if (err)
 			goto out;
 

diff --git a/test/jbuf.c b/test/jbuf.c
@@ -70,7 +70,7 @@ int test_jbuf(void)
 	void *mem = NULL;
 	int err;
 
-	err = jbuf_alloc(&jb, 0, 10);
+	err = jbuf_alloc(&jb, 0, 100, 10);
 	if (err)
 		return err;
 
@@ -190,10 +190,11 @@ int test_jbuf_adaptive(void)
 	struct jbuf *jb = NULL;
 	char *frv[4];
 	uint32_t min_lat = 100; /* [ms] */
+	uint32_t max_lat = 500; /* [ms] */
 	void *mem = NULL;
 	int err;
 
-	err = jbuf_alloc(&jb, min_lat, 10);
+	err = jbuf_alloc(&jb, min_lat, max_lat, 50);
 	TEST_ERR(err);
 	err = jbuf_set_type(jb, JBUF_ADAPTIVE);
 	TEST_ERR(err);
@@ -247,10 +248,11 @@ int test_jbuf_video(void)
 	struct jbuf *jb = NULL;
 	char *frv[4];
 	uint32_t min_lat = 100; /* [ms] */
+	uint32_t max_lat = 500; /* [ms] */
 	void *mem = NULL;
 	int err;
 
-	err = jbuf_alloc(&jb, min_lat, 10);
+	err = jbuf_alloc(&jb, min_lat, max_lat, 50);
 	TEST_ERR(err);
 	err = jbuf_set_type(jb, JBUF_ADAPTIVE);
 	TEST_ERR(err);