Skip to content

Conversation

@lyakh
Copy link
Collaborator

@lyakh lyakh commented Jun 3, 2024

src-lite is currently broken since the SRC_LITE macro in src.c is never defined. Fix it by making src-lite mutually exclusive with other src options since that seems to be the only currently available option.

src-lite is currently broken since the SRC_LITE macro in src.c is
never defined. Fix it by making src-lite mutually exclusive with
other src options since that seems to be the only currently available
option.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
#ifdef CONFIG_COMP_SRC
#ifdef CONFIG_COMP_SRC_LITE
#include <audio/src/src_lite.toml>
#else
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this should be

#elif CONFIG_COMP_SRC

of course. Same below. Will fix if the PR's approach as such is accepted

Copy link
Member

Choose a reason for hiding this comment

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

Can we build both as separate modules with different UUIDs today ? i.e. that way topology can pick depending on memory, mcps, performance etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lgirdwood we don't have any topologies, using src-lite, otherwise someone could've noticed that it wasn't working. "Modules" (you mean Module Adapter drivers, right?) are usually built into the image, in which case you probably wouldn't be able to build src.c twice - it would have conflicting symbols. You could build them as loadable llext modules, but it would be too weird to only be able to build something as an llext and not built-in. And you wouldn't be able to load them simultaneously. In principle it should be possible to build both even without copying src.c and eliminate name conflicts, however, I'm not sure how ugly that would look in cmake. And is this really needed? Isn't "lite" just yet another src option along with "tiny," "small," etc.? And those cannot be built together. Unlike "lite" they do all share the same UUID, but semantically they seem similar.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

...a different approach would be doing it similar to volume.c and other "modules," implementing support to multiple UUIDs. For that we'd need to include both sets of coefficients and to switch between them depending on which UUID is instantiated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

replying to myself: no, it wouldn't be that easy. Those coefficient headers do indeed define the same symbols, and they aren't static, so they definitely would conflict

@lyakh lyakh mentioned this pull request Jun 7, 2024
@lyakh
Copy link
Collaborator Author

lyakh commented Jun 13, 2024

alternative #9206 got merged

@lyakh lyakh closed this Jun 13, 2024
@lyakh lyakh deleted the src-lite branch June 13, 2024 07:56
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.

2 participants