Skip to content
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.

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

Audio: Improvement of multiband_drc memory allocation error handling. #9177

Merged

Conversation

ShriramShastry
Copy link
Contributor

This check-in improves error handling in the multiband_drc component by enabling error logging and returning -ENOMEM if memory allocation fails. It replaces asserts with conditional checks to detect and log errors during memory copy operations. Finally, it streamlines the return logic in multiband_drc_setup and propagates the init_coef error code.

@ShriramShastry ShriramShastry marked this pull request as ready for review May 30, 2024 17:16
@ShriramShastry ShriramShastry requested a review from a team as a code owner May 30, 2024 17:16
@ShriramShastry ShriramShastry requested a review from lyakh May 30, 2024 17:16
Copy link
Contributor

@cujomalainey cujomalainey left a comment

Choose a reason for hiding this comment

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

I don't see much value to this change currently, will need more convincing before I will sign off

src/audio/multiband_drc/multiband_drc.c Outdated Show resolved Hide resolved
src/audio/multiband_drc/multiband_drc.c Outdated Show resolved Hide resolved
src/audio/multiband_drc/multiband_drc.c Outdated Show resolved Hide resolved
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

@ShriramShastry any update ? It looks like this PR can be simplified based on the review comments. Thanks

@ShriramShastry
Copy link
Contributor Author

@ShriramShastry any update ? It looks like this PR can be simplified based on the review comments. Thanks

yes, I'II update it shortly.

@ShriramShastry ShriramShastry force-pushed the multiband_drc_enhancements branch 2 times, most recently from 1fd88f5 to 2d3f81e Compare June 21, 2024 12:41
src/audio/multiband_drc/multiband_drc.c Outdated Show resolved Hide resolved
src/audio/multiband_drc/multiband_drc.c Outdated Show resolved Hide resolved
src/audio/multiband_drc/multiband_drc.c Show resolved Hide resolved
Replaces intermediate variable and redundant comments with a direct
call to return multiband_drc_init_coef function.

Signed-off-by: Shriram Shastry <[email protected]>
@ShriramShastry ShriramShastry force-pushed the multiband_drc_enhancements branch 2 times, most recently from 470d047 to 84ccf1b Compare June 23, 2024 17:19
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.

I don't see where this commit replaces "redundant comments"...

@lgirdwood
Copy link
Member

@wszypelt good to merge ? Looks like CI been pending a while.

@lgirdwood lgirdwood added this to the v2.11 milestone Jun 25, 2024
@cujomalainey
Copy link
Contributor

CI is stuck >5d nows and this is isolated to a module not in default topologies, assuming CI is borked and bypassing

@cujomalainey cujomalainey merged commit 8d1eea2 into thesofproject:main Jun 28, 2024
41 of 46 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.

4 participants