[Sound-open-firmware] [PATCH] SRC: Sink and source buffers handling bug fixes
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@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;
On Tue, 2017-09-12 at 13:45 +0300, Seppo Ingalsuo wrote:
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@linux.intel.com
src/audio/src.c | 34 ++++++++++++++++++++++++++++------ src/audio/src_core.c | 23 +++++++++++------------ 2 files changed, 39 insertions(+), 18 deletions(-)
Applied.
Thanks
Liam
--------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
participants (2)
-
Liam Girdwood
-
Seppo Ingalsuo