[Sound-open-firmware] [PATCH] SRC: Sink and source buffers handling bug fixes

[Seppo Ingalsuo]

This patch improves robustness to various buffering settings by adding
more check for sufficient buffers sizes vs. internal block processing
length of SRC. The resize of sink buffer is changed to allow at least
internal output block length plus one period to the buffer. The bug in
rejecting disabled SRC modes is fixed in SRC core module.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo at linux.intel.com>
---
 src/audio/src.c      | 34 ++++++++++++++++++++++++++++------
 src/audio/src_core.c | 23 +++++++++++------------
 2 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/src/audio/src.c b/src/audio/src.c
index 54c798e..2ecf025 100644
--- a/src/audio/src.c
+++ b/src/audio/src.c
@@ -60,7 +60,7 @@ struct comp_data {
 	int scratch_length;
 	uint32_t sink_rate;
 	uint32_t source_rate;
-	uint32_t period_bytes;	/* sink period */
+	uint32_t period_bytes; /* sink period */
 	void (*src_func)(struct comp_dev *dev,
 		struct comp_buffer *source,
 		struct comp_buffer *sink,
@@ -308,12 +308,12 @@ static int src_params(struct comp_dev *dev)
 	struct sof_ipc_comp_src *src = COMP_GET_IPC(dev, sof_ipc_comp_src);
 	struct sof_ipc_comp_config *config = COMP_GET_CONFIG(dev);
 	struct comp_data *cd = comp_get_drvdata(dev);
-	struct comp_buffer *sink;
+	struct comp_buffer *sink, *source;
 	struct src_alloc need;
 	size_t delay_lines_size;
 	uint32_t source_rate, sink_rate;
 	int32_t *buffer_start;
-	int n = 0, i, err, frames_is_for_source;
+	int n = 0, i, err, frames_is_for_source, q;
 
 	trace_src("par");
 
@@ -405,9 +405,22 @@ static int src_params(struct comp_dev *dev)
 		dev->params.sample_container_bytes * dev->params.channels;
 	cd->period_bytes = dev->frames * dev->frame_bytes;
 
-	/* configure downstream buffer */
-	sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
-	err = buffer_set_size(sink, cd->period_bytes * config->periods_sink);
+	/* The downstream buffer must be at least length of blk_out plus
+	 * dev->frames and an integer multiple of dev->frames. The
+	 * buffer_set_size will return an error if the required length would
+	 * be too long.
+	 */
+	q = need.blk_out / dev->frames;
+	if (q * dev->frames < need.blk_out)
+		++q;
+
+	if (q * dev->frames < need.blk_out + dev->frames)
+		++q;
+
+	/* Configure downstream buffer */
+	sink = list_first_item(&dev->bsink_list, struct comp_buffer,
+		source_list);
+	err = buffer_set_size(sink, q * dev->frames * dev->frame_bytes);
 	if (err < 0) {
 		trace_src_error("eSz");
 		return err;
@@ -415,6 +428,15 @@ static int src_params(struct comp_dev *dev)
 
 	buffer_reset_pos(sink);
 
+	/* Check that source buffer has sufficient size */
+	source = list_first_item(&dev->bsource_list, struct comp_buffer,
+		sink_list);
+	if (source->size < need.blk_in * dev->frame_bytes) {
+		trace_src_error("eSy");
+		return -EINVAL;
+	}
+
+
 	return 0;
 }
 
diff --git a/src/audio/src_core.c b/src/audio/src_core.c
index cc44734..a9331a0 100644
--- a/src/audio/src_core.c
+++ b/src/audio/src_core.c
@@ -151,10 +151,12 @@ int src_buffer_lengths(struct src_alloc *a, int fs_in, int fs_out, int nch,
 		k = max_frames / blk_out;
 	}
 
-	/* Return with error if max_frames is too small even for smallest
-	   possible SRC block length. */
+	/* Mininum k is 1, when 0 max_frames is less than block length. In
+	 * that case need to check in src.c that sink/source size is large
+	 * enough for at least one block.
+	 */
 	if (k < 1)
-		return -EINVAL;
+		k = 1;
 
 	a->blk_mult = k;
 	a->blk_in = blk_in * k;
@@ -209,9 +211,6 @@ static int init_stages(
 		if (stage1->blk_out == 0)
 			return -EINVAL;
 	} else {
-		if ((stage1->blk_out == 0) || (stage1->blk_in == 0))
-			return -EINVAL;
-
 		src->stage1_times = res->stage1_times;
 		src->stage2_times = res->stage2_times;
 		src->blk_in = res->blk_in;
@@ -219,14 +218,14 @@ static int init_stages(
 	}
 
 	/* Delay line sizes */
-	src->state1.fir_delay_size = res->fir_s1; //src_fir_delay_length(stage1);
-	src->state1.out_delay_size = res->out_s1; //src_out_delay_length(stage1);
+	src->state1.fir_delay_size = res->fir_s1;
+	src->state1.out_delay_size = res->out_s1;
 	src->state1.fir_delay = delay_lines_start;
 	src->state1.out_delay =
 		src->state1.fir_delay + src->state1.fir_delay_size;
 	if (n > 1) {
-		src->state2.fir_delay_size = res->fir_s2; // src_fir_delay_length(stage2);
-		src->state2.out_delay_size = res->out_s2; // src_out_delay_length(stage2);
+		src->state2.fir_delay_size = res->fir_s2;
+		src->state2.out_delay_size = res->out_s2;
 		src->state2.fir_delay =
 			src->state1.out_delay + src->state1.out_delay_size;
 		src->state2.out_delay =
@@ -289,7 +288,7 @@ int src_polyphase_init(struct polyphase_src *src, int fs1, int fs2,
 		return -EINVAL;
 
 	/* Get number of stages used for optimize opportunity. 2nd
-	 * stage lenth is one if conversion needs only one stage.
+	 * stage length is one if conversion needs only one stage.
 	 */
 	n_stages = (src->stage2->filter_length == 1) ? 1 : 2;
 
@@ -297,7 +296,7 @@ int src_polyphase_init(struct polyphase_src *src, int fs1, int fs2,
 	 * mode from in/out matrix. Computing of such SRC mode needs
 	 * to be prevented.
 	 */
-	if (src->stage2->filter_length == 0)
+	if (src->stage1->filter_length == 0)
 		return -EINVAL;
 
 	return n_stages;
-- 
2.11.0