Repository navigation
P20 eq iir - convert the eq_iir module to only use the sink/source api - #11234
piotrhoppeintel wants to merge 2 commits into
Conversation
Migrate EQ IIR processing from legacy stream buffers to the source/sink module API. Use circular buffer views for generic processing and IPC3 S32-to-S16/S24 conversion paths, with exact source release and sink commit handling. Signed-off-by: Piotr Hoppe <piotr.hoppe@intel.com>
Update the EQ IIR CMocka test to use audio-buffer-backed source and sink objects. Signed-off-by: Piotr Hoppe <piotr.hoppe@intel.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved review issues were identified.
Review effort: Lite
Findings: None
What changed in this PR
Reworks the EQ IIR module to use the sink/source API for Pipeline 2.0 compatibility.
Changes:
- Migrates processing and circular-buffer handling to sink/source APIs.
- Updates EQ, passthrough, and format-conversion callbacks.
- Adapts unit tests to the new processing interface.
| File | Description |
|---|---|
test/cmocka/src/audio/eq_iir/eq_iir_process.c |
Updates EQ IIR tests for sink/source processing. |
src/audio/eq_iir/eq_iir.h |
Updates processing interfaces and state. |
src/audio/eq_iir/eq_iir.c |
Integrates sink/source processing and preparation. |
src/audio/eq_iir/eq_iir_ipc3.c |
Adapts IPC3 format-conversion routines. |
src/audio/eq_iir/eq_iir_generic.c |
Migrates generic IIR and passthrough processing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| size_t frame_count; | ||
| int ret; | ||
|
|
||
| if (num_input_buffers != 1 || num_output_buffers != 1) { |
There was a problem hiding this comment.
do we commonly check this in all / most modules? This is a hot path, wouldn't it be enough to check this where buffers / connections are configured?
There was a problem hiding this comment.
ack, we could actually put this in a cold path and have a simpler helper too. This shouldn't block this, @piotrhoppeintel we should follow up in another PR as I suspect we have copy and paste for this in more than 1 place.
There was a problem hiding this comment.
We dont need to check it every process call. One check in prepare should be fine.
|
@piotrhoppeintel 1 CI open. |
softwarecki
left a comment
softwarecki
left a comment
There was a problem hiding this comment.
Please check variable types in every processing function.
|
|
||
| source = sources[0]; | ||
| sink = sinks[0]; | ||
| cd->channels = source_get_channels(source); |
There was a problem hiding this comment.
Do we need this in every process call?
| &source_buf.buf_start, &source_buf_size); | ||
| if (ret < 0) | ||
| return ret; | ||
| if (source_buf_size < source_bytes) { |
| source_release_data(source, 0); | ||
| return ret; | ||
| } | ||
| if (sink_buf_size < sink_bytes) { |
| if (!sourceb || !sinkb) { | ||
| /* EQ component will only ever have 1 source and 1 sink buffer. */ | ||
| if (num_of_sources != 1 || num_of_sinks != 1) { | ||
| comp_err(dev, "no source or sink buffer"); |
| comp_err(dev, "source and sink channel counts do not match"); | ||
| return -EINVAL; | ||
| } | ||
| cd->channels = channels; |
There was a problem hiding this comment.
cd is filled here, so doing it in process is redundant.
| struct output_stream_buffer *bsink, uint32_t frames); | ||
| typedef void (*eq_iir_func)(struct processing_module *mod, | ||
| struct cir_buf_source *source, | ||
| struct cir_buf_sink *sink, uint32_t frames); |
| int32_t *iir_delay; /**< pointer to allocated RAM */ | ||
| size_t config_size; /**< configuration size */ | ||
| size_t iir_delay_size; /**< allocated size */ | ||
| int channels; /**< number of channels */ |
| int j; | ||
| int n; | ||
| const int nch = audio_stream_get_channels(source); | ||
| const int nch = cd->channels; |
| int n; | ||
| const int nch = audio_stream_get_channels(source); | ||
| const int nch = cd->channels; | ||
| const int samples = frames * nch; |
Rework the eq_iir module to only use the sink/source api to
prepare the SOF for the full transition to pipeline 2.0.