From: Andre Noll <maan@systemlinux.org>
Date: Sat, 24 Oct 2009 19:03:06 +0000 (+0200)
Subject: vss: Fix computation of extra slices.
X-Git-Tag: v0.4.0~6^2
X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=3f9051d22fcf6e968259946eaf521ac3daac470e

vss: Fix computation of extra slices.

On stream start we check in setup_next_fec_group() whether the FEC
parameters requested by the client are sufficient for the current audio
file. We want each FEC group contain at least one non-header slice.

Since header slices can not contain any non-header data, the number
of required slices is given by the number of slices needed for the
header plus the number of slices needed for the largest chunk of the
audio file.

The current code gets this wrong because it computes the number of
slices needed for the header plus the largest chunk, which may be
strictly less than what is actually needed. IOW, the inequality

  num_slices(header + chunk) <= num_slices(header) + num_slices(chunk)

may be strict.

This bug could trigger the assertion assert(g->num_chunks) further
down in setup_next_fec_group().

This patch fixes this bug and also changes a log message in
udp_init_session() which always printed the requested FEC parameters,
not considering extra slices.
---

diff --git a/udp_send.c b/udp_send.c
index 7217c960..7d13f148 100644
--- a/udp_send.c
+++ b/udp_send.c
@@ -179,9 +179,7 @@ static int udp_init_session(struct udp_target *ut)
 	}
 	add_close_on_fork_list(ut->fd);
 	ut->cq = cq_new(UDP_CQ_BYTES);
-	PARA_NOTICE_LOG("sending to udp %s#%d using fec parms %d:%d:%d\n",
-		ut->host, ut->port , ut->fcp.max_slice_bytes,
-		ut->fcp.data_slices_per_group, ut->fcp.slices_per_group);
+	PARA_NOTICE_LOG("sending to udp %s#%d\n", ut->host, ut->port);
 	return 1;
 }
 
diff --git a/vss.c b/vss.c
index aae601d1..6db42bde 100644
--- a/vss.c
+++ b/vss.c
@@ -254,38 +254,39 @@ static int setup_next_fec_group(struct fec_client *fc, struct vss_task *vsst)
 	uint32_t max_data_size;
 
 	assert(chunk_tv);
-	k = fc->fcp->data_slices_per_group + fc->num_extra_slices;
 	if (fc->first_stream_chunk < 0) {
-		uint32_t largest = afh_get_largest_chunk_size(&mmd->afd.afhi)
-			+ vsst->header_len;
-		uint8_t needed, want;
+		uint8_t hs, ds; /* needed header/data slices */
+		uint8_t rs = fc->fcp->slices_per_group
+			- fc->fcp->data_slices_per_group; /* redundant slices */
+		int n;
 
-		ret = num_slices(largest, fc, &needed);
+		ret = num_slices(vsst->header_len, fc, &hs);
+		if (ret < 0)
+			return ret;
+		ret = num_slices(afh_get_largest_chunk_size(&mmd->afd.afhi),
+			fc, &ds);
+		if (ret < 0)
+			return ret;
+		k = (int)hs + ds;
+		if (k > 255)
+			return -E_BAD_CT;
+		if (k < fc->fcp->data_slices_per_group)
+			k = fc->fcp->data_slices_per_group;
+		n = k + rs;
+		fc->num_extra_slices = k - fc->fcp->data_slices_per_group;
+		PARA_NOTICE_LOG("fec parms %d:%d:%d (%d extra slices)\n",
+			slice_bytes, k, n, fc->num_extra_slices);
+		fec_free(fc->parms);
+		fc->src_data = para_realloc(fc->src_data, k * sizeof(char *));
+		ret = fec_new(k, n, &fc->parms);
 		if (ret < 0)
 			return ret;
-		if (needed > fc->fcp->data_slices_per_group)
-			PARA_WARNING_LOG("fec parms insufficient for this audio file\n");
-		want = PARA_MAX(needed, fc->fcp->data_slices_per_group);
-		if (want != k) {
-			fec_free(fc->parms);
-			fc->src_data = para_realloc(fc->src_data, want * sizeof(char *));
-			ret = fec_new(want, want + fc->fcp->slices_per_group
-				- fc->fcp->data_slices_per_group, &fc->parms);
-			if (ret < 0)
-				return ret;
-			k = want;
-			fc->num_extra_slices = 0;
-			if (k > fc->fcp->data_slices_per_group) {
-				fc->num_extra_slices = k - fc->fcp->data_slices_per_group;
-				PARA_NOTICE_LOG("using %d extra slices\n",
-					fc->num_extra_slices);
-			}
-		}
 		fc->stream_start = *now;
 		fc->first_stream_chunk = mmd->current_chunk;
 		g->first_chunk = mmd->current_chunk;
 		g->num = 0;
 	} else {
+		k = fc->fcp->data_slices_per_group + fc->num_extra_slices;
 		/* use duration of the previous group for the timing of this group */
 		set_slice_duration(fc, g);
 		g->first_chunk += g->num_chunks;