P20 mux - Rework the mux module to only use the sink/source api - #10998
P20 mux - Rework the mux module to only use the sink/source api#10998piotrhoppeintel wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the mux/demux module to use the SOF sink/source APIs (instead of directly manipulating audio_stream / comp_buffer), as a step toward full pipeline 2.0 adoption.
Changes:
- Extend sink/source APIs with optional
get_state()ops and helper accessors to query connected component state. - Rework mux/demux processing to acquire/commit data via
source_get_data()/sink_get_buffer()and operate on explicit circular-buffer views. - Add a generic helper for computing frames-until-wrap for circular buffers.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/include/module/audio/source_api.h | Adds optional get_state op and source_get_comp_state() helper. |
| src/include/module/audio/sink_api.h | Adds optional get_state op and sink_get_state() helper. |
| src/include/module/audio/audio_stream.h | Adds circ_buf_frames_without_wrap() helper and circular-buffer view structs. |
| src/audio/mux/mux.h | Updates mux/demux processing function signatures for sink/source API + circular buffer views. |
| src/audio/mux/mux.c | Refactors mux/demux process functions to use sink/source API and new state helpers. |
| src/audio/mux/mux_generic.c | Ports generic mux/demux implementations to operate on sink/source API buffers and circular-buffer wrap logic. |
| src/audio/buffers/comp_buffer.c | Implements sink/source get_state() for comp_buffer-backed endpoints. |
| frames = source_get_data_frames_available(source); | ||
| for (i = 0; i < num_of_sinks; i++) { | ||
| if (sink_get_state(sinks[i]) != dev->state) | ||
| continue; | ||
|
|
| static inline int circ_buf_frames_without_wrap(const void *began, const void *end, | ||
| int sample_bytes, int channels) | ||
| { | ||
| return ((const char *)end - (const char *)began) / sample_bytes / channels; | ||
| } |
|
@piotrhoppeintel I see this still has fixup commits in the series... Is this ready for review (if not, you can submit first as draft and then convert ready-for-review when it's ready for others)? |
|
@kv2019i – It's ready now. Please review. |
abonislawski
left a comment
There was a problem hiding this comment.
Please update the PR title to match the community standards.
| * | ||
| * see comment of sink_get_state() | ||
| */ | ||
| int (*get_state)(struct sof_sink *sink); |
There was a problem hiding this comment.
Separate commit for api changes
There was a problem hiding this comment.
You need to change the order in order not to break bisect. E.g. this API change commit needs to come first, and the implementaion second.
| dst -= y_size; | ||
| lookup->copy_elem[elem].src = src; | ||
| lookup->copy_elem[elem].dest = dst; | ||
| } |
There was a problem hiding this comment.
keeping these fragments in functions as before (e.g. demux_check_for_wrap()) could reduce the diff a bit
kv2019i
left a comment
There was a problem hiding this comment.
The diff is quite large for a single commit, but code looks. Idea to split out the API change is good and will help to make the mux change diff a bit smaller for reviewing. Please check the copilot comments .. most seem harmless, but worth checking.
|
@piotrhoppeintel pls do respond/resolve copilot comments on all P20 PRs as we need these closed before merge. Thanks ! |
Rework the mux module to only use the sink/source api to prepare the SOF for the full transition to pipeline 2.0. Signed-off-by: Piotr Hoppe <piotr.hoppe@intel.com>
Add optional get_state callbacks and accessors for querying upstream and downstream component states. Return COMP_STATE_NOT_EXIST when unavailable. Signed-off-by: Piotr Hoppe <piotr.hoppe@intel.com>
364e497 to
b5db075
Compare
|
I've resolved all the comments. @lgirdwood please review. |
kv2019i
left a comment
There was a problem hiding this comment.
The commit order needs to be fixed. Other comments nice-to-have.
| dst -= y_size; | ||
| lookup->copy_elem[elem].src = src; | ||
| lookup->copy_elem[elem].dest = dst; | ||
| } |
There was a problem hiding this comment.
This could indeed be a common function .. seems only thing that changes is the "source_size" shift.
| * | ||
| * see comment of sink_get_state() | ||
| */ | ||
| int (*get_state)(struct sof_sink *sink); |
There was a problem hiding this comment.
You need to change the order in order not to break bisect. E.g. this API change commit needs to come first, and the implementaion second.
Rework the mux module to only use the sink/source api to
prepare the SOF for the full transition to pipeline 2.0.