From 4b79ef638214cdd5960a84a7088a49f31d47605b Mon Sep 17 00:00:00 2001 From: Curtis Malainey Date: Fri, 18 Mar 2022 17:33:05 -0700 Subject: [PATCH] pipeline: params: don't use uninitialized hw_params pipeline_comp_hw_params does 2 things, it fetches the hw_params from the DAI and writes those parameters to buffers. The issue here is that we walk from host to DAI, so any buffer we see along the way gets set with uninitialized hw_params as we don't get the params till the end of the walk. Typically this is not an issue in most pipelines that are linear as the following call to pipeline_comp_params will overwrite any bad values in the buffer params. Not good practice but not harmful. The problem arises when components such as the demux or SRC which pass flags into comp_verify_params receive these bad values. There are edge cases as a result where the values are not cleared and the buffers end up with bad data. The result is usually an xrun, in the case of identifying this bug, the uncleared buffer had 4.6k channels. Signed-off-by: Curtis Malainey --- src/audio/pipeline/pipeline-params.c | 30 +++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/audio/pipeline/pipeline-params.c b/src/audio/pipeline/pipeline-params.c index 2d2e0d840eee..295e9bf68ef5 100644 --- a/src/audio/pipeline/pipeline-params.c +++ b/src/audio/pipeline/pipeline-params.c @@ -139,9 +139,7 @@ static void pipeline_update_buffer_pcm_params(struct comp_buffer *buffer, params->chmap[i] = buffer->chmap[i]; } -/* fetch hardware stream parameters from DAI and propagate them to the remaining - * buffers in pipeline. - */ +/* fetch hardware stream parameters from DAI */ static int pipeline_comp_hw_params(struct comp_dev *current, struct comp_buffer *calling_buf, struct pipeline_walk_context *ctx, int dir) @@ -168,6 +166,20 @@ static int pipeline_comp_hw_params(struct comp_dev *current, } } + return ret; +} + +/* propagate hw_params to buffers in pipeline. */ +static int pipeline_comp_hw_params_buf(struct comp_dev *current, + struct comp_buffer *calling_buf, + struct pipeline_walk_context *ctx, int dir) +{ + struct pipeline_data *ppl_data = ctx->comp_data; + int ret; + + ret = pipeline_for_each_comp(current, ctx, dir); + if (ret < 0) + return ret; /* set buffer parameters */ if (calling_buf) { calling_buf = buffer_acquire(calling_buf); @@ -207,6 +219,11 @@ int pipeline_params(struct pipeline *p, struct comp_dev *host, .comp_data = &data, .skip_incomplete = true, }; + struct pipeline_walk_context buf_param_ctx = { + .comp_func = pipeline_comp_hw_params_buf, + .comp_data = &data, + .skip_incomplete = true, + }; struct pipeline_walk_context param_ctx = { .comp_func = pipeline_comp_params, .comp_data = &data, @@ -232,6 +249,13 @@ int pipeline_params(struct pipeline *p, struct comp_dev *host, return ret; } + ret = buf_param_ctx.comp_func(host, NULL, &buf_param_ctx, dir); + if (ret < 0) { + pipe_err(p, "pipeline_params(): ret = %d, dev->comp.id = %u", + ret, dev_comp_id(host)); + return ret; + } + /* setting pcm params */ data.params = params; data.start = host;