-
Notifications
You must be signed in to change notification settings - Fork 349
src: fix src-lite #9206
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
src: fix src-lite #9206
Conversation
All the coefficient data and many functions in src don't need to be global, make them static. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
src_lite uses all the code as src only with a different set of coefficients. Make this obvious in Kconfig and in TOML. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
src_lite is currently broken: its definition of the SRC_LITE macro at the top of the file has no effect and its coefficients are never used. Fix it by moving coefficient evaluation into a separate function and calling it from both src.c and src_lite.c. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
|
@singalsu pls review |
|
|
||
| #ifdef CONFIG_CADENCE_CODEC | ||
| #include <audio/module_adapter/module/cadence.toml> | ||
| #endif |
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.
Why no tgl.toml and tgl-h.toml update?
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.
tgl doesn't do modular toml
singalsu
left a comment
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.
Approve, because the code looks right. The issues in running src_lite do not seem to be caused by this PR. Normal src works.
But I wasn't able to run this in (IPC3) testbench despite doing the testbench changes to load the component and created the tplg1 test pipelines for it. The component load in IPC3 didn't pass correctly the source and sink rate, so prepare() failed. IPC3 for src_lite needs more work. It likely needs another IPC exception add to module adapter to pass the larger than normal process IPC. But src_lite was never intended for IPC3 so is probably not needed.
With IPC4 and real TGL UPX device I did the the tgl.toml edit to be able to load the component with topology sof-hda-benchmark-src_lite32.tplg. However I'm not able to play anything non-48 kHz with desired rate. The topology or kernel may have an issue in formats with rates.
singalsu
left a comment
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.
Canceling approve, with topology sof-hda-benchmark-src32.tplg I can hear the playback, the src consumes 2.1 MCPS average for 48 kHz playback. The tplg or kernel issue prevents non-48 kHz to render at correct rate.
The topology sof-hda-benchmark-src_lite32.tplg runs but playback audio is muted. The playback consumes 9.9 MCPS. Something is wrong.
How did you test this PR?
@singalsu I only tested pipelines with full src (without checking the audio quality) and ran testbench. I couldn't test src-lite with real audio for 2 reasons: (1) we don't have such topologies at least not among those, used with the CI and (2) I think, that src-lite is currently broken in the sense, that even if you test such a topology, what you test won't actually be src-lite but the normal full-scale src, because src-lite coefficients are never enabled. |
singalsu
left a comment
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 just discussed with Guennadi. The src_lite is still broken with this PR. But it does not seem to be due to these changes, probably has been broken in the original code. We can approve this and file a bug and decide later what to do with this.
|
@lyakh can you rebase/push so we can get a clean CI result with zephyr update. Thanks |
This is a "more gentle" attempt to fix src-lite than #9182