From 3c26552f6e03c65a6814d74f53576d35a0b321c3 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Sat, 6 Jan 2024 14:19:45 -0800 Subject: [PATCH 1/4] audio_stream: refactor: Move init_alignment_constants to C file audio_stream_init_alignment_constants() isn't a particularly small function, isn't used in performance-sensitive contexts, and doesn't really belong in a header. Move to audio_stream.c for hygiene, and because it's about to be modified. Also move the depended-on function audio_stream_frame_align_get(), and (as it has no consumers outside of audio_stream) remove its declaration from the header. Signed-off-by: Andy Ross --- src/audio/audio_stream.c | 26 +++++++++++++++++++++ src/include/sof/audio/audio_stream.h | 35 +++------------------------- 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/audio/audio_stream.c b/src/audio/audio_stream.c index 2a1a7a05e64a..5d23daaf7d6c 100644 --- a/src/audio/audio_stream.c +++ b/src/audio/audio_stream.c @@ -67,6 +67,32 @@ static int audio_stream_get_data(struct sof_source *source, size_t req_size, return 0; } +static uint32_t audio_stream_frame_align_get(const uint32_t byte_align, + const uint32_t frame_align_req, + uint32_t frame_size) +{ + /* Figure out how many frames are needed to meet the byte_align alignment requirements */ + uint32_t frame_num = byte_align / gcd(byte_align, frame_size); + + /** return the lcm of frame_num and frame_align_req*/ + return frame_align_req * frame_num / gcd(frame_num, frame_align_req); +} + + +void audio_stream_init_alignment_constants(const uint32_t byte_align, + const uint32_t frame_align_req, + struct audio_stream *stream) +{ + uint32_t process_size; + uint32_t frame_size = audio_stream_frame_bytes(stream); + + stream->runtime_stream_params.align_frame_cnt = + audio_stream_frame_align_get(byte_align, frame_align_req, frame_size); + process_size = stream->runtime_stream_params.align_frame_cnt * frame_size; + stream->runtime_stream_params.align_shift_idx = + (is_power_of_2(process_size) ? 31 : 32) - clz(process_size); +} + static int audio_stream_release_data(struct sof_source *source, size_t free_size) { struct audio_stream *audio_stream = container_of(source, struct audio_stream, source_api); diff --git a/src/include/sof/audio/audio_stream.h b/src/include/sof/audio/audio_stream.h index cb2bcd4967db..6e7c89b2fe08 100644 --- a/src/include/sof/audio/audio_stream.h +++ b/src/include/sof/audio/audio_stream.h @@ -331,25 +331,6 @@ static inline uint32_t audio_stream_sample_bytes(const struct audio_stream *buf) return get_sample_bytes(buf->runtime_stream_params.frame_fmt); } -/** - * Return the frames that meet the align requirement of both byte_align and - * frame_align_req. - * @param byte_align Processing byte alignment requirement. - * @param frame_align_req Processing frames alignment requirement. - * @param frame_size Size of the frame in bytes. - * @return frame number. - */ -static inline uint32_t audio_stream_frame_align_get(const uint32_t byte_align, - const uint32_t frame_align_req, - uint32_t frame_size) -{ - /* Figure out how many frames are needed to meet the byte_align alignment requirements */ - uint32_t frame_num = byte_align / gcd(byte_align, frame_size); - - /** return the lcm of frame_num and frame_align_req*/ - return frame_align_req * frame_num / gcd(frame_num, frame_align_req); -} - /** * Set align_shift_idx and align_frame_cnt of stream according to byte_align and * frame_align_req alignment requirement. Once the channel number,frame size @@ -362,19 +343,9 @@ static inline uint32_t audio_stream_frame_align_get(const uint32_t byte_align, * @param frame_align_req Processing frames alignment requirement. * @param stream Sink or source stream structure which to be set. */ -static inline void audio_stream_init_alignment_constants(const uint32_t byte_align, - const uint32_t frame_align_req, - struct audio_stream *stream) -{ - uint32_t process_size; - uint32_t frame_size = audio_stream_frame_bytes(stream); - - stream->runtime_stream_params.align_frame_cnt = - audio_stream_frame_align_get(byte_align, frame_align_req, frame_size); - process_size = stream->runtime_stream_params.align_frame_cnt * frame_size; - stream->runtime_stream_params.align_shift_idx = - (is_power_of_2(process_size) ? 31 : 32) - clz(process_size); -} +void audio_stream_init_alignment_constants(const uint32_t byte_align, + const uint32_t frame_align_req, + struct audio_stream *stream); /** * Applies parameters to the buffer. From 99e3c64142fa33fa5410560dadb5823082fbcb1f Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Fri, 22 Dec 2023 17:11:30 -0800 Subject: [PATCH 2/4] audio_stream: Persist requirements and lazy-recalculate alignment As specified, this function was a bit of a booby trap: it has to be called exactly once, after all other setup that modifies frame size has been done. If it is called in the wrong order, or not at all, any consumers of the frame alignment API on this stream will see garbage data in the computed fields. That's way too much complexity for the component code that needs to use this tool, especially as the impact is not in the component itself (e.g. it's a downstream copier widget with SIMD code that fails). Instead, preserve the two requirements set by this function in the audio_stream struct and recompute them any time any of the upstream values change. That allows this to be safely used from any context. There's a mild complexity with audio_sink/source layer, which is written to directly modify the format (it keeps a pointer into audio_stream of its own) without going through the audio_stream API itself. There, the framework gives us "on_audio_format_set()" callbacks on source and sink, so implement it there. This is a partial fix for Issue #8639 Signed-off-by: Andy Ross --- src/audio/audio_stream.c | 35 +++++++++++++++++++++++++--- src/include/sof/audio/audio_stream.h | 12 ++++++---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/audio/audio_stream.c b/src/audio/audio_stream.c index 5d23daaf7d6c..8e6528c8cd8d 100644 --- a/src/audio/audio_stream.c +++ b/src/audio/audio_stream.c @@ -79,10 +79,10 @@ static uint32_t audio_stream_frame_align_get(const uint32_t byte_align, } -void audio_stream_init_alignment_constants(const uint32_t byte_align, - const uint32_t frame_align_req, - struct audio_stream *stream) +void audio_stream_recalc_align(struct audio_stream *stream) { + const uint32_t byte_align = stream->byte_align_req; + const uint32_t frame_align_req = stream->frame_align_req; uint32_t process_size; uint32_t frame_size = audio_stream_frame_bytes(stream); @@ -93,6 +93,16 @@ void audio_stream_init_alignment_constants(const uint32_t byte_align, (is_power_of_2(process_size) ? 31 : 32) - clz(process_size); } +void audio_stream_init_alignment_constants(const uint32_t byte_align, + const uint32_t frame_align_req, + struct audio_stream *stream) +{ + stream->byte_align_req = byte_align; + stream->frame_align_req = frame_align_req; + audio_stream_recalc_align(stream); +} + + static int audio_stream_release_data(struct sof_source *source, size_t free_size) { struct audio_stream *audio_stream = container_of(source, struct audio_stream, source_api); @@ -145,11 +155,28 @@ static int audio_stream_sink_set_alignment_constants(struct sof_sink *sink, return 0; } +static int source_format_set(struct sof_source *source) +{ + struct audio_stream *s = container_of(source, struct audio_stream, source_api); + + audio_stream_recalc_align(s); + return 0; +} + +static int sink_format_set(struct sof_sink *sink) +{ + struct audio_stream *s = container_of(sink, struct audio_stream, sink_api); + + audio_stream_recalc_align(s); + return 0; +} + static const struct source_ops audio_stream_source_ops = { .get_data_available = audio_stream_get_data_available, .get_data = audio_stream_get_data, .release_data = audio_stream_release_data, .audio_set_ipc_params = audio_stream_set_ipc_params_source, + .on_audio_format_set = source_format_set, .set_alignment_constants = audio_stream_source_set_alignment_constants }; @@ -158,6 +185,7 @@ static const struct sink_ops audio_stream_sink_ops = { .get_buffer = audio_stream_get_buffer, .commit_buffer = audio_stream_commit_buffer, .audio_set_ipc_params = audio_stream_set_ipc_params_sink, + .on_audio_format_set = sink_format_set, .set_alignment_constants = audio_stream_sink_set_alignment_constants }; @@ -167,6 +195,7 @@ void audio_stream_init(struct audio_stream *audio_stream, void *buff_addr, uint3 audio_stream->addr = buff_addr; audio_stream->end_addr = (char *)audio_stream->addr + size; + audio_stream_init_alignment_constants(1, 1, audio_stream); source_init(audio_stream_get_source(audio_stream), &audio_stream_source_ops, &audio_stream->runtime_stream_params); sink_init(audio_stream_get_sink(audio_stream), &audio_stream_sink_ops, diff --git a/src/include/sof/audio/audio_stream.h b/src/include/sof/audio/audio_stream.h index 6e7c89b2fe08..1a0181c5d659 100644 --- a/src/include/sof/audio/audio_stream.h +++ b/src/include/sof/audio/audio_stream.h @@ -61,11 +61,15 @@ struct audio_stream { void *r_ptr; /**< Buffer read position */ void *addr; /**< Buffer base address */ void *end_addr; /**< Buffer end address */ + uint8_t byte_align_req; + uint8_t frame_align_req; /* runtime stream params */ struct sof_audio_stream_params runtime_stream_params; }; +void audio_stream_recalc_align(struct audio_stream *stream); + static inline void *audio_stream_get_rptr(const struct audio_stream *buf) { return buf->r_ptr; @@ -175,6 +179,7 @@ static inline void audio_stream_set_frm_fmt(struct audio_stream *buf, enum sof_ipc_frame val) { buf->runtime_stream_params.frame_fmt = val; + audio_stream_recalc_align(buf); } static inline void audio_stream_set_valid_fmt(struct audio_stream *buf, @@ -191,6 +196,7 @@ static inline void audio_stream_set_rate(struct audio_stream *buf, uint32_t val) static inline void audio_stream_set_channels(struct audio_stream *buf, uint16_t val) { buf->runtime_stream_params.channels = val; + audio_stream_recalc_align(buf); } static inline void audio_stream_set_underrun(struct audio_stream *buf, @@ -363,11 +369,7 @@ static inline int audio_stream_set_params(struct audio_stream *buffer, buffer->runtime_stream_params.rate = params->rate; buffer->runtime_stream_params.channels = params->channels; - /* set the default alignment info. - * set byte_align as 1 means no alignment limit on byte. - * set frame_align as 1 means no alignment limit on frame. - */ - audio_stream_init_alignment_constants(1, 1, buffer); + audio_stream_recalc_align(buffer); return 0; } From ea30d37d523c5ec1932f06590a4e972680c3f6c1 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Fri, 22 Dec 2023 17:17:00 -0800 Subject: [PATCH 3/4] various components: Remove default/stub init_alignment_constants usage Traditionally audio_stream has failed to initialize its computed alignment fields, forcing components to do this themselves even when they don't actually have special alignment requirements. Remove all the code that was merely setting default values, leaving only a handful of spots with specialr equirements (e.g. eq/area need to treat pairs of samples, a few others have HiFi-optimized variants which need SIMD alignment). Signed-off-by: Andy Ross --- src/audio/aria/aria.c | 4 +--- src/audio/asrc/asrc.c | 3 --- src/audio/crossover/crossover.c | 5 +---- src/audio/dcblock/dcblock.c | 13 ------------ src/audio/drc/drc.c | 9 -------- .../google/google_rtc_audio_processing.c | 2 -- src/audio/kpb.c | 4 ---- src/audio/mfcc/mfcc.c | 12 ----------- src/audio/mixer/mixer.c | 11 +--------- src/audio/multiband_drc/multiband_drc.c | 13 +----------- src/audio/mux/mux.c | 21 ------------------- src/audio/mux/mux_ipc4.c | 6 ------ src/audio/volume/volume.c | 12 +---------- 13 files changed, 5 insertions(+), 110 deletions(-) diff --git a/src/audio/aria/aria.c b/src/audio/aria/aria.c index d338ae22743e..cd9046e61dea 100644 --- a/src/audio/aria/aria.c +++ b/src/audio/aria/aria.c @@ -162,9 +162,7 @@ static void aria_set_stream_params(struct comp_buffer *buffer, const struct ipc4_audio_format *audio_fmt = &mod->priv.cfg.base_cfg.audio_fmt; ipc4_update_buffer_format(buffer, audio_fmt); -#ifdef ARIA_GENERIC - audio_stream_init_alignment_constants(1, 1, &buffer->stream); -#else +#ifndef ARIA_GENERIC audio_stream_init_alignment_constants(8, 1, &buffer->stream); #endif } diff --git a/src/audio/asrc/asrc.c b/src/audio/asrc/asrc.c index c89785fcb790..8ce6c711dfc9 100644 --- a/src/audio/asrc/asrc.c +++ b/src/audio/asrc/asrc.c @@ -550,9 +550,6 @@ static int asrc_prepare(struct processing_module *mod, sinkb = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - audio_stream_init_alignment_constants(1, 1, &sourceb->stream); - audio_stream_init_alignment_constants(1, 1, &sinkb->stream); - /* get source data format and period bytes */ cd->source_format = audio_stream_get_frm_fmt(&sourceb->stream); source_period_bytes = audio_stream_period_bytes(&sourceb->stream, diff --git a/src/audio/crossover/crossover.c b/src/audio/crossover/crossover.c index 4bb0be6f239f..69135368671a 100644 --- a/src/audio/crossover/crossover.c +++ b/src/audio/crossover/crossover.c @@ -546,14 +546,11 @@ static int crossover_prepare(struct processing_module *mod, /* Get source data format */ cd->source_format = audio_stream_get_frm_fmt(&source->stream); channels = audio_stream_get_channels(&source->stream); - audio_stream_init_alignment_constants(1, 1, &source->stream); /* Validate frame format and buffer size of sinks */ list_for_item(sink_list, &dev->bsink_list) { sink = container_of(sink_list, struct comp_buffer, source_list); - if (cd->source_format == audio_stream_get_frm_fmt(&sink->stream)) { - audio_stream_init_alignment_constants(1, 1, &sink->stream); - } else { + if (cd->source_format != audio_stream_get_frm_fmt(&sink->stream)) { comp_err(dev, "crossover_prepare(): Source fmt %d and sink fmt %d are different.", cd->source_format, audio_stream_get_frm_fmt(&sink->stream)); ret = -EINVAL; diff --git a/src/audio/dcblock/dcblock.c b/src/audio/dcblock/dcblock.c index a7ef67309c6a..8cca278d285d 100644 --- a/src/audio/dcblock/dcblock.c +++ b/src/audio/dcblock/dcblock.c @@ -183,17 +183,6 @@ static int dcblock_process(struct processing_module *mod, return 0; } -/* init and calculate the aligned setting for available frames and free frames retrieve*/ -static inline void dcblock_set_frame_alignment(struct audio_stream *source, - struct audio_stream *sink) -{ - const uint32_t byte_align = 1; - const uint32_t frame_align_req = 1; - - audio_stream_init_alignment_constants(byte_align, frame_align_req, source); - audio_stream_init_alignment_constants(byte_align, frame_align_req, sink); -} - /** * \brief Prepares DC Blocking Filter component for processing. * \param[in,out] dev DC Blocking Filter base component device. @@ -221,8 +210,6 @@ static int dcblock_prepare(struct processing_module *mod, /* get sink data format and period bytes */ cd->sink_format = audio_stream_get_frm_fmt(&sinkb->stream); - dcblock_set_frame_alignment(&sourceb->stream, &sinkb->stream); - dcblock_init_state(cd); cd->dcblock_func = dcblock_find_func(cd->source_format); if (!cd->dcblock_func) { diff --git a/src/audio/drc/drc.c b/src/audio/drc/drc.c index 9eff93d79019..7080fdea55f1 100644 --- a/src/audio/drc/drc.c +++ b/src/audio/drc/drc.c @@ -260,14 +260,6 @@ static int drc_get_config(struct processing_module *mod, return comp_data_blob_get_cmd(cd->model_handler, cdata, fragment_size); } -static void drc_set_alignment(struct audio_stream *source, - struct audio_stream *sink) -{ - /* Currently no optimizations those would use wider loads and stores */ - audio_stream_init_alignment_constants(1, 1, source); - audio_stream_init_alignment_constants(1, 1, sink); -} - static int drc_process(struct processing_module *mod, struct input_stream_buffer *input_buffers, int num_input_buffers, @@ -344,7 +336,6 @@ static int drc_prepare(struct processing_module *mod, /* DRC component will only ever have 1 source and 1 sink buffer */ sourceb = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); sinkb = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - drc_set_alignment(&sourceb->stream, &sinkb->stream); /* get source data format */ cd->source_format = audio_stream_get_frm_fmt(&sourceb->stream); diff --git a/src/audio/google/google_rtc_audio_processing.c b/src/audio/google/google_rtc_audio_processing.c index 3834278ba42e..6c59c60aa632 100644 --- a/src/audio/google/google_rtc_audio_processing.c +++ b/src/audio/google/google_rtc_audio_processing.c @@ -576,7 +576,6 @@ static int google_rtc_audio_processing_prepare(struct processing_module *mod, microphone_stream_channels); } - audio_stream_init_alignment_constants(1, 1, &source->stream); i++; } @@ -592,7 +591,6 @@ static int google_rtc_audio_processing_prepare(struct processing_module *mod, return -EINVAL; } - audio_stream_init_alignment_constants(1, 1, &output->stream); frame_fmt = audio_stream_get_frm_fmt(&output->stream); rate = audio_stream_get_rate(&output->stream); output_stream_channels = audio_stream_get_channels(&output->stream); diff --git a/src/audio/kpb.c b/src/audio/kpb.c index 43f75519850e..6d0507a033d1 100644 --- a/src/audio/kpb.c +++ b/src/audio/kpb.c @@ -893,16 +893,12 @@ static int kpb_prepare(struct comp_dev *dev) */ if (kpb->ipc4_cfg.base_cfg.ibs != kpb->ipc4_cfg.base_cfg.obs) { struct list_item *sink_list; - const uint32_t byte_align = 1; - const uint32_t frame_align_req = 1; uint32_t sink_id; list_for_item(sink_list, &dev->bsink_list) { struct comp_buffer *sink = container_of(sink_list, struct comp_buffer, source_list); - audio_stream_init_alignment_constants(byte_align, frame_align_req, - &sink->stream); sink_id = buf_get_id(sink); if (sink_id == 0) diff --git a/src/audio/mfcc/mfcc.c b/src/audio/mfcc/mfcc.c index 109c3636a4a1..9b88f1bc238a 100644 --- a/src/audio/mfcc/mfcc.c +++ b/src/audio/mfcc/mfcc.c @@ -177,15 +177,6 @@ static int mfcc_process(struct processing_module *mod, return 0; } -static void mfcc_set_alignment(struct audio_stream *source, struct audio_stream *sink) -{ - const uint32_t byte_align = 1; - const uint32_t frame_align_req = 1; - - audio_stream_init_alignment_constants(byte_align, frame_align_req, source); - audio_stream_init_alignment_constants(byte_align, frame_align_req, sink); -} - static int mfcc_prepare(struct processing_module *mod, struct sof_source **sources, int num_of_sources, struct sof_sink **sinks, int num_of_sinks) @@ -208,9 +199,6 @@ static int mfcc_prepare(struct processing_module *mod, /* get source data format */ source_format = audio_stream_get_frm_fmt(&sourceb->stream); - /* set align requirements */ - mfcc_set_alignment(&sourceb->stream, &sinkb->stream); - /* get sink data format and period bytes */ sink_format = audio_stream_get_frm_fmt(&sinkb->stream); sink_period_bytes = audio_stream_period_bytes(&sinkb->stream, dev->frames); diff --git a/src/audio/mixer/mixer.c b/src/audio/mixer/mixer.c index 1fe9c381c142..38be94676c29 100644 --- a/src/audio/mixer/mixer.c +++ b/src/audio/mixer/mixer.c @@ -205,17 +205,8 @@ static inline void mixer_set_frame_alignment(struct audio_stream *source) /*There is no limit for frame number, so set it as 1*/ const uint32_t frame_align_req = 1; -#else - - /* Since the generic version process signal sample by sample, so there is no - * limit for it, then set the byte_align and frame_align_req to be 1. - */ - const uint32_t byte_align = 1; - const uint32_t frame_align_req = 1; - -#endif - audio_stream_init_alignment_constants(byte_align, frame_align_req, source); +#endif } static int mixer_prepare(struct processing_module *mod, diff --git a/src/audio/multiband_drc/multiband_drc.c b/src/audio/multiband_drc/multiband_drc.c index 5454df29318a..e10b63d28d9a 100644 --- a/src/audio/multiband_drc/multiband_drc.c +++ b/src/audio/multiband_drc/multiband_drc.c @@ -310,14 +310,6 @@ static int multiband_drc_get_config(struct processing_module *mod, return multiband_drc_get_ipc_config(mod, cdata, fragment_size); } -static void multiband_drc_set_alignment(struct audio_stream *source, - struct audio_stream *sink) -{ - /* Currently no optimizations those would use wider loads and stores */ - audio_stream_init_alignment_constants(1, 1, source); - audio_stream_init_alignment_constants(1, 1, sink); -} - static int multiband_drc_process(struct processing_module *mod, struct input_stream_buffer *input_buffers, int num_input_buffers, struct output_stream_buffer *output_buffers, @@ -356,7 +348,7 @@ static int multiband_drc_prepare(struct processing_module *mod, { struct multiband_drc_comp_data *cd = module_get_private_data(mod); struct comp_dev *dev = mod->dev; - struct comp_buffer *sourceb, *sinkb; + struct comp_buffer *sourceb; int channels; int rate; int ret = 0; @@ -369,9 +361,6 @@ static int multiband_drc_prepare(struct processing_module *mod, /* DRC component will only ever have 1 source and 1 sink buffer */ sourceb = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); - sinkb = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - - multiband_drc_set_alignment(&sourceb->stream, &sinkb->stream); /* get source data format */ cd->source_format = audio_stream_get_frm_fmt(&sourceb->stream); diff --git a/src/audio/mux/mux.c b/src/audio/mux/mux.c index b57588655941..15a00461a5f9 100644 --- a/src/audio/mux/mux.c +++ b/src/audio/mux/mux.c @@ -378,12 +378,8 @@ static int mux_prepare(struct processing_module *mod, { struct comp_dev *dev = mod->dev; struct comp_data *cd = module_get_private_data(mod); - struct list_item *blist; - struct comp_buffer *source; - struct comp_buffer *sink; struct sof_mux_config *config; size_t blob_size; - int state; int ret; comp_dbg(dev, "mux_prepare()"); @@ -410,23 +406,6 @@ static int mux_prepare(struct processing_module *mod, return -EINVAL; } - /* check each mux source state, set source align to 1 byte, 1 frame */ - list_for_item(blist, &dev->bsource_list) { - source = container_of(blist, struct comp_buffer, sink_list); - state = source->source->state; - audio_stream_init_alignment_constants(1, 1, &source->stream); - - /* only prepare downstream if we have no active sources */ - if (state == COMP_STATE_PAUSED || state == COMP_STATE_ACTIVE) - return PPL_STATUS_PATH_STOP; - } - - /* set sink align to 1 byte, 1 frame */ - list_for_item(blist, &dev->bsink_list) { - sink = container_of(blist, struct comp_buffer, source_list); - audio_stream_init_alignment_constants(1, 1, &sink->stream); - } - /* prepare downstream */ return 0; } diff --git a/src/audio/mux/mux_ipc4.c b/src/audio/mux/mux_ipc4.c index ada22b2731e6..bf6e591665bc 100644 --- a/src/audio/mux/mux_ipc4.c +++ b/src/audio/mux/mux_ipc4.c @@ -80,8 +80,6 @@ static void set_mux_params(struct processing_module *mod) struct comp_buffer *sink, *source; struct list_item *source_list; int j; - const uint32_t byte_align = 1; - const uint32_t frame_align_req = 1; params->direction = dev->direction; params->channels = cd->md.base_cfg.audio_fmt.channels_count; @@ -101,8 +99,6 @@ static void set_mux_params(struct processing_module *mod) /* update sink format */ if (!list_is_empty(&dev->bsink_list)) { sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - audio_stream_init_alignment_constants(byte_align, frame_align_req, - &sink->stream); if (!sink->hw_params_configured) { ipc4_update_buffer_format(sink, &cd->md.output_format); @@ -117,8 +113,6 @@ static void set_mux_params(struct processing_module *mod) list_for_item(source_list, &dev->bsource_list) { source = container_of(source_list, struct comp_buffer, sink_list); - audio_stream_init_alignment_constants(byte_align, frame_align_req, - &source->stream); j = buf_get_id(source); cd->config.streams[j].pipeline_id = source->pipeline_id; if (j == BASE_CFG_QUEUED_ID) diff --git a/src/audio/volume/volume.c b/src/audio/volume/volume.c index 002ceff9d852..694d778db6b0 100644 --- a/src/audio/volume/volume.c +++ b/src/audio/volume/volume.c @@ -623,7 +623,6 @@ static void volume_set_alignment(struct audio_stream *source, struct audio_stream *sink) { #if XCHAL_HAVE_HIFI3 || XCHAL_HAVE_HIFI4 - /* Both source and sink buffer in HiFi 3 or HiFi4 processing version, * xtensa intrinsics ask for 8-byte aligned. 5.1 format SSE audio * requires 16-byte aligned. @@ -633,18 +632,9 @@ static void volume_set_alignment(struct audio_stream *source, /*There is no limit for frame number, so both source and sink set it to be 1*/ const uint32_t frame_align_req = 1; -#else - - /* Since the generic version process signal sample by sample, so there is no - * limit for it, then set the byte_align and frame_align_req to be 1. - */ - const uint32_t byte_align = 1; - const uint32_t frame_align_req = 1; - -#endif - audio_stream_init_alignment_constants(byte_align, frame_align_req, source); audio_stream_init_alignment_constants(byte_align, frame_align_req, sink); +#endif } /** From 541fac3d6e13669cc60c073b52e38e1eb8d8e455 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Mon, 8 Jan 2024 07:32:06 -0800 Subject: [PATCH 4/4] audio_stream: Rename/redocument audio_stream_set_align After recent changes, the audio_stream_init_alignment_constants() routine isn't an "init" step anymore, it sets requirements and can be called at any time. Rename it to "audio_stream_set_align()" to better capture its behavior, and rework the documentation to make it clearer how it works. Signed-off-by: Andy Ross --- src/audio/aria/aria.c | 2 +- src/audio/audio_stream.c | 8 ++++---- src/audio/eq_fir/eq_fir.c | 4 ++-- src/audio/eq_iir/eq_iir.c | 4 ++-- src/audio/mixer/mixer.c | 2 +- src/audio/selector/selector.c | 4 ++-- src/audio/tdfb/tdfb.c | 4 ++-- src/audio/volume/volume.c | 4 ++-- src/include/sof/audio/audio_stream.h | 20 ++++++++++---------- 9 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/audio/aria/aria.c b/src/audio/aria/aria.c index cd9046e61dea..16d6eea1565d 100644 --- a/src/audio/aria/aria.c +++ b/src/audio/aria/aria.c @@ -163,7 +163,7 @@ static void aria_set_stream_params(struct comp_buffer *buffer, ipc4_update_buffer_format(buffer, audio_fmt); #ifndef ARIA_GENERIC - audio_stream_init_alignment_constants(8, 1, &buffer->stream); + audio_stream_set_align(8, 1, &buffer->stream); #endif } diff --git a/src/audio/audio_stream.c b/src/audio/audio_stream.c index 8e6528c8cd8d..d479bf80c788 100644 --- a/src/audio/audio_stream.c +++ b/src/audio/audio_stream.c @@ -93,7 +93,7 @@ void audio_stream_recalc_align(struct audio_stream *stream) (is_power_of_2(process_size) ? 31 : 32) - clz(process_size); } -void audio_stream_init_alignment_constants(const uint32_t byte_align, +void audio_stream_set_align(const uint32_t byte_align, const uint32_t frame_align_req, struct audio_stream *stream) { @@ -139,7 +139,7 @@ static int audio_stream_source_set_alignment_constants(struct sof_source *source { struct audio_stream *audio_stream = container_of(source, struct audio_stream, source_api); - audio_stream_init_alignment_constants(byte_align, frame_align_req, audio_stream); + audio_stream_set_align(byte_align, frame_align_req, audio_stream); return 0; } @@ -150,7 +150,7 @@ static int audio_stream_sink_set_alignment_constants(struct sof_sink *sink, { struct audio_stream *audio_stream = container_of(sink, struct audio_stream, sink_api); - audio_stream_init_alignment_constants(byte_align, frame_align_req, audio_stream); + audio_stream_set_align(byte_align, frame_align_req, audio_stream); return 0; } @@ -195,7 +195,7 @@ void audio_stream_init(struct audio_stream *audio_stream, void *buff_addr, uint3 audio_stream->addr = buff_addr; audio_stream->end_addr = (char *)audio_stream->addr + size; - audio_stream_init_alignment_constants(1, 1, audio_stream); + audio_stream_set_align(1, 1, audio_stream); source_init(audio_stream_get_source(audio_stream), &audio_stream_source_ops, &audio_stream->runtime_stream_params); sink_init(audio_stream_get_sink(audio_stream), &audio_stream_sink_ops, diff --git a/src/audio/eq_fir/eq_fir.c b/src/audio/eq_fir/eq_fir.c index 3068cd0971c2..b7402dba2207 100644 --- a/src/audio/eq_fir/eq_fir.c +++ b/src/audio/eq_fir/eq_fir.c @@ -402,8 +402,8 @@ static void eq_fir_set_alignment(struct audio_stream *source, const uint32_t byte_align = 1; const uint32_t frame_align_req = 2; /* Process multiples of 2 frames */ - audio_stream_init_alignment_constants(byte_align, frame_align_req, source); - audio_stream_init_alignment_constants(byte_align, frame_align_req, sink); + audio_stream_set_align(byte_align, frame_align_req, source); + audio_stream_set_align(byte_align, frame_align_req, sink); } static int eq_fir_prepare(struct processing_module *mod, diff --git a/src/audio/eq_iir/eq_iir.c b/src/audio/eq_iir/eq_iir.c index b5d539759091..3c3312d1557c 100644 --- a/src/audio/eq_iir/eq_iir.c +++ b/src/audio/eq_iir/eq_iir.c @@ -174,8 +174,8 @@ static void eq_iir_set_alignment(struct audio_stream *source, const uint32_t byte_align = 8; const uint32_t frame_align_req = 2; - audio_stream_init_alignment_constants(byte_align, frame_align_req, source); - audio_stream_init_alignment_constants(byte_align, frame_align_req, sink); + audio_stream_set_align(byte_align, frame_align_req, source); + audio_stream_set_align(byte_align, frame_align_req, sink); } static int eq_iir_prepare(struct processing_module *mod, diff --git a/src/audio/mixer/mixer.c b/src/audio/mixer/mixer.c index 38be94676c29..e90a75c838ff 100644 --- a/src/audio/mixer/mixer.c +++ b/src/audio/mixer/mixer.c @@ -205,7 +205,7 @@ static inline void mixer_set_frame_alignment(struct audio_stream *source) /*There is no limit for frame number, so set it as 1*/ const uint32_t frame_align_req = 1; - audio_stream_init_alignment_constants(byte_align, frame_align_req, source); + audio_stream_set_align(byte_align, frame_align_req, source); #endif } diff --git a/src/audio/selector/selector.c b/src/audio/selector/selector.c index 136fcac0232a..f2de27b2f3ef 100644 --- a/src/audio/selector/selector.c +++ b/src/audio/selector/selector.c @@ -847,8 +847,8 @@ static int selector_prepare(struct processing_module *mod, sourceb = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); sinkb = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - audio_stream_init_alignment_constants(4, 1, &sourceb->stream); - audio_stream_init_alignment_constants(4, 1, &sinkb->stream); + audio_stream_set_align(4, 1, &sourceb->stream); + audio_stream_set_align(4, 1, &sinkb->stream); /* get source data format and period bytes */ cd->source_format = audio_stream_get_frm_fmt(&sourceb->stream); diff --git a/src/audio/tdfb/tdfb.c b/src/audio/tdfb/tdfb.c index 7a7f751b1548..149e8dc7982b 100644 --- a/src/audio/tdfb/tdfb.c +++ b/src/audio/tdfb/tdfb.c @@ -518,8 +518,8 @@ static void tdfb_set_alignment(struct audio_stream *source, const uint32_t byte_align = 1; const uint32_t frame_align_req = 2; /* Process multiples of 2 frames */ - audio_stream_init_alignment_constants(byte_align, frame_align_req, source); - audio_stream_init_alignment_constants(byte_align, frame_align_req, sink); + audio_stream_set_align(byte_align, frame_align_req, source); + audio_stream_set_align(byte_align, frame_align_req, sink); } static int tdfb_prepare(struct processing_module *mod, diff --git a/src/audio/volume/volume.c b/src/audio/volume/volume.c index 694d778db6b0..b9f1d1ddbc5b 100644 --- a/src/audio/volume/volume.c +++ b/src/audio/volume/volume.c @@ -632,8 +632,8 @@ static void volume_set_alignment(struct audio_stream *source, /*There is no limit for frame number, so both source and sink set it to be 1*/ const uint32_t frame_align_req = 1; - audio_stream_init_alignment_constants(byte_align, frame_align_req, source); - audio_stream_init_alignment_constants(byte_align, frame_align_req, sink); + audio_stream_set_align(byte_align, frame_align_req, source); + audio_stream_set_align(byte_align, frame_align_req, sink); #endif } diff --git a/src/include/sof/audio/audio_stream.h b/src/include/sof/audio/audio_stream.h index 1a0181c5d659..cc143acd6532 100644 --- a/src/include/sof/audio/audio_stream.h +++ b/src/include/sof/audio/audio_stream.h @@ -338,20 +338,20 @@ static inline uint32_t audio_stream_sample_bytes(const struct audio_stream *buf) } /** - * Set align_shift_idx and align_frame_cnt of stream according to byte_align and - * frame_align_req alignment requirement. Once the channel number,frame size - * are determined，the align_frame_cnt and align_shift_idx are determined too. - * these two feature will be used in audio_stream_get_avail_frames_aligned - * to calculate the available frames. it should be called in component prepare - * or param functions only once before stream copy. if someone forgets to call - * this first, there would be unexampled error such as nothing is copied at all. + * @brief Set processing alignment requirements + * + * Sets the sample byte alignment and aligned frame count required for + * processing done on this stream. This function may be called at any + * time, the internal constants are recalculated if the frame/sample + * size changes. @see audio_stream_avail_frames_aligned(). + * * @param byte_align Processing byte alignment requirement. * @param frame_align_req Processing frames alignment requirement. * @param stream Sink or source stream structure which to be set. */ -void audio_stream_init_alignment_constants(const uint32_t byte_align, - const uint32_t frame_align_req, - struct audio_stream *stream); +void audio_stream_set_align(const uint32_t byte_align, + const uint32_t frame_align_req, + struct audio_stream *stream); /** * Applies parameters to the buffer.