Skip to content

[FEATURE] organize audio module source code to per-module directories #8630

Closed
Closed
[FEATURE] organize audio module source code to per-module directories#8630
Labels
enhancementNew feature or request
Milestone
v2.9

Description

@kv2019i
kv2019i
opened on Dec 14, 2023

Is your feature request related to a problem? Please describe.
Currently audio module source files are a bit spread around the codebase. Clean up the codebase to so that all key files and definitions for a module are in a single place.

Describe the solution you'd like

  1. Per module directory which contains its own headers, Kconfig and CMakefile.

  2. Any metadata should be in this folder (.e.g IPC4 toml file sections to describe the cycle budgets)

  3. A README.MD that describes module features, capabilities and performance.

cc:

Activity

  1. kv2019i
    added
    enhancementNew feature or request
    on Dec 14, 2023

  2. kv2019i commented on Dec 14, 2023

    @kv2019i
    kv2019i
    on Dec 14, 2023
    CollaboratorAuthor

    Related PRs

  3. kv2019i commented on Dec 14, 2023

    @kv2019i
    kv2019i
    on Dec 14, 2023
    CollaboratorAuthor

    One open issue is how to tackel modules that build on each others functionality directly (like crossover, drc and mutliband-drc). Reusing functionality is something we want to promite as well, so the effort to isolate modules should not make this harder.

  4. marc-hb commented on Dec 15, 2023

    @marc-hb
    marc-hb
    on Dec 15, 2023
    Collaborator

    zephyr_library() and zephyr_library_include_directories() is what Zephyr seems to use to solve the CMake part of this problem.

    west grep zephyr.*include_dir | wc -l
805

west manifest --path
zephyr/west.yml

    I couldn't find documentation about this in https://docs.zephyrproject.org/latest/ but there is a lot in the source of https://github.com/zephyrproject-rtos/zephyr/blob/main/cmake/modules/extensions.cmake

    cc: @andyross, @cujomalainey , @johnylin76 , @btian1 , @lyakh , @singalsu , @RanderWang

  5. lgirdwood
    added this to the v2.9 milestone on Dec 15, 2023

  6. btian1 commented on Dec 18, 2023

    @btian1
    btian1
    on Dec 18, 2023
    Contributor

    @marc-hb ， if we use zephyr syntax， how about non-zephyr case？ I mean CONFIG_ZEPHYR_NATIVE_DRIVERS not defined?

    There are three ways to cover this:

    1. keep current ../../
    2. Marc's proposal.
    3. move those common to a unify header and store it in math folder.

  7. marc-hb commented on Dec 18, 2023

    @marc-hb
    marc-hb
    on Dec 18, 2023
    Collaborator

    how about non-zephyr case？ I mean CONFIG_ZEPHYR_NATIVE_DRIVERS not defined?

    Then we need an indirection like the one used in

    Marc's proposal.

    EDIT: I don't have any full "proposal" at the moment. I'm just researching and brainstorming so far.

  8. lgirdwood commented on Dec 19, 2023

    @lgirdwood
    lgirdwood
    on Dec 19, 2023
    Member

    @marc-hb ， if we use zephyr syntax， how about non-zephyr case？ I mean CONFIG_ZEPHYR_NATIVE_DRIVERS not defined?

    There are three ways to cover this:

    1. keep current ../../
    2. Marc's proposal.
    3. move those common to a unify header and store it in math folder.

    Any modules that have any public module APIs can have these APIs published in the include/module directory. e.g. I think IIR and FIR will have public APIs for other modules to use/call.

  9. kv2019i commented on Dec 20, 2023

    @kv2019i
    kv2019i
    on Dec 20, 2023
    CollaboratorAuthor

    @lgirdwood wrote:

    Any modules that have any public module APIs can have these APIs published in the include/module directory. e.g. I think IIR
    and FIR will have public APIs for other modules to use/call.

    I'm not sure the crosssover-drc-multibanddrc falls into this case. This is rather a closely related family of algorithms where multiband-drc uses the e.g. crossover completely. Exposing a complete algorithm under "math" does not seem good. I still think the "../../" solution fits this case better. Or we keep the headers of "shared modules" in the include directory (include/module perhaps) -- and not apply this reorganization to these shared modules.

  10. lgirdwood commented on Dec 21, 2023

    @lgirdwood
    lgirdwood
    on Dec 21, 2023
    Member

    @lgirdwood wrote:

    Any modules that have any public module APIs can have these APIs published in the include/module directory. e.g. I think IIR
    and FIR will have public APIs for other modules to use/call.

    I'm not sure the crosssover-drc-multibanddrc falls into this case. This is rather a closely related family of algorithms where multiband-drc uses the e.g. crossover completely. Exposing a complete algorithm under "math" does not seem good. I still think the "../../" solution fits this case better. Or we keep the headers of "shared modules" in the include directory (include/module perhaps) -- and not apply this reorganization to these shared modules.

    I mean src/include/module/module/crossover directory where clients would #include <module/crossover/x.h>

  11. btian1 commented on Jan 5, 2024

    @btian1
    btian1
    on Jan 5, 2024
    Contributor

    most modules refactor are done for now, main open for this feature is: README.MD, need @singalsu input on this.

  12. lgirdwood commented on Jan 8, 2024

    @lgirdwood
    lgirdwood
    on Jan 8, 2024
    Member

    most modules refactor are done for now, main open for this feature is: README.MD, need @singalsu input on this.

    Great, whats' still remaining - I still see some active PRs for some 3P modules.

  13. btian1
    closed this as completedon Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions