Module api convert crossover, drc, multiband_drc, fir and iir modules #10289
Conversation
jsarha
requested review from
a team,
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1,
plbossart and
singalsu
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR converts several audio processing modules (crossover, DRC, multiband_DRC, FIR, and IIR) to use the module adapter API instead of direct memory allocation functions and component-specific interfaces.
Key changes include:
- Replacing direct memory allocation functions (
rzalloc,rfree,rballoc) with module-specific variants (mod_zalloc,mod_free,mod_balloc) - Converting component data blob handlers to module data blob handlers (
comp_data_blob_handler_new→mod_data_blob_handler_new) - Adding
struct processing_module *modparameters to functions that need access to module memory management
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/include/module/crossover/crossover_common.h |
Updates crossover function signatures to accept processing_module parameter for memory management |
src/audio/multiband_drc/multiband_drc.h |
Updates multiband DRC IIR reset function to use module memory management |
src/audio/multiband_drc/multiband_drc.c |
Converts multiband DRC implementation to use module adapter API for memory allocation and error handling |
src/audio/eq_iir/eq_iir_ipc4.c |
Removes unused alloc.h include |
src/audio/eq_iir/eq_iir_ipc3.c |
Removes unused alloc.h include |
src/audio/eq_iir/eq_iir_generic.c |
Updates IIR generic implementation to use module memory management |
src/audio/eq_iir/eq_iir.h |
Updates function signature to accept processing_module parameter |
src/audio/eq_iir/eq_iir.c |
Converts IIR implementation to use module adapter API |
src/audio/eq_fir/eq_fir.c |
Converts FIR implementation to use module adapter API |
src/audio/drc/drc_algorithm.h |
Updates DRC algorithm function signatures for module adapter compatibility |
src/audio/drc/drc.c |
Converts DRC implementation to use module adapter API |
src/audio/crossover/crossover.c |
Converts crossover implementation to use module adapter API |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
lyakh
left a comment
lyakh
left a comment
There was a problem hiding this comment.
haven't reviewed completely, but let's check the whole PR set for when manual memory freeing is needed
| return 0; | ||
|
|
||
| cd_fail: | ||
| comp_data_blob_handler_free(cd->model_handler); |
There was a problem hiding this comment.
same here - `.init()? might need to clean up manually
There was a problem hiding this comment.
There was a problem hiding this comment.
same here - `.init()? might need to clean up manually
@jsarha oh... Well, yes, that fixes it... I was initially a bit uncomfortable with that because of the "if a function fails, it should release all the resources it has managed to allocate before failing." But that's one of the modes how this module-managed allocations work: if a later stage calls mod_alloc() which succeeds, but then another call fails, the function won't call mod_free() explicitly and will rely on the automatic clean up. So maybe we can do it here too...
jsarha
force-pushed
the
module_api_convert_crossover_drc_mdrc_fir_iir
branch
from
7c3209e to
69ea54c
Compare
src/audio/crossover/crossover.c
Outdated
| * across all channels | ||
| */ | ||
| static void crossover_reset_state(struct comp_data *cd) | ||
| static void crossover_reset_state(struct processing_module *mod, |
There was a problem hiding this comment.
Could for simplicity just pass mod and get cd from it.
src/audio/eq_fir/eq_fir.c
Outdated
| } | ||
|
|
||
| static int eq_fir_setup(struct comp_dev *dev, struct comp_data *cd, int nch) | ||
| static int eq_fir_setup(struct processing_module *mod, struct comp_data *cd, int nch) |
There was a problem hiding this comment.
Also here, could get cd from mod for simpler function call.
jsarha
force-pushed
the
module_api_convert_crossover_drc_mdrc_fir_iir
branch
from
69ea54c to
d4732c9
Compare
| return 0; | ||
|
|
||
| cd_fail: | ||
| comp_data_blob_handler_free(cd->model_handler); |
There was a problem hiding this comment.
same here - `.init()? might need to clean up manually
@jsarha oh... Well, yes, that fixes it... I was initially a bit uncomfortable with that because of the "if a function fails, it should release all the resources it has managed to allocate before failing." But that's one of the modes how this module-managed allocations work: if a later stage calls mod_alloc() which succeeds, but then another call fails, the function won't call mod_free() explicitly and will rely on the automatic clean up. So maybe we can do it here too...
Convert crossover, drc, and multiband_drc to use module API. These modules are bundled to one commit as they have inter dependencies, and keeping the changes separate would cause build breakage between the commits. Allocate all memory, blob handlers, and fast_get() buffers through module API mod_alloc() and friends. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Allocate all memory, blob handlers, and fast_get() buffers through module API mod_alloc() and friends. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Allocate all memory, blob handlers, and fast_get() buffers through module API mod_alloc() and friends. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
jsarha
force-pushed
the
module_api_convert_crossover_drc_mdrc_fir_iir
branch
from
d4732c9 to
4e36541
Compare
|
These new versions of module conversions use module API to allocate the resources, but the code still has all the resource freeing code in place. |
6 participants
Crossover, drc, and multiband_drc modules are bundled to one commit as they have inter dependencies, and keeping the changes separate would cause build breakage between the commits.