TDFB code reorganize #8457
TDFB code reorganize #8457
Conversation
src/audio/tdfb/tdfb.h
Outdated
| /* The driver assigns running numbers for control index. If there's single control of | ||
| * type switch, enum, binary they all have index 0. | ||
| */ | ||
| #define CTRL_INDEX_PROCESS 0 /* switch */ |
There was a problem hiding this comment.
These would be good to prefix with TDFB_.
There was a problem hiding this comment.
ok， let me use another patch to address, this patch focus on refine.
src/audio/tdfb/tdfb_ipc3.c
Outdated
| ipc_msg_send(cd->msg, cd->ctrl_data, false); | ||
| } | ||
|
|
||
| int tdfb_sub_init(struct processing_module *mod) |
There was a problem hiding this comment.
The new function name is not descriptive, it should describe better what is done here.
src/audio/tdfb/tdfb_ipc4.c
Outdated
| ipc_msg_send(msg, NULL, false); | ||
| } | ||
|
|
||
| int tdfb_sub_init(struct processing_module *mod) |
There was a problem hiding this comment.
Update this function name too. Perhaps tdfb_common_notification_init()?
There was a problem hiding this comment.
Cant use common in name as this is within an ipc4 file.
| ipc4_base_module_cfg_to_stream_params(&mod->priv.cfg.base_cfg, params); | ||
| component_set_nearest_period_frames(dev, params->rate); | ||
|
|
||
| sourceb = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); |
There was a problem hiding this comment.
I realized here's similar issue as there was with my work with rtc aec. This is similar as original, but it's not handling correctly a typical case where source has more channels than sink. Please add a reminder /* TODO: */ note for it. I can fix it later when I have suitable topology to use for test.
There was a problem hiding this comment.
The merged fix for it is here: #8452
src/audio/tdfb/tdfb_ipc4.c
Outdated
| tdfb_send_notification(cd->msg, cd->az_value_estimate); | ||
| } | ||
|
|
||
| int tdfb_get_config_common(struct processing_module *mod, |
There was a problem hiding this comment.
sorry, not sure why you call these functions *_common()? They differ between IPC3 and IPC4, I would use "common" for functions shared between IPC versions?
There was a problem hiding this comment.
ack, sounds like they are not common, otherwise it LGTM. Lets fix the ipc/common naming.
There was a problem hiding this comment.
both ipc3/ipc4 have this function implementation, my purpose is:
keep interface function defined in main source file, i.e, keep: tdfb_get_config & tdfb_set_config in tdfb.c.
Otherwise, I can move tdfb_get_config & tdfb_get_config to ipc3/ipc4 separately, this will make tdfb.c looks lack something
There was a problem hiding this comment.
the code separation is good, we're only proposing to change the function naming. "common" in this case suggests that those functions are common for IPC3 and IPC4. The code in tdfb.c is common for IPC3 and IPC4. But functions, implemented separately for IPC3 and IPC4 are IPC-version-specific. I'm not sure I can propose a good name suffix for those version-specific function parts, how about ..._private or ..._priv or ..._specific?
There was a problem hiding this comment.
no perfect name actually, let's keep for now?
There was a problem hiding this comment.
no perfect name actually, let's keep for now?
personally each of the proposed options seems better to me than "common," e.g. I'd go with "priv"
btian1
force-pushed
the
tdfb_refine
branch
2 times, most recently
from
3720fa5 to
91b935f
Compare
src/audio/tdfb/tdfb_ipc4.c
Outdated
| tdfb_send_notification(cd->msg, cd->az_value_estimate); | ||
| } | ||
|
|
||
| int tdfb_get_config_common(struct processing_module *mod, |
There was a problem hiding this comment.
Still a few not-common common functions. While at it, I propose a better alternative to "private". Especially if "private" is put in the middle of the function (like get_private_params()), this implies we are getting some private parameters (i..e attribute of params, and not attribution of the function itself).
btian1
force-pushed
the
tdfb_refine
branch
2 times, most recently
from
47484ce to
712443e
Compare
Move it from include path to tdfb module folder. Signed-off-by: Baofeng Tian <[email protected]>
Split out ipc3 and ipc4 specific code from tdfb module, also refined header files for new source file. Signed-off-by: Baofeng Tian <[email protected]>
Add prefix SOF_TDFB to macros to avoid repeat definition. Signed-off-by: Baofeng Tian <[email protected]>
move it from include path to tdfb module folder.