module: multiband_drc: rework module to use sink/source api#10813
module: multiband_drc: rework module to use sink/source api#10813softwarecki wants to merge 1 commit into
Conversation
softwarecki
requested review from
abonislawski,
Copilot,
marcinszkudlinski,
serhiy-katsyuba-intel,
tmleman and
wjablon1
softwarecki
requested review from
a team,
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull request overview
This PR refactors the multiband_drc module to use the sink/source API exclusively, aligning it with the ongoing transition toward Pipeline 2.0.
Changes:
- Migrates Multiband DRC module processing from
audio_stream/buffer-position updates tosof_source/sof_sinkacquire+commit semantics. - Updates Multiband DRC processing function signatures to return status (
int) and propagate errors. - Adds small helper utilities for computing “samples until wrap” in circular buffers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/include/module/audio/audio_stream.h |
Adds circular-buffer wrap helper inlines used by sink/source-based sample processing. |
src/audio/multiband_drc/multiband_drc.h |
Updates processing function typedefs/prototypes to use sink/source API and return int. |
src/audio/multiband_drc/multiband_drc.c |
Switches module interface to .process and routes processing through sink/source APIs. |
src/audio/multiband_drc/multiband_drc_generic.c |
Reworks passthrough + per-format processing to acquire/release source data and sink buffers via sink/source APIs. |
| 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, | ||
| int num_output_buffers) | ||
| struct sof_source **sources, int num_of_sources, | ||
| struct sof_sink **sinks, int num_of_sinks) | ||
| { | ||
| struct multiband_drc_comp_data *cd = module_get_private_data(mod); | ||
| struct comp_dev *dev = mod->dev; | ||
| struct audio_stream *source = input_buffers[0].data; | ||
| struct audio_stream *sink = output_buffers[0].data; | ||
| int frames = input_buffers[0].size; | ||
| int frames = source_get_data_frames_available(sources[0]); | ||
| int ret; |
| /** | ||
| * @brief Calculates numbers of s16 samples to buffer wrap. | ||
| * @param ptr Read or write pointer of circular buffer. | ||
| * @param buf_start Start address of circular buffer. | ||
| * @return Number of samples to buffer wrap. | ||
| */ | ||
| static inline int cir_buf_samples_to_wrap_s16(const int16_t *ptr, const int16_t *buf_start, | ||
| int buf_samples) | ||
| { |
There was a problem hiding this comment.
Please add, these will be used a lot
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
LGTM @softwarecki , but can you resolve the copilot comments. Thanks !
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Looks good. Can you document the one missing argument copilot picked up. No other issues I can see.
| nbuf = cir_buf_samples_to_wrap_s32(x, buf_x_start, buf_x_samples); | ||
| npcm = MIN(samples, nbuf); | ||
| nbuf = audio_stream_samples_without_wrap_s24(sink, y); | ||
| nbuf = cir_buf_samples_to_wrap_s32(y, buf_y_start, buf_y_samples); |
There was a problem hiding this comment.
@singalsu FYI, I think this is pattern we need for multiple modules
| /** | ||
| * @brief Calculates numbers of s16 samples to buffer wrap. | ||
| * @param ptr Read or write pointer of circular buffer. | ||
| * @param buf_start Start address of circular buffer. | ||
| * @return Number of samples to buffer wrap. | ||
| */ | ||
| static inline int cir_buf_samples_to_wrap_s16(const int16_t *ptr, const int16_t *buf_start, | ||
| int buf_samples) | ||
| { |
There was a problem hiding this comment.
Please add, these will be used a lot
|
@softwarecki ping - others in this series being merged now btw. |
piotrhoppeintel
force-pushed
the
p20-multiband
branch
4 times, most recently
from
defef3c to
39e9e35
Compare
| #include <stdint.h> | ||
| #include <stdbool.h> | ||
| #ifndef CONFIG_CORE_COUNT | ||
| #define assert(x) { } |
There was a problem hiding this comment.
this is a bit strange. Is assert() not defined for non-Intel builds or for tests?
| * @param buf_samples Total size of circular buffer in samples. | ||
| * @return Number of samples to buffer wrap. | ||
| */ | ||
| static inline size_t cir_buf_samples_to_wrap_s16(const int16_t *ptr, const int16_t *buf_start, |
There was a problem hiding this comment.
aren't other similar inlines defined in src/include/sof/audio/audio_stream.h - should these ones go there too?
There was a problem hiding this comment.
This file was introduced to separate a subset of the sof headers that are intended to be used by loadable modules. Because of that, any new functions that are meant to be accessible from loadable modules were added here. Ultimately, once this refactoring process is completed, the functions from the file you mentioned should be moved into this one.
Rework the multiband drc module to only use the sink/source api to prepare sof for the full transition to pipeline 2.0. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
piotrhoppeintel
force-pushed
the
p20-multiband
branch
from
39e9e35 to
217506f
Compare
Rework the multiband drc module to only use the sink/source api to prepare sof for the full transition to pipeline 2.0.