Skip to content

[RFC] src: fix src-lite #9182

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.

Sign up for GitHub

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

Closed
lyakh wants to merge 1 commit into from
Closed

[RFC] src: fix src-lite #9182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/boards/intel_adsp_ace15_mtpm.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ CONFIG_IPC4_BASE_FW_INTEL=y

CONFIG_COMP_SRC=y
CONFIG_COMP_SRC_IPC4_FULL_MATRIX=y
CONFIG_COMP_SRC_LITE=y
CONFIG_COMP_DRC=y
CONFIG_COMP_CROSSOVER=y
CONFIG_COMP_MULTIBAND_DRC=y
Expand Down
22 changes: 11 additions & 11 deletions src/audio/src/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,6 @@ config COMP_SRC
help
Select for SRC component

config COMP_SRC_LITE
bool "SRC_LITE component"
default y
help
Select for SRC_LITE component
which only supports a subset of conversions
supported by the SRC module:
48 -> 16kHz
44.1 -> 16 kHz
32 -> 16 kHz
44.1 -> 48
if COMP_SRC

choice
Expand Down Expand Up @@ -63,6 +52,17 @@ config COMP_SRC_IPC4_FULL_MATRIX
storate consumes 241 kB. The runtime needs 9 kB. Use this to
make the full conversions set available for IPC4 build.

config COMP_SRC_LITE
bool "SRC_LITE component"
help
Select for SRC_LITE component
which only supports a subset of conversions
supported by the SRC module:
48 -> 16kHz
44.1 -> 16 kHz
32 -> 16 kHz
44.1 -> 48

endchoice

endif # SRC
7 changes: 6 additions & 1 deletion src/audio/src/src.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
#include "src.h"
#include "src_config.h"

#ifdef SRC_LITE
#if CONFIG_COMP_SRC_LITE
#include "coef/src_lite_ipc4_int32_define.h"
#include "coef/src_lite_ipc4_int32_table.h"
#elif SRC_SHORT || CONFIG_COMP_SRC_TINY
Expand Down Expand Up @@ -703,5 +703,10 @@ static const struct module_interface src_interface = {
.free = src_free,
};

#if CONFIG_COMP_SRC_LITE
DECLARE_SOF_RT_UUID("src_lite", src_uuid, 0x33441051, 0x44CD, 0x466A,
0x83, 0xA3, 0x17, 0x84, 0x78, 0x70, 0x8A, 0xEA);
#endif

DECLARE_MODULE_ADAPTER(src_interface, src_uuid, src_tr);
SOF_MODULE_INIT(src, sys_comp_module_src_interface_init);
2 changes: 2 additions & 0 deletions src/audio/src/src_ipc3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,11 @@
#include "src.h"
#include "src_config.h"

#if !CONFIG_COMP_SRC_LITE
/* c1c5326d-8390-46b4-aa47-95c3beca6550 */
DECLARE_SOF_RT_UUID("src", src_uuid, 0xc1c5326d, 0x8390, 0x46b4,
0xaa, 0x47, 0x95, 0xc3, 0xbe, 0xca, 0x65, 0x50);
#endif

DECLARE_TR_CTX(src_tr, SOF_UUID(src_uuid), LOG_LEVEL_INFO);

Expand Down
2 changes: 2 additions & 0 deletions src/audio/src/src_ipc4.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,11 @@
#include "src.h"
#include "src_config.h"

#if !CONFIG_COMP_SRC_LITE
/* e61bb28d-149a-4c1f-b709-46823ef5f5a3 */
DECLARE_SOF_RT_UUID("src", src_uuid, 0xe61bb28d, 0x149a, 0x4c1f,
0xb7, 0x09, 0x46, 0x82, 0x3e, 0xf5, 0xf5, 0xae);
#endif

DECLARE_TR_CTX(src_tr, SOF_UUID(src_uuid), LOG_LEVEL_INFO);

Expand Down
32 changes: 0 additions & 32 deletions src/audio/src/src_lite.c
View file
Open in desktop

This file was deleted.

8 changes: 3 additions & 5 deletions tools/rimage/config/lnl.toml.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@
#include <audio/asrc/asrc.toml>
#endif

#ifdef CONFIG_COMP_SRC
#ifdef CONFIG_COMP_SRC_LITE
#include <audio/src/src_lite.toml>
#else
Copy link
Collaborator Author

@lyakh lyakh Jun 3, 2024

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

@lgirdwood lgirdwood Jun 3, 2024

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

@lyakh lyakh Jun 3, 2024

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

@lyakh lyakh Jun 3, 2024

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

@lyakh lyakh Jun 4, 2024

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

#include <audio/src/src.toml>
#endif

Expand Down Expand Up @@ -120,9 +122,5 @@
#include <audio/igo_nr/igo_nr.toml>
#endif

#ifdef CONFIG_COMP_SRC_LITE
#include <audio/src/src_lite.toml>
#endif

[module]
count = __COUNTER__
8 changes: 3 additions & 5 deletions tools/rimage/config/mtl.toml.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@
#include <audio/asrc/asrc.toml>
#endif

#ifdef CONFIG_COMP_SRC
#ifdef CONFIG_COMP_SRC_LITE
#include <audio/src/src_lite.toml>
#else
#include <audio/src/src.toml>
#endif

Expand Down Expand Up @@ -116,10 +118,6 @@
#include <audio/codec/dts/dts.toml>
#endif

#ifdef CONFIG_COMP_SRC_LITE
#include <audio/src/src_lite.toml>
#endif

#ifdef CONFIG_CADENCE_CODEC
#include <audio/module_adapter/module/cadence.toml>
#endif
Expand Down
3 changes: 0 additions & 3 deletions zephyr/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,9 +754,6 @@ elseif(CONFIG_IPC_MAJOR_4)
)
endif()

zephyr_library_sources_ifdef(CONFIG_COMP_SRC_LITE
${SOF_AUDIO_PATH}/src/src_lite.c
)
zephyr_library_sources_ifdef(CONFIG_COMP_BASEFW_IPC4
${SOF_AUDIO_PATH}/base_fw.c
)
Expand Down