Skip to content

fix: remove reference to comp_buffer form sink/src based mod#9400

Merged
kv2019i merged 1 commit intothesofproject:mainfrom
marcinszkudlinski:igo_nr_fix
Sep 9, 2024
Merged

fix: remove reference to comp_buffer form sink/src based mod#9400
kv2019i merged 1 commit intothesofproject:mainfrom
marcinszkudlinski:igo_nr_fix

Conversation

@marcinszkudlinski
Copy link
Contributor

When using sink/src API and DP processing, there are no comp_buffer based buffers on inputs and outputs
There must not be any usage of comp_buffer in the module

this commit changes a forgotten reference to comp_buffer

When using sink/src API and DP processing, there are no
comp_buffer based buffers on inputs and outputs
There must not be any usage of comp_buffer in the module

this commit changes a forgotten reference to comp_buffer

Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com>
@marcinszkudlinski marcinszkudlinski added the bug Something isn't working as expected label Aug 23, 2024
@marcinszkudlinski marcinszkudlinski marked this pull request as ready for review August 23, 2024 13:52
@lgirdwood
Copy link
Member

@lyakh any comments ?

@lyakh
Copy link
Collaborator

lyakh commented Sep 3, 2024

@lyakh any comments ?

@lgirdwood not really. The commit message seems to suggest that it's wrong in its present form, so is it broken? But we cannot test igo-nr, can we? Maybe @andyross can take a look?

@marcinszkudlinski
Copy link
Contributor Author

@lyakh yes, it is wrong. The code tries to access to (&sourceb->stream) assuming there's struct comp_buffer there, but in sink/source API there may be no comp_buffer at all.
(to be precise: currently there're always (&sourceb->stream) and comp_buffer in use, in case of DP as a hybrid buffer, but this is only a workaround. So as for today the code works, yet I'm working hard to make direct usage of other buffer types possible and the workaround will be removed)

Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

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

Just noticed I was summoned: this is fine per my understanding too. @marcinszkudlinski is just saying that comp_buffer is no longer an API and that modules should use the arrays of sources/sinks at prepare() time to enumerate their connections.

In this particular case it was degenerate anyway: the only use of the buffer was to get the sample format, which is now an API on source/sink directly. Seems clean as I understand the design.

But also as was mentioned: it's sort of a wart that we have components in-tree that can't be unit tested. I gave some thought to a rig that would unit-test individual processing_modules in-image, though that still requires some per-type code be written to get them configured correctly.

@kv2019i kv2019i merged commit 0df649a into thesofproject:main Sep 9, 2024
@marcinszkudlinski marcinszkudlinski deleted the igo_nr_fix branch September 9, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants