Skip to content

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

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
Replaces intermediate variable and redundant comments with a direct
call to return multiband_drc_init_coef function.

Signed-off-by: Shriram Shastry <malladi.sastry@intel.com>
@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
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