Skip to content

Conversation

@jsarha
Copy link
Contributor

@jsarha jsarha commented Oct 9, 2025

No description provided.

Copilot AI review requested due to automatic review settings October 9, 2025 15:09
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 three audio processing modules (Google CTC, Google RTC, and IGO NR) from component-based API to module-based API. The conversion involves updating memory allocation functions and error handling patterns to use module-specific APIs.

  • Updated memory allocation from rzalloc/rballoc to mod_zalloc/mod_balloc
  • Replaced comp_data_blob_handler_new with mod_data_blob_handler_new
  • Simplified error handling by removing manual cleanup and early returns

Reviewed Changes

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

File Description
src/audio/igo_nr/igo_nr.c Converted IGO NR module to use module API with simplified error handling
src/audio/google/google_rtc_audio_processing.c Updated Google RTC module to use module allocation functions
src/audio/google/google_ctc_audio_processing.c Converted Google CTC module with updated allocation and simplified cleanup
Comments suppressed due to low confidence (2)

src/audio/google/google_rtc_audio_processing.c:1

  • The variable 'cd' is used without being declared or retrieved from the module. Missing declaration: struct comp_data *cd = module_get_private_data(mod);
// SPDX-License-Identifier: BSD-3-Clause

src/audio/google/google_ctc_audio_processing.c:1

  • The variable 'cd' is used without being declared or retrieved from the module. Missing declaration: struct comp_data *cd = module_get_private_data(mod);
// SPDX-License-Identifier: BSD-3-Clause

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

@jsarha
Copy link
Contributor Author

jsarha commented Oct 10, 2025

SOFCI TEST

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.

Looks good. A mention that mod_free_all() call was recently added in commit 86a5460 (i.e. why the free calls are now redundant) would help reviewers understand the background, but I guess most people looking at this PR will know about it anyways.

@jsarha
Copy link
Contributor Author

jsarha commented Oct 17, 2025

Looks good. A mention that mod_free_all() call was recently added in commit 86a5460 (i.e. why the free calls are now redundant) would help reviewers understand the background, but I guess most people looking at this PR will know about it anyways.

There is now quite a few commit messages in multiple PRs needing this update. I wonder if Copilot would be able to add it, not to screw-up anything while at it.

@jsarha jsarha force-pushed the module_api_google_ctc_google_rtc_igo_nr branch from 58d0c51 to bbea445 Compare October 21, 2025 21:33
@lgirdwood
Copy link
Member

no data in some CI so rerun.

@lgirdwood
Copy link
Member

SOFCI TEST

if (!cd->input) {
comp_err(dev, "Failed to allocate input buffer");
ctc_free(mod);
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.

same problem with initialisation as in #10295

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

Jyri Sarha added 4 commits October 31, 2025 10:45
Allocate all memory, blob handlers, and fast_get() buffers through
module API mod_alloc() and friends.

The change does not touch the google_ctc_audio_processing.h API or its
mock implementation, that still uses rballoc() and rfree().

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Allocated data blob handler should be freed too.

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.

The change does not touch the google_rtc_audio_processing.h API or its
mock implementation, that still uses rballoc() and rfree().

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_google_ctc_google_rtc_igo_nr branch from bbea445 to eb182ac Compare October 31, 2025 09:15
@jsarha
Copy link
Contributor Author

jsarha commented Oct 31, 2025

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.

@kv2019i kv2019i merged commit 289e609 into thesofproject:main Oct 31, 2025
38 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