Skip to content
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

Update waves codec integration for Zephyr production builds #9820

Merged
merged 2 commits into from
Feb 10, 2025

Conversation

andyross
Copy link
Contributor

@andyross andyross commented Feb 8, 2025

This wasn't including the needed third party library. Also rework it a little to make the cmake a little less of a mess. Not sure I actually improved things, but I tried.

Also one trivial warning fixup I stumbled on with a variant xt-clang.

The "group" variable is set but unused, causing a -Werror build
failure with Zephyr.

Signed-off-by: Andy Ross <[email protected]>
Include the proprietary static library when not building the stub, do
a little reorganization of the cmake code to reduce duplication and
clarify things a bit.  Maybe.  CMake code doesn't clean up very well.

Signed-off-by: Andy Ross <[email protected]>
@johnylin76
Copy link
Contributor

${WAVES_DIR}/maxx_stub.c)
else()
zephyr_library_import(waves_codec
${sof_top_dir}/third_party/lib/libMaxxChrome.a)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think it would also be good to enable modular build with the library?

message(FATAL_ERROR "Add library linking support in src/audio/module_adapter/module/waves/llext/CMakeLists.txt")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undeniably, I just don't know how the llext stuff works yet and was afraid to break anything. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That can be done as stage 2 - no reason to block this PR today.

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment inline, but no blockers.

else()
zephyr_library_sources(${WAVES_DIR}/waves.c)
if(CONFIG_WAVES_CODEC_STUB)
zephyr_library_sources_ifdef(CONFIG_WAVES_CODEC_STUB
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit weird with CONFIG_WAVES_CODEC_STUB tested both at LL1151 and then 1152, but should work still.

${WAVES_DIR}/maxx_stub.c)
else()
zephyr_library_import(waves_codec
${sof_top_dir}/third_party/lib/libMaxxChrome.a)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That can be done as stage 2 - no reason to block this PR today.

@lgirdwood lgirdwood merged commit 0a0cc6b into thesofproject:main Feb 10, 2025
44 of 48 checks passed
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.

5 participants