Skip to content

P20 eq iir - convert the eq_iir module to only use the sink/source api #11234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
piotrhoppeintel wants to merge 2 commits into
base: main
Choose a base branch
from
Open

P20 eq iir - convert the eq_iir module to only use the sink/source api #11234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c7b7ad1
audio: eq_iir: switch to source/sink processing API
piotrhoppeintel Sep 23, 2026
3796c56
test: cmocka: eq_iir: switch to source/sink processing API
piotrhoppeintel Sep 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 100 additions & 26 deletions src/audio/eq_iir/eq_iir.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <sof/audio/buffer.h>
#include <sof/audio/format.h>
#include <sof/audio/pipeline.h>
#include <sof/audio/sink_source_utils.h>
#include <sof/audio/ipc-config.h>
#include <sof/common.h>
#include <rtos/panic.h>
Expand Down Expand Up @@ -113,15 +114,33 @@ static int eq_iir_get_config(struct processing_module *mod,
}

static int eq_iir_process(struct processing_module *mod,
struct input_stream_buffer *input_buffers, int num_input_buffers,
struct output_stream_buffer *output_buffers, int num_output_buffers)
struct sof_source **sources, int num_input_buffers,
struct sof_sink **sinks, int num_output_buffers)
{
struct comp_data *cd = module_get_private_data(mod);
struct audio_stream *source = input_buffers[0].data;
struct audio_stream *sink = output_buffers[0].data;
uint32_t frame_count = input_buffers[0].size;
struct sof_source *source;
struct sof_sink *sink;
struct cir_buf_source source_buf;
struct cir_buf_sink sink_buf;
size_t source_frame_bytes;
size_t sink_frame_bytes;
size_t source_bytes;
size_t sink_bytes;
size_t source_buf_size;
size_t sink_buf_size;
size_t frame_count;
int ret;

if (num_input_buffers != 1 || num_output_buffers != 1) {

@lyakh lyakh Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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?

@lgirdwood lgirdwood Sep 28, 2026 •
edited
Loading

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.

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.

@softwarecki softwarecki Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We dont need to check it every process call. One check in prepare should be fine.

comp_err(mod->dev, "EQ IIR supports one source and one sink");
return -EINVAL;
}

source = sources[0];
sink = sinks[0];
cd->channels = source_get_channels(source);

@softwarecki softwarecki Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need this in every process call?

cd->frame_bytes = source_get_frame_bytes(source);

/* Check for changed configuration. The IPC-time validator installed
* in eq_iir_init() has already structurally validated the blob, so
* only NULL needs to be guarded here.
Expand All @@ -131,67 +150,120 @@ static int eq_iir_process(struct processing_module *mod,
if (!cd->config)
return -EINVAL;

ret = eq_iir_new_blob(mod, audio_stream_get_frm_fmt(source),
audio_stream_get_frm_fmt(sink),
audio_stream_get_channels(source));
ret = eq_iir_new_blob(mod, source_get_frm_fmt(source),
sink_get_frm_fmt(sink),
source_get_channels(source));
if (ret)
return ret;
}

if (frame_count) {
cd->eq_iir_func(mod, &input_buffers[0], &output_buffers[0], frame_count);
module_update_buffer_position(&input_buffers[0], &output_buffers[0], frame_count);
if (!cd->eq_iir_func)
return -EINVAL;

frame_count = source_sink_avail_frames_aligned(source, sink);
if (!frame_count)
return 0;

source_frame_bytes = source_get_frame_bytes(source);
sink_frame_bytes = sink_get_frame_bytes(sink);
source_bytes = frame_count * source_frame_bytes;
sink_bytes = frame_count * sink_frame_bytes;

ret = source_get_data(source, source_bytes, &source_buf.ptr,
&source_buf.buf_start, &source_buf_size);
if (ret < 0)
return ret;
if (source_buf_size < source_bytes) {

@softwarecki softwarecki Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it possible?

source_release_data(source, 0);
return -ENOSPC;
}
return 0;
source_buf.buf_end = (const char *)source_buf.buf_start + source_buf_size;

ret = sink_get_buffer(sink, sink_bytes, &sink_buf.ptr,
&sink_buf.buf_start, &sink_buf_size);
if (ret < 0) {
source_release_data(source, 0);
return ret;
}
if (sink_buf_size < sink_bytes) {

@softwarecki softwarecki Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ditto

source_release_data(source, 0);
sink_commit_buffer(sink, 0);
return -ENOSPC;
}
sink_buf.buf_end = (char *)sink_buf.buf_start + sink_buf_size;

cd->eq_iir_func(mod, &source_buf, &sink_buf, frame_count);

ret = source_release_data(source, source_bytes);
if (ret < 0) {
sink_commit_buffer(sink, 0);
return ret;
}

return sink_commit_buffer(sink, sink_bytes);
}

/**
* \brief Set EQ IIR frames alignment limit.
* \param[in,out] source Structure pointer of source.
* \param[in,out] sink Structure pointer of sink.
*/
static void eq_iir_set_alignment(struct audio_stream *source,
struct audio_stream *sink)
static int eq_iir_set_alignment(struct sof_source *source,
struct sof_sink *sink)
{
const uint32_t byte_align = SOF_FRAME_BYTE_ALIGN;
const uint32_t frame_align_req = 2;
int ret;

audio_stream_set_align(byte_align, frame_align_req, source);
audio_stream_set_align(byte_align, frame_align_req, sink);
ret = source_set_alignment_constants(source, byte_align, frame_align_req);
if (ret < 0)
return ret;

return sink_set_alignment_constants(sink, byte_align, frame_align_req);
}

static int eq_iir_prepare(struct processing_module *mod,
struct sof_source **sources, int num_of_sources,
struct sof_sink **sinks, int num_of_sinks)
{
struct comp_data *cd = module_get_private_data(mod);
struct comp_buffer *sourceb, *sinkb;
struct comp_dev *dev = mod->dev;
struct sof_source *source;
struct sof_sink *sink;
enum sof_ipc_frame source_format;
enum sof_ipc_frame sink_format;
int channels;
int ret = 0;

comp_dbg(dev, "entry");

/* EQ component will only ever have 1 source and 1 sink buffer */
sourceb = comp_dev_get_first_data_producer(dev);
sinkb = comp_dev_get_first_data_consumer(dev);
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");

@softwarecki softwarecki Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fix error msg

return -ENOTCONN;
}

source = sources[0];
sink = sinks[0];

ret = eq_iir_prepare_sub(mod);
if (ret < 0)
return ret;

eq_iir_set_alignment(&sourceb->stream, &sinkb->stream);
ret = eq_iir_set_alignment(source, sink);
if (ret < 0)
return ret;

/* get source and sink data format */
channels = audio_stream_get_channels(&sinkb->stream);
source_format = audio_stream_get_frm_fmt(&sourceb->stream);
sink_format = audio_stream_get_frm_fmt(&sinkb->stream);
channels = source_get_channels(source);
source_format = source_get_frm_fmt(source);
sink_format = sink_get_frm_fmt(sink);
if (channels != sink_get_channels(sink)) {
comp_err(dev, "source and sink channel counts do not match");
return -EINVAL;
}
cd->channels = channels;

@softwarecki softwarecki Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

cd is filled here, so doing it in process is redundant.

cd->frame_bytes = source_get_frame_bytes(source);

cd->config = comp_get_data_blob(cd->model_handler, &cd->config_size, NULL);

Expand Down Expand Up @@ -224,6 +296,8 @@ static int eq_iir_reset(struct processing_module *mod)
eq_iir_free_delaylines(mod);

cd->eq_iir_func = NULL;
cd->channels = 0;
cd->frame_bytes = 0;
for (i = 0; i < PLATFORM_MAX_CHANNELS; i++)
iir_reset_df1(&cd->iir[i]);

Expand All @@ -233,7 +307,7 @@ static int eq_iir_reset(struct processing_module *mod)
static const struct module_interface eq_iir_interface = {
.init = eq_iir_init,
.prepare = eq_iir_prepare,
.process_audio_stream = eq_iir_process,
.process = eq_iir_process,
.set_configuration = eq_iir_set_config,
.get_configuration = eq_iir_get_config,
.reset = eq_iir_reset,
Expand Down
28 changes: 14 additions & 14 deletions src/audio/eq_iir/eq_iir.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,17 @@

#include <stdint.h>
#include <sof/audio/module_adapter/module/generic.h>
#include <sof/audio/audio_stream.h>
#include <sof/math/iir_df2t.h>
#include <sof/math/iir_df1.h>

/** \brief Macros to convert without division bytes count to samples count */
#define EQ_IIR_BYTES_TO_S16_SAMPLES(b) ((b) >> 1)
#define EQ_IIR_BYTES_TO_S32_SAMPLES(b) ((b) >> 2)

struct audio_stream;
struct comp_dev;

/** \brief Type definition for processing function select return value. */
typedef void (*eq_iir_func)(struct processing_module *mod, struct input_stream_buffer *bsource,
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);

@softwarecki softwarecki Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

frames as size_t


/** \brief IIR EQ processing functions map item. */
struct eq_iir_func_map {
Expand All @@ -41,21 +39,23 @@ struct comp_data {
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 */

@softwarecki softwarecki Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unsigned int

size_t frame_bytes; /**< source frame size */
eq_iir_func eq_iir_func; /**< processing function */
};

#ifdef UNIT_TEST
void sys_comp_module_eq_iir_interface_init(void);
#endif

void eq_iir_s16_default(struct processing_module *mod, struct input_stream_buffer *bsource,
struct output_stream_buffer *bsink, uint32_t frames);
void eq_iir_s16_default(struct processing_module *mod, struct cir_buf_source *source,
struct cir_buf_sink *sink, uint32_t frames);

void eq_iir_s24_default(struct processing_module *mod, struct input_stream_buffer *bsource,
struct output_stream_buffer *bsink, uint32_t frames);
void eq_iir_s24_default(struct processing_module *mod, struct cir_buf_source *source,
struct cir_buf_sink *sink, uint32_t frames);

void eq_iir_s32_default(struct processing_module *mod, struct input_stream_buffer *bsource,
struct output_stream_buffer *bsink, uint32_t frames);
void eq_iir_s32_default(struct processing_module *mod, struct cir_buf_source *source,
struct cir_buf_sink *sink, uint32_t frames);

int eq_iir_new_blob(struct processing_module *mod, enum sof_ipc_frame source_format,
enum sof_ipc_frame sink_format, int channels);
Expand All @@ -66,8 +66,8 @@ void eq_iir_set_passthrough_func(struct comp_data *cd,

int eq_iir_prepare_sub(struct processing_module *mod);

void eq_iir_pass(struct processing_module *mod, struct input_stream_buffer *bsource,
struct output_stream_buffer *bsink, uint32_t frames);
void eq_iir_pass(struct processing_module *mod, struct cir_buf_source *source,
struct cir_buf_sink *sink, uint32_t frames);

int eq_iir_setup(struct processing_module *mod, int nch);

Expand Down
Loading
Loading