Skip to content

Conversation

@lyakh
Copy link
Collaborator

@lyakh lyakh commented Jun 7, 2024

This is a "more gentle" attempt to fix src-lite than #9182

lyakh added 2 commits June 7, 2024 12:47
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>
@lgirdwood
Copy link
Member

@singalsu pls review

@lgirdwood lgirdwood added this to the v2.10 milestone Jun 10, 2024

#ifdef CONFIG_CADENCE_CODEC
#include <audio/module_adapter/module/cadence.toml>
#endif
Copy link
Collaborator

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?

Copy link
Collaborator Author

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

Copy link
Collaborator

@singalsu singalsu left a 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.

Copy link
Collaborator

@singalsu singalsu left a 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
Copy link
Collaborator

@lyakh With fix to benchmark topologies #9218 the PCM now accepts non-48 kHz. The muted audio with src_lite impacts only 48000 to 48000 playback. Apparently the pass-through is not working in your version. The playback with 32000 or 44100 works normally with sound heard.

@lyakh
Copy link
Collaborator Author

lyakh commented Jun 12, 2024

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.
I propose a choice: either we remove src-lite completely because it's currently broken and unused, or we take "any" fix that makes it usable "formally" and if any audio performance isn't correct with it, it should be fixed incrementally.

Copy link
Collaborator

@singalsu singalsu left a 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.

@lgirdwood
Copy link
Member

@lyakh can you rebase/push so we can get a clean CI result with zephyr update. Thanks

@kv2019i kv2019i merged commit 7ee322f into thesofproject:main Jun 13, 2024
@lyakh lyakh deleted the src branch June 13, 2024 07:55
@lyakh lyakh mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants