From 6d3cba437261d2e6728aca68c0efbcb6b62025f7 Mon Sep 17 00:00:00 2001 From: Paul Olaru Date: Fri, 31 Mar 2023 13:16:10 +0300 Subject: [PATCH 1/2] audio: mixer: Improve handling of inactive input streams The old code does not deal correctly with input streams going away without everything stopping. This commit ensures only active streams are considered as mixer input, and the other streams are ignored (considered zeroed out). Signed-off-by: Paul Olaru --- src/audio/mixer/mixer.c | 32 +++++++++++++++++++------------- src/include/sof/audio/mixer.h | 2 -- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/audio/mixer/mixer.c b/src/audio/mixer/mixer.c index b98645195a7a..32e3b564b053 100644 --- a/src/audio/mixer/mixer.c +++ b/src/audio/mixer/mixer.c @@ -82,11 +82,12 @@ static int mixer_process(struct processing_module *mod, struct mixer_data *md = module_get_private_data(mod); struct comp_dev *dev = mod->dev; const struct audio_stream __sparse_cache *sources_stream[PLATFORM_MAX_STREAMS]; - int32_t i = 0; + int32_t i = 0, j = 0; uint32_t frames = INT32_MAX; /* Redundant, but helps the compiler */ uint32_t source_bytes = 0; uint32_t sink_bytes; + int active_input_buffers = 0; comp_dbg(dev, "mixer_process() %d", num_input_buffers); @@ -105,10 +106,11 @@ static int mixer_process(struct processing_module *mod, if (avail_frames == 0) continue; + active_input_buffers++; frames = MIN(frames, avail_frames); } - if (!num_input_buffers || (frames == 0 && md->sources_inactive)) { + if (!active_input_buffers) { /* * Generate silence when sources are inactive. When * sources change to active, additionally keep @@ -119,33 +121,37 @@ static int mixer_process(struct processing_module *mod, if (!audio_stream_set_zero(mod->output_buffers[0].data, sink_bytes)) mod->output_buffers[0].size = sink_bytes; - md->sources_inactive = true; - return 0; } /* Every source has the same format, so calculate bytes based on the first one */ source_bytes = frames * audio_stream_frame_bytes(mod->input_buffers[0].data); - if (md->sources_inactive) { - md->sources_inactive = false; - comp_dbg(dev, "mixer_copy exit sources_inactive state"); - } - sink_bytes = frames * audio_stream_frame_bytes(mod->output_buffers[0].data); comp_dbg(dev, "mixer_process(), source_bytes = 0x%x, sink_bytes = 0x%x", source_bytes, sink_bytes); /* mix streams */ - for (i = 0; i < num_input_buffers; i++) - sources_stream[i] = mod->input_buffers[i].data; + for (i = 0; i < num_input_buffers; i++) { + uint32_t avail_frames; + + avail_frames = audio_stream_avail_frames_aligned(mod->input_buffers[i].data, + mod->output_buffers[0].data); + + /* if one source is inactive, skip it */ + if (avail_frames == 0) + continue; + + sources_stream[j++] = mod->input_buffers[i].data; + } - md->mix_func(dev, mod->output_buffers[0].data, sources_stream, num_input_buffers, frames); + if (j) + md->mix_func(dev, mod->output_buffers[0].data, sources_stream, j, frames); mod->output_buffers[0].size = sink_bytes; /* update source buffer consumed bytes */ - for (i = 0; i < num_input_buffers; i++) + for (i = 0; i < j; i++) mod->input_buffers[i].consumed = source_bytes; return 0; diff --git a/src/include/sof/audio/mixer.h b/src/include/sof/audio/mixer.h index edbc19b5f04d..8c4aeab7f189 100644 --- a/src/include/sof/audio/mixer.h +++ b/src/include/sof/audio/mixer.h @@ -32,8 +32,6 @@ void sys_comp_module_mixer_interface_init(void); /* mixer component private data */ struct mixer_data { - bool sources_inactive; - void (*mix_func)(struct comp_dev *dev, struct audio_stream __sparse_cache *sink, const struct audio_stream __sparse_cache **sources, uint32_t count, uint32_t frames); From 6542a7a76de9946c6d0868f30196ac71cce1d679 Mon Sep 17 00:00:00 2001 From: Paul Olaru Date: Fri, 31 Mar 2023 13:22:26 +0300 Subject: [PATCH 2/2] audio: module_adapter: Ensure prepare always runs on modules Some components like mixer have special handling of the prepare function whenever they are already active. Ensure said handling has a chance to run. With the old code, that code doesn't run, mixer doesn't have a chance to return PPL_STATUS_PATH_STOP and the DAI then receives a second prepare request, which is invalid given that it's already active. Signed-off-by: Paul Olaru --- src/audio/module_adapter/module_adapter.c | 42 +++++++++++------------ 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/audio/module_adapter/module_adapter.c b/src/audio/module_adapter/module_adapter.c index 9d775a6459cb..52b27e014cd1 100644 --- a/src/audio/module_adapter/module_adapter.c +++ b/src/audio/module_adapter/module_adapter.c @@ -167,21 +167,16 @@ int module_adapter_prepare(struct comp_dev *dev) comp_dbg(dev, "module_adapter_prepare() start"); - /* - * check if the component is already active. This could happen in the case of mixer when - * one of the sources is already active - */ - if (dev->state == COMP_STATE_ACTIVE) - return 0; + /* Prepare module */ + ret = module_prepare(mod); + if (ret) { + if (ret == PPL_STATUS_PATH_STOP) + return ret; - /* Are we already prepared? */ - ret = comp_set_state(dev, COMP_TRIGGER_PREPARE); - if (ret < 0) - return ret; + comp_err(dev, "module_adapter_prepare() error %x: module prepare failed", + ret); - if (ret == COMP_STATUS_STATE_ALREADY_SET) { - comp_warn(dev, "module_adapter_prepare(): module has already been prepared"); - return PPL_STATUS_PATH_STOP; + return -EIO; } /* Get period_bytes first on prepare(). At this point it is guaranteed that the stream @@ -195,16 +190,21 @@ int module_adapter_prepare(struct comp_dev *dev) buffer_release(sink_c); - /* Prepare module */ - ret = module_prepare(mod); - if (ret) { - if (ret == PPL_STATUS_PATH_STOP) - return ret; + /* + * check if the component is already active. This could happen in the case of mixer when + * one of the sources is already active + */ + if (dev->state == COMP_STATE_ACTIVE) + return PPL_STATUS_PATH_STOP; - comp_err(dev, "module_adapter_prepare() error %x: module prepare failed", - ret); + /* Are we already prepared? */ + ret = comp_set_state(dev, COMP_TRIGGER_PREPARE); + if (ret < 0) + return ret; - return -EIO; + if (ret == COMP_STATUS_STATE_ALREADY_SET) { + comp_warn(dev, "module_adapter_prepare(): module has already been prepared"); + return PPL_STATUS_PATH_STOP; } mod->deep_buff_bytes = 0;