-
Notifications
You must be signed in to change notification settings - Fork 349
cmocka: add eq_fir unit test #6308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@singalsu @andrula-song this PR uses pass-through coef. Please help to figure out a non-pass-through coef case. Thanks! |
2f7eb04 to
09a7f76
Compare
Thanks! I will add the Matlab scripts to generate the test vectors for pass-through and some other response shape, e.g. the Loudness EQ in examples for FIR. The the process to create the numbers blobs will be then documented in the scripts. |
|
@RanderWang we have the infra for unifying the unit tests for modules using the module adapter now. Please have a look at volume for example. Could you please use that as well? |
Sure, I will. The most important thing for me is to study FIR algorithm and how to make it work. We need to make a usable FIR test first then I can change interface or framework. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be the same input as IIR is using ../../../include/cmocka_chirp_2ch.h ? Not duplicate it here but instead use the same header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure. And what's the ref output ? If we uses pass-through coef, this is not a problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm doing a script that exports all needed ref and coef blob files. Then it will be easy to add more tests and change them.
I think the cause of fail with the current error criteria is quantization of FIR coefficients to 16 bit. Need to generate the reference with decoded coefficients from the blob. I'm working on it now.
09a7f76 to
7681a4b
Compare
|
Thanks Seppo for helping to make it perfect |
|
@RanderWang pls check CI |
|
Even more important: check why a cmocka PR fails to compile cmocka in CI but not for you. |
|
@lgirdwood it depends on #6194 and 6194 should be merged first |
Now merged. |
|
SOFCI TEST |
7681a4b to
dee5be0
Compare
|
@softwarecki Unit test is failed for this PR but it passed with ./script/run-cmock.sh locally. I am sure there is no missed change in local environment. Do you know how does CI test unit test ? thanks! |
|
@RanderWang see the logs (missing Kconfig option or file in mock build ?) https://sof-ci.01.org/sof-pr-viewer/#/build/PR6308/build10653042 |
@lgirdwood I can't reproduce it with my environment. All these symbols are included and be built. So I want to know how does CI build it |
|
A while back I discovered QB uses a "static" version of CMocka: Github Actions builds the same commit fine: |
Thanks, Marc, so it is caused by staic version of CMocka ? @kkarask |
|
SOFCI_TEST |
|
@lgirdwood the error in unit test is not caused by my PR. please check marc's comment. This PR works with local test. The error is caused by static version of CMocka Github Actions builds the same commit fine: |
Ok, I think we have a GH using x86 cmocka with GCC and internal CI using XCC and xtensa target. @RanderWang can you make the update so XCC cmocka builds work. |
I did not say that. I mentioned one difference I found but the root cause could be something else. |
f23d497 to
af61070
Compare
|
It looks like the tests are now compiling in https://sof-ci.01.org/sof-pr-viewer/#/build/PR6308/build10629900 but they are not all passing. |
|
@singalsu now We get a few samples error in only S24_LE test with XCC built FW, no such issue with GCC. The ref = 0x7FFFFF, but FW output = 0xFF800000. @andrula-song will check it. Thanks! |
Eq-fir is a process component and use ext_data_length with module adapter. Signed-off-by: Rander Wang <rander.wang@intel.com>
af61070 to
2b314dd
Compare
here is the fix: #6468 |
|
@RanderWang @andrula-song ping when #6468 is merged. |
|
SOFCI TEST |
|
@RanderWang still some errors in CI UT. |
|
error: The last error was : The ref = 0x7FFFFF, but FW output = 0xFF800000 before #6468 |
Looks like we are comparing a 24bit number to a 64bit number - > do we need a conversion for xt-run or host ?
|
Only sof-ci/jenkins knows these magic words and it does not run CMocka. You must force-push. Pro tip: |
This test uses module interface for eq_fir. The use of decoded 16 bit FIR blob coefficients decreases a lot the mismatch between float reference FIR vs. SOF fixed point implementation. This patch adds the script tools/tune/eq/cmocka_data_eq_fir.m that generates the files cmocka_fir_coef_2ch.h and cmocka_fir_ref.h. The script debug_files_plot.m can be used to visualize the unit test result and error for data that is generated if macro DEBUG_FILES is defined. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com> Signed-off-by: Andrula Song <xiaoyuan.song@intel.com> Signed-off-by: Rander Wang <rander.wang@intel.com>
2b314dd to
18227c6
Compare
Thanks! @lgirdwood pass eq_fir now. need to trigger cmoka test again |
This test uses module interface.
Signed-off-by: Rander Wang rander.wang@intel.com