Skip to content

EQ: Check in new() coefficient blob size before allocating dev and cd#470

Merged
lgirdwood merged 1 commit intothesofproject:masterfrom
singalsu:eq_new_simplify_check_first_coefficient_blob_size
Oct 10, 2018
Merged

EQ: Check in new() coefficient blob size before allocating dev and cd#470
lgirdwood merged 1 commit intothesofproject:masterfrom
singalsu:eq_new_simplify_check_first_coefficient_blob_size

Conversation

@singalsu
Copy link
Copy Markdown
Collaborator

@singalsu singalsu commented Oct 8, 2018

This patch simplifies new() function code and avoids unnecessary
allocation of component device and data resources if the the coefficient
blob is rejected due to size. The change is identical for FIR and IIR
EQs.

Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com

@singalsu singalsu requested review from lgirdwood and mmaka1 October 8, 2018 15:59
Copy link
Copy Markdown
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.

Need some extra trace, also CI failing.

Comment thread src/audio/eq_fir.c Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we probably want to trace this error to report this.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yep

This patch simplifies new() function code and avoids unnecessary
allocation of component device and data resources if the the coefficient
blob is rejected due to size. The change is identical for FIR and IIR
EQs.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@singalsu singalsu force-pushed the eq_new_simplify_check_first_coefficient_blob_size branch from b7f2df7 to 323c55a Compare October 9, 2018 08:39
@singalsu
Copy link
Copy Markdown
Collaborator Author

singalsu commented Oct 9, 2018

@lgirdwood I just pushed the updated version. I've sent email to help understand the CI error logs.

I'm testing here successfully with UP2 all my PRs. Though I need to apply the memory.h fix PR #464, otherwise things fail at boot or playback start time.

Comment thread src/audio/eq_fir.c
*/
if (bs > SOF_EQ_FIR_MAX_SIZE) {
trace_eq_error("ens");
trace_error_value(bs);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Btw, trace has been updated so you can pass > 1 value with one call. These can be done as incremental changes in EQ etc

@lgirdwood lgirdwood merged commit 3bb9c83 into thesofproject:master Oct 10, 2018
@singalsu singalsu deleted the eq_new_simplify_check_first_coefficient_blob_size branch April 29, 2019 08:46
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.

2 participants