Skip to content

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 <andyross@google.com>
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 <andyross@google.com>
@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