module_adapter: build params first then verify it #8612
Conversation
We need to build params based on module base config first then we can verify it since the income params is built for the source of the this module. In comp_verify_params the params is applied to buffer so the sequence is vital. And also remove redundant stream_param set. Signed-off-by: Rander Wang <rander.wang@intel.com>
RanderWang
force-pushed
the
module_adapter_params
branch
from
7f6aaff to
974dc90
Compare
RanderWang
requested review from
kv2019i,
lgirdwood,
lyakh and
marcinszkudlinski
| int ret; | ||
| struct processing_module *mod = comp_get_drvdata(dev); | ||
|
|
||
| module_adapter_set_params(mod, params); |
There was a problem hiding this comment.
while at it you can also replace rzalloc() with rmalloc() in line 596 because the memory is completely overwritten, and also we can check the size and if it's the same, avoid re-allocation?
There was a problem hiding this comment.
good idea. I will submit another PR.
btian1
left a comment
btian1
left a comment
There was a problem hiding this comment.
" the this module" something wrong with this in commit message.
There was a problem hiding this comment.
@RanderWang The commit message is hard to read. My immediate reaction is that if the order is wrong, how has this worked before this commit?
I won't block as others seem to have understood this better, but in future commits, please expand (in the commit message) what problem the patch is solving. And if it's something obvious (like here, we verify before parameter object is ready), reviewers (and people later maintaining the code) would really appreciate an explanation how did this work despite the flaw.
|
@kv2019i agree with you, commit messages should explain WHY the change is needed and how in this specific case it worked before. I Acked this patch only because on IPC3 this patch is a NO-OP and assumed that IPC4-gang reviewed for that case. |
This is a general solution and all the build-in modules do it by themself. Loadable module will use this general solution. |
7 participants
We need to build params based on module base config first then we can verify it since the income params is built for the source of the this module . In comp_verify_params the params is applied to buffer so the sequence is vital. And also remove redundant stream_param set.