Crossover refactor #8652
Crossover refactor #8652
Conversation
btian1
commented
Dec 20, 2023
btian1
commented
- move crossover and multiband DRC common part to math include path.
- split ipc3 and ipc4 specific code.
btian1
force-pushed
the
move_common_to_math
branch
from
c464fdd
to
b2fc3b8
Compare
btian1
requested review from
marc-hb,
a team,
lgirdwood,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
| @@ -204,7 +206,7 @@ set(dcblock_sources dcblock/dcblock.c dcblock/dcblock_generic.c dcblock/dcblock_ | |||
| set(crossover_sources crossover/crossover.c crossover/crossover_generic.c) | |||
| set(tdfb_sources tdfb/tdfb.c tdfb/tdfb_generic.c tdfb/tdfb_direction.c) | |||
| set(drc_sources drc/drc.c drc/drc_generic.c drc/drc_math_generic.c) | |||
| set(multiband_drc_sources multiband_drc/multiband_drc_generic.c crossover/crossover.c crossover/crossover_generic.c drc/drc.c drc/drc_generic.c drc/drc_math_generic.c multiband_drc/multiband_drc.c ) | |||
| set(multiband_drc_sources multiband_drc/multiband_drc_generic.c crossover/crossover.c drc/drc.c drc/drc_generic.c drc/drc_math_generic.c multiband_drc/multiband_drc.c ) | |||
There was a problem hiding this comment.
Not sure it fits scope of this PR but eventually the crossover/crossover.c content should move to maths library where it's enabled by kconfig if crossover or multiband-drc component is built. I think the current sources linking duplicates the crossover .text and consumes more RAM.
There was a problem hiding this comment.
Not all of crossover.c (e.g. prepare and other component ops) would be in maths library but only the common filter related functions those are really shared.
There was a problem hiding this comment.
I tried to figure out yesterday, however, the common part is prepare / params, not actually the processing functions, not easy to move filter related to math, you can check crossover_common.h
all listed are required by multiband_drc.
Regarding linking, I searched sof assembly, take crossover.c:
static void crossover_s32_default(struct comp_data *cd,
struct input_stream_buffer *bsource,
struct output_stream_buffer **bsinks,
int32_t num_sinks,
uint32_t frames)
as example, it only have one time results.
also for crossover_common part, all are embeded as inline, no more duplicated code found in assembly.
There was a problem hiding this comment.
now move it to include/module, seems more reasonable compared with math, since most of common part does not related with math.
There was a problem hiding this comment.
Not fan of math folder usage. dct.h and matrix.h fit nicely to math, but putting whole algorithms there. But @singalsu , I do note the iir/fir core headers are there already. so maybe this agreed upon and we should put all "reusable algorithm core" headers here. I won't block the way...
btian1
force-pushed
the
move_common_to_math
branch
from
72d12e0
to
4a33936
Compare
btian1
force-pushed
the
move_common_to_math
branch
from
4a33936
to
2c46dad
Compare
There was a problem hiding this comment.
One minor note w.r.t. the "user" header. Also, @johnylin76 @cujomalainey please check if this looks reasonable (starts to be ready to merge).
| @@ -10,9 +10,7 @@ | |||
|
|
|||
| #include <stdint.h> | |||
| #include <user/eq.h> | |||
|
|
|||
| /* Maximum Number of sinks allowed in config */ | |||
| #define SOF_CROSSOVER_MAX_STREAMS 4 | |||
There was a problem hiding this comment.
Hmm, this is a bit problematic. The "user/crossover.h" header should not have dependency to implementation parts, it should be independent description of the parameter interface (like user/eq.h).
There was a problem hiding this comment.
it was referenced by multiband DRC user header file with:
/* Maximum number of frequency band for Multiband DRC */
#define SOF_MULTIBAND_DRC_MAX_BANDS SOF_CROSSOVER_MAX_STREAMS
btian1
force-pushed
the
move_common_to_math
branch
from
2c46dad
to
7779866
Compare
…d DRC Multiband DRC requires crossover functionality to cover crossover specific process, previously, these information defined in crossover specific directory, now, create a new shared header file for common information shared both in crossover and multiband DRC. Signed-off-by: Baofeng Tian <baofeng.tian@intel.com>
This is a clean up, purpose is declutter headers, toml files, Readme.md etc per module basis, since today everything is scattered in current code base. Signed-off-by: Baofeng Tian <baofeng.tian@intel.com>
This is a clean up, purpose is declutter headers, toml files, Readme.md etc per module basis, since today everything is scattered in current code base. Signed-off-by: Baofeng Tian <baofeng.tian@intel.com>
This is a clean up, purpose is declutter headers, toml files, Readme.md etc per module basis, since today everything is scattered in current code base. Signed-off-by: Baofeng Tian <baofeng.tian@intel.com>
