Skip to content

Conversation

@jsarha
Copy link
Contributor

@jsarha jsarha commented Oct 10, 2025

No description provided.

Copilot AI review requested due to automatic review settings October 10, 2025 11:58
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR converts several audio modules (TDFB, template, tensorflow, up down mixer, and volume) to use a new module-based memory allocation API. The changes replace direct memory allocation calls with module-aware allocation functions and remove manual memory cleanup since the module framework now handles this automatically.

Key changes:

  • Replace rzalloc(), rmalloc(), and rballoc() with mod_zalloc(), mod_alloc(), and mod_balloc()
  • Remove manual rfree() calls as memory is now managed by the module framework
  • Update function signatures to pass module context where needed for allocation

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/audio/volume/volume_ipc4.c Convert volume IPC4 module to use module allocators and remove manual memory management
src/audio/volume/volume_ipc3.c Convert volume IPC3 module to use module allocators and remove manual memory management
src/audio/volume/volume.c Remove manual memory cleanup from volume module free function
src/audio/up_down_mixer/up_down_mixer.c Convert up/down mixer to use module allocators and simplify error handling
src/audio/tensorflow/tflm-classify.c Convert TensorFlow module to use module allocators and remove manual cleanup
src/audio/template/template.c Convert template module to use module allocators and remove manual cleanup
src/audio/tdfb/tdfb_ipc3.c Convert TDFB IPC3 control data allocation to use module allocator
src/audio/tdfb/tdfb_direction.c Convert TDFB direction estimation to use module allocators and update function signature
src/audio/tdfb/tdfb_comp.h Update TDFB direction init function signature and remove free function declaration
src/audio/tdfb/tdfb.c Convert main TDFB module to use module allocators and remove manual cleanup

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@jsarha jsarha force-pushed the module_api_tdfb_template_tensorflow_updownmixer_volume branch from 4715d52 to 7bfc9e6 Compare October 21, 2025 21:45
Copy link
Collaborator

@singalsu singalsu left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@lgirdwood
Copy link
Member

@jsarha can you check CI on LNL. Lots of red.

@jsarha
Copy link
Contributor Author

jsarha commented Oct 22, 2025

SOFCI TEST

@jsarha jsarha force-pushed the module_api_tdfb_template_tensorflow_updownmixer_volume branch 2 times, most recently from 023efff to 6691f9d Compare October 23, 2025 10:07
@jsarha jsarha changed the title Module api conversion of TDFB, template, tensorflow, up down mixer, and volume modules. Module api conversion of TDFB, template, tensorflow, and up down mixer modules. Oct 23, 2025
@jsarha
Copy link
Contributor Author

jsarha commented Oct 23, 2025

Dropped volume module to a separate PR to understand the CI failures better: #10323


/* Allocate all FIR channels data in a big chunk and clear it */
cd->fir_delay = rballoc(SOF_MEM_FLAG_USER, delay_size);
cd->fir_delay = mod_balloc(mod, delay_size);
Copy link
Contributor Author

@jsarha jsarha Oct 23, 2025

Choose a reason for hiding this comment

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

This line will crash without this fix: #10324

ret = tdfb_ipc_notification_init(mod);
if (ret)
goto err_free_cd;
return ret;
Copy link
Collaborator

Choose a reason for hiding this comment

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

same issue as in multiple other PRs

ret = -ENOMEM;
goto cd_fail;
comp_err(dev, "mod_data_blob_handler_new() failed.");
return -ENOMEM;
Copy link
Collaborator

Choose a reason for hiding this comment

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

potentially problematic too?

cd->buf_in = mod_balloc(mod, mod->priv.cfg.base_cfg.ibs);
cd->buf_out = mod_balloc(mod, mod->priv.cfg.base_cfg.obs);
if (!cd->buf_in || !cd->buf_out)
return -ENOMEM;
Copy link
Collaborator

Choose a reason for hiding this comment

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

here too

Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

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

fixed by #10320

Copy link
Collaborator

@softwarecki softwarecki left a comment

Choose a reason for hiding this comment

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

Requires adjustment based on changes made in #10323.

@jsarha jsarha force-pushed the module_api_tdfb_template_tensorflow_updownmixer_volume branch from 6691f9d to 2e620f4 Compare October 31, 2025 23:37
@jsarha
Copy link
Contributor Author

jsarha commented Oct 31, 2025

@softwarecki

Requires adjustment based on changes made in #10323.

These new versions of module conversions use module API to allocate the resources, but the code still has all the resource freeing code in place.

  • couple of memory leaks due to incomplete clean up code fixed

Jyri Sarha added 2 commits November 3, 2025 16:38
Allocate all memory, blob handlers, and fast_get() buffers through
module API mod_alloc() and friends.

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Fix data blob handler leak in init failure.

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Jyri Sarha added 4 commits November 3, 2025 16:38
Allocate all memory, blob handlers, and fast_get() buffers through
module API mod_alloc() and friends.

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Allocate all memory, blob handlers, and fast_get() buffers through
module API mod_alloc() and friends.

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Fix memory leaks from failed initialization.

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Allocate all memory, blob handlers, and fast_get() buffers through
module API mod_alloc() and friends.

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
@jsarha jsarha force-pushed the module_api_tdfb_template_tensorflow_updownmixer_volume branch from 2e620f4 to b8f0958 Compare November 3, 2025 14:38
@jsarha
Copy link
Contributor Author

jsarha commented Nov 3, 2025

Rebased on top of latest main branch to get #10347 fix in.

@lgirdwood
Copy link
Member

Requires adjustment based on changes made in #10323.

@softwarecki now done.

@lgirdwood lgirdwood dismissed softwarecki’s stale review November 4, 2025 13:38

Changes requested by @softwarecki have now all been made and integrated so -1 is no longer needed.

@lgirdwood lgirdwood merged commit 1f83b15 into thesofproject:main Nov 4, 2025
39 of 42 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.

6 participants