Pipeline2 0 - audio_buffer / sink / source api extensions and simplifications #9594
Conversation
marcinszkudlinski
requested review from
ranj063,
jxstelter,
lgirdwood,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
marcinszkudlinski
force-pushed
the
pipeline2_0_6
branch
from
f81a806
to
37936d9
Compare
src/audio/buffers/comp_buffer.c
Outdated
| @@ -152,6 +152,19 @@ static int comp_buffer_sink_set_alignment_constants(struct sof_sink *sink, | |||
| return 0; | |||
| } | |||
|
|
|||
| static void comp_buffer_clean(struct sof_audio_buffer *audio_buffer) | |||
There was a problem hiding this comment.
why is buffer data cleaning needed? Is this for silence generation? Not sure if this can be of importance for security / privacy?
There was a problem hiding this comment.
in fact I don't know. Privacy, maybe silence generation?
Just wanted the changes to be 100% transparent and buffer cleaning was in the code
sof_audo_buffer_from_* => sof_audio_buffer_from_* Signed-off-by: Marcin Szkudlinski <[email protected]>
clean method does clean all buffer data leaving config as is Need to be implemented as virtual method as is implemented difrently in each buffer type Signed-off-by: Marcin Szkudlinski <[email protected]>
There are 3 APIs each buffer must implement (Sink/src/audio_buffer) for data producers/consumers/maintenance each of them need to have methods doing same operations - like settings stream parameters this commit provides simplification for buffer implementation - it is enough to implement those methods for audio_buffer API, default handlers for sink/src will propagate calls accordingly Specific handlers for sink and source may still be provided i.e. in case when sink or source API is not provided by a buffer but some other data producer (i.e. DAI) Also a default implementation of .audio_set_ipc_params is provided, that probably will fit needs of all buffers' implementation, except of currently used legacy comp_buffer Signed-off-by: Marcin Szkudlinski <[email protected]>
This commit modifies both existing buffers implementations to use simplified API for sink/source/audio_buffer Signed-off-by: Marcin Szkudlinski <[email protected]>
All control structures of buffers shared between cores are now stored in a non-cached memory alias. Cache writeback is no longer needed here Signed-off-by: Marcin Szkudlinski <[email protected]>
|
@marcinszkudlinski is this version good to go or do you have a new coming coming? I see you marked some of my (minor) comments resolved but did not update the PR. As per other review, I see no showstoppers here and we could proceed and merge this. |
Just need the simplification for reset(). |
@kv2019i @marcinszkudlinski in fact I'd have one request: I understand that this is a step on the way to a new API, right? But do you have a full set ready yet - at least as a draft? FWIW I usually cannot just design a project and then work on polishing its parts before I have the whole thing together and working in some draft form. So maybe you have the whole series somewhere or maybe you could upload it for reference - as a draft PR? |
marcinszkudlinski
force-pushed
the
pipeline2_0_6
branch
from
37936d9
to
0fc1d22
Compare
|
I don't have a draft with changes. With closest short-term target: allow using ring_buffer directly, not as a hybrid with comp_buffer, which I hope will be ready in next PR. Second goal: allow to connect modules directly, without buffers in the middle (as described above, for all modules with internal buffers like DAI and couple of others). I'll put a low level design of this in the document above shortly. And - I don't really know what else need to be in audio_buffer API. Unfortunately we use C - there's no way to make effective encapsulation of internal data, enforcing of API usage (by putting structures in .c file and make non-inline API functions) is less important that optimalization etc. As a result currently a lot of operations are performed directly on data structures by everybody and everywhere, which is hard to track/find. So I'm making changes step by step, with API growing. This may require sometimes a step back, optimalizations, modifications etc. - as in this PR |
|
@kv2019i my final version |
|
@kv2019i @lgirdwood - can we proceed? |
There are 3 APIs each buffer must implement (Sink/src/audio_buffer) - for data producers/consumers/maintenance
each of them need to have methods doing same operations - like settings stream parameters
this PR adds methods to audio_buffer API allowing access to stream parameters
It also introduces a simplification: in case of sink/src API provided by buffers the following methods:
need to be implement only once (not for sink/sourcer/audio_buffer API separately). Proper implementation will be called by a default wrappers
(how do I miss C++, such things are sooooooo natural there, in C there's a need for sophisticated tricks like this PR)