Skip to content

Module api conversion of math functions and mfcc module, and cmocka fixes #10293

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

Merged
kv2019i merged 16 commits into from
Oct 31, 2025
Merged

Module api conversion of math functions and mfcc module, and cmocka fixes #10293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
3c3c03f
Math: Make mod_alloc() versions of all heap using functions
Aug 21, 2025
2aba590
Audio: mfcc: Memory, blob, and fast_get allocs to module API
Aug 21, 2025
eb7c7c6
cmocka: Add mocks for mod_balloc_align() and mod_alloc_align()
Aug 22, 2025
30ffeb8
cmocka: math: Test using module memory API math function
Aug 23, 2025
b567aa4
Math: Remove directly heap using math functions
Aug 24, 2025
eb7d6f0
Math: auditory: Add mod_psy_free_mel_filterbank()
Aug 26, 2025
b17aaab
cmocka: math: auditiory: Fix memory leaks
Aug 26, 2025
562f7e6
cmocka: math: fft: Fix memory leaks
Aug 26, 2025
ab08569
cmocka: math: matrix: Fix memory leaks
Aug 26, 2025
098a097
Math: DCT: Add mod_dct_free_16()
Aug 26, 2025
b85e939
cmocka: math: dct: Fix memory leaks
Aug 26, 2025
1a4c8fa
cmocka: module_adapter_test: Fix memory freeing error
Aug 26, 2025
2556bc8
cmocka: volume: Fix memory leaks
Aug 26, 2025
731d362
cmocka: mux_copy: Fix memory leaks
Aug 26, 2025
9090979
cmocka: demux_copy: Fix memory leaks
Aug 26, 2025
fd4f807
cmocka: pipeline: Fix memory leaks
Aug 26, 2025
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
12 changes: 6 additions & 6 deletions src/audio/mfcc/mfcc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,13 @@ static int mfcc_init(struct processing_module *mod)
return -EINVAL;
}

cd = rzalloc(SOF_MEM_FLAG_USER, sizeof(*cd));
cd = mod_zalloc(mod, sizeof(*cd));
if (!cd)
return -ENOMEM;

/* Handler for configuration data */
md->private = cd;
cd->model_handler = comp_data_blob_handler_new(dev);
cd->model_handler = mod_data_blob_handler_new(mod);
if (!cd->model_handler) {
comp_err(dev, "comp_data_blob_handler_new() failed.");
ret = -ENOMEM;
Expand All @@ -114,7 +114,7 @@ static int mfcc_init(struct processing_module *mod)
comp_data_blob_handler_free(cd->model_handler);

err:
rfree(cd);
mod_free(mod, cd);
return ret;
}

Expand All @@ -123,9 +123,9 @@ static int mfcc_free(struct processing_module *mod)
struct mfcc_comp_data *cd = module_get_private_data(mod);

comp_info(mod->dev, "mfcc_free()");
comp_data_blob_handler_free(cd->model_handler);
mfcc_free_buffers(cd);
rfree(cd);
mod_data_blob_handler_free(mod, cd->model_handler);
mod_free(mod, cd);
mfcc_free_buffers(mod);
return 0;
}

Expand Down
39 changes: 20 additions & 19 deletions src/audio/mfcc/mfcc_setup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ static int mfcc_get_window(struct mfcc_state *state, enum sof_mfcc_fft_window_ty
* coef[i] = 1.0 + 0.5 * lifter * sin(pi * i / lifter), i = 0 to num_ceps-1
*/

static int mfcc_get_cepstral_lifter(struct mfcc_cepstral_lifter *cl)
static int mfcc_get_cepstral_lifter(struct processing_module *mod, struct mfcc_cepstral_lifter *cl)
{
int32_t inv_cepstral_lifter;
int32_t val;
Expand All @@ -75,7 +75,7 @@ static int mfcc_get_cepstral_lifter(struct mfcc_cepstral_lifter *cl)
if (cl->num_ceps > DCT_MATRIX_SIZE_MAX)
return -EINVAL;

cl->matrix = mat_matrix_alloc_16b(1, cl->num_ceps, 9); /* Use Q7.9 */
cl->matrix = mod_mat_matrix_alloc_16b(mod, 1, cl->num_ceps, 9); /* Use Q7.9 */
if (!cl->matrix)
return -ENOMEM;

Expand Down Expand Up @@ -171,8 +171,7 @@ int mfcc_setup(struct processing_module *mod, int max_frames, int sample_rate, i
comp_info(dev, "mfcc_setup(), buffer_size = %d, prev_size = %d",
state->buffer_size, state->prev_data_size);

state->buffers = rzalloc(SOF_MEM_FLAG_USER,
state->sample_buffers_size);
state->buffers = mod_zalloc(mod, state->sample_buffers_size);
if (!state->buffers) {
comp_err(dev, "Failed buffer allocate");
ret = -ENOMEM;
Expand All @@ -189,14 +188,14 @@ int mfcc_setup(struct processing_module *mod, int max_frames, int sample_rate, i
#else
fft->fft_buffer_size = fft->fft_padded_size * sizeof(struct icomplex32);
#endif
fft->fft_buf = rzalloc(SOF_MEM_FLAG_USER, fft->fft_buffer_size);
fft->fft_buf = mod_zalloc(mod, fft->fft_buffer_size);
if (!fft->fft_buf) {
comp_err(dev, "Failed FFT buffer allocate");
ret = -ENOMEM;
goto free_buffers;
}

fft->fft_out = rzalloc(SOF_MEM_FLAG_USER, fft->fft_buffer_size);
fft->fft_out = mod_zalloc(mod, fft->fft_buffer_size);
if (!fft->fft_out) {
comp_err(dev, "Failed FFT output allocate");
ret = -ENOMEM;
Expand All @@ -206,8 +205,8 @@ int mfcc_setup(struct processing_module *mod, int max_frames, int sample_rate, i
fft->fft_fill_start_idx = 0; /* From config pad_type */

/* Setup FFT */
fft->fft_plan = fft_plan_new(fft->fft_buf, fft->fft_out, fft->fft_padded_size,
MFCC_FFT_BITS);
fft->fft_plan = mod_fft_plan_new(mod, fft->fft_buf, fft->fft_out, fft->fft_padded_size,
MFCC_FFT_BITS);
if (!fft->fft_plan) {
comp_err(dev, "Failed FFT init");
ret = -EINVAL;
Expand Down Expand Up @@ -242,7 +241,7 @@ int mfcc_setup(struct processing_module *mod, int max_frames, int sample_rate, i
fb->scratch_data2 = (int16_t *)fft->fft_out;
fb->scratch_length1 = fft->fft_buffer_size / sizeof(int16_t);
fb->scratch_length2 = fft->fft_buffer_size / sizeof(int16_t);
ret = psy_get_mel_filterbank(fb);
ret = mod_psy_get_mel_filterbank(mod, fb);
if (ret < 0) {
comp_err(dev, "Failed Mel filterbank");
goto free_fft_out;
Expand All @@ -253,15 +252,15 @@ int mfcc_setup(struct processing_module *mod, int max_frames, int sample_rate, i
dct->num_out = config->num_ceps;
dct->type = (enum dct_type)config->dct;
dct->ortho = true;
ret = dct_initialize_16(dct);
ret = mod_dct_initialize_16(mod, dct);
if (ret < 0) {
comp_err(dev, "Failed DCT init");
goto free_melfb_data;
}

state->lifter.num_ceps = config->num_ceps;
state->lifter.cepstral_lifter = config->cepstral_lifter; /* Q7.9 max 64.0*/
ret = mfcc_get_cepstral_lifter(&state->lifter);
ret = mfcc_get_cepstral_lifter(mod, &state->lifter);
if (ret < 0) {
comp_err(dev, "Failed cepstral lifter");
goto free_dct_matrix;
Expand Down Expand Up @@ -317,13 +316,15 @@ int mfcc_setup(struct processing_module *mod, int max_frames, int sample_rate, i
return ret;
}

void mfcc_free_buffers(struct mfcc_comp_data *cd)
void mfcc_free_buffers(struct processing_module *mod)
{
fft_plan_free(cd->state.fft.fft_plan);
rfree(cd->state.fft.fft_buf);
rfree(cd->state.fft.fft_out);
rfree(cd->state.buffers);
rfree(cd->state.melfb.data);
rfree(cd->state.dct.matrix);
rfree(cd->state.lifter.matrix);
struct mfcc_comp_data *cd = module_get_private_data(mod);

mod_fft_plan_free(mod, cd->state.fft.fft_plan);
mod_free(mod, cd->state.fft.fft_buf);
mod_free(mod, cd->state.fft.fft_out);
mod_free(mod, cd->state.buffers);
mod_free(mod, cd->state.melfb.data);
mod_free(mod, cd->state.dct.matrix);
mod_free(mod, cd->state.lifter.matrix);
Copy link
Collaborator

@lyakh lyakh Oct 27, 2025

Choose a reason for hiding this comment

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

mfcc_free_buffers() is only called from mfcc_free(). So we don't need explicit mod_free() calls? Also mod_fft_plan_free() only calls mod_free() internally

Copy link
Contributor Author

@jsarha jsarha Oct 31, 2025

Choose a reason for hiding this comment

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

It indeed appears so. But now that I am restoring all the clean up code anyway, this code is again needed.

}
2 changes: 1 addition & 1 deletion src/include/sof/audio/mfcc/mfcc_comp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ static inline int16_t *mfcc_buffer_wrap(struct mfcc_buffer *buffer, int16_t *ptr

int mfcc_setup(struct processing_module *mod, int max_frames, int rate, int channels);

void mfcc_free_buffers(struct mfcc_comp_data *cd);
void mfcc_free_buffers(struct processing_module *mod);

void mfcc_s16_default(struct processing_module *mod, struct input_stream_buffer *bsource,
struct output_stream_buffer *bsink, int frames);
Expand Down
4 changes: 3 additions & 1 deletion src/include/sof/math/auditory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ int16_t psy_mel_to_hz(int16_t mel);
* filter coefficients.
* \return Zero when success, otherwise error code.
*/
int psy_get_mel_filterbank(struct psy_mel_filterbank *mel_fb);
struct processing_module;
int mod_psy_get_mel_filterbank(struct processing_module *mod, struct psy_mel_filterbank *mel_fb);
int mod_psy_free_mel_filterbank(struct processing_module *mod, struct psy_mel_filterbank *mel_fb);

/**
* \brief Convert linear complex spectra from FFT into Mel band energies in desired
Expand Down
4 changes: 3 additions & 1 deletion src/include/sof/math/dct.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ struct dct_plan_16 {
bool ortho;
};

int dct_initialize_16(struct dct_plan_16 *dct);
int mod_dct_initialize_16(struct processing_module *mod, struct dct_plan_16 *dct);

int mod_dct_free_16(struct processing_module *mod, struct dct_plan_16 *dct);

#endif /* __SOF_MATH_DCT_H__ */
4 changes: 4 additions & 0 deletions src/include/sof/math/fft.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,12 @@ struct fft_plan {

/* interfaces of the library */
struct fft_plan *fft_plan_new(void *inb, void *outb, uint32_t size, int bits);
struct processing_module;
struct fft_plan *mod_fft_plan_new(struct processing_module *mod, void *inb,
void *outb, uint32_t size, int bits);
void fft_execute_16(struct fft_plan *plan, bool ifft);
void fft_execute_32(struct fft_plan *plan, bool ifft);
void fft_plan_free(struct fft_plan *plan16);
void mod_fft_plan_free(struct processing_module *mod, struct fft_plan *plan16);

#endif /* __SOF_FFT_H__ */
15 changes: 15 additions & 0 deletions src/include/sof/math/matrix.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#ifndef __SOF_MATH_MATRIX_H__
#define __SOF_MATH_MATRIX_H__

#include <sof/audio/module_adapter/module/generic.h>
#include <rtos/alloc.h>
#include <ipc/topology.h>
#include <stdint.h>
Expand Down Expand Up @@ -44,6 +45,20 @@ static inline struct mat_matrix_16b *mat_matrix_alloc_16b(int16_t rows, int16_t
return mat;
}

static inline struct mat_matrix_16b *mod_mat_matrix_alloc_16b(struct processing_module *mod,
int16_t rows, int16_t columns,
int16_t fractions)
{
struct mat_matrix_16b *mat;
const int mat_size = sizeof(int16_t) * rows * columns + sizeof(struct mat_matrix_16b);

mat = mod_zalloc(mod, mat_size);
if (mat)
mat_init_16b(mat, rows, columns, fractions);

return mat;
}

static inline void mat_copy_from_linear_16b(struct mat_matrix_16b *mat, const int16_t *lin_data)
{
size_t bytes = sizeof(int16_t) * mat->rows * mat->columns;
Expand Down
11 changes: 8 additions & 3 deletions src/math/auditory/auditory.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//
// Author: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

#include <sof/audio/module_adapter/module/generic.h>
#include <sof/audio/format.h>
#include <rtos/alloc.h>
#include <sof/math/auditory.h>
Expand Down Expand Up @@ -85,7 +86,7 @@ int16_t psy_mel_to_hz(int16_t mel)
return hz;
}

int psy_get_mel_filterbank(struct psy_mel_filterbank *fb)
int mod_psy_get_mel_filterbank(struct processing_module *mod, struct psy_mel_filterbank *fb)
{
int32_t up_slope;
int32_t down_slope;
Expand Down Expand Up @@ -200,8 +201,7 @@ int psy_get_mel_filterbank(struct psy_mel_filterbank *fb)
}

fb->data_length = &fb->scratch_data2[base_idx] - &fb->scratch_data2[0];
fb->data = rzalloc(SOF_MEM_FLAG_USER,
sizeof(int16_t) * fb->data_length);
fb->data = mod_zalloc(mod, sizeof(int16_t) * fb->data_length);
if (!fb->data)
return -ENOMEM;

Expand All @@ -210,3 +210,8 @@ int psy_get_mel_filterbank(struct psy_mel_filterbank *fb)
fb->scratch_data2, sizeof(int16_t) * fb->data_length);
return 0;
}

int mod_psy_free_mel_filterbank(struct processing_module *mod, struct psy_mel_filterbank *mel_fb)
Copy link
Collaborator

@lyakh lyakh Oct 27, 2025

Choose a reason for hiding this comment

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

is this ever used outside of the module destruction path?

Copy link
Contributor Author

@jsarha jsarha Oct 31, 2025

Choose a reason for hiding this comment

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

As I mentioned earlier, the cmocka tests to not implement the resource tracking and everything has to be freed manually in order to make Valgrind happy. And of course due to recent turn of events, the resource tracking has taken a step back in any case.

Copy link
Contributor Author

@jsarha jsarha Nov 1, 2025

Choose a reason for hiding this comment

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

Of course there probably is one or two places in mfcc where this function should be called, but that is another story. Once I get my per module resource tracking ready, the mfcc is one module that should take it into use among the first ones.

{
return mod_free(mod, mel_fb->data);
}
10 changes: 8 additions & 2 deletions src/math/dct.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//
// Author: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

#include <sof/audio/module_adapter/module/generic.h>
#include <sof/audio/format.h>
#include <sof/math/matrix.h>
#include <sof/math/dct.h>
Expand Down Expand Up @@ -31,7 +32,7 @@
* multiply with the returned matrix.
* \param[in,out] dct In input provide DCT type and size, in output the DCT matrix
*/
int dct_initialize_16(struct dct_plan_16 *dct)
int mod_dct_initialize_16(struct processing_module *mod, struct dct_plan_16 *dct)
{
int16_t dct_val;
int32_t arg;
Expand All @@ -51,7 +52,7 @@ int dct_initialize_16(struct dct_plan_16 *dct)
if (dct->num_in > DCT_MATRIX_SIZE_MAX || dct->num_out > DCT_MATRIX_SIZE_MAX)
return -EINVAL;

dct->matrix = mat_matrix_alloc_16b(dct->num_in, dct->num_out, 15);
dct->matrix = mod_mat_matrix_alloc_16b(mod, dct->num_in, dct->num_out, 15);
if (!dct->matrix)
return -ENOMEM;

Expand All @@ -77,3 +78,8 @@ int dct_initialize_16(struct dct_plan_16 *dct)

return 0;
}
Copy link
Collaborator

@lyakh lyakh Oct 27, 2025

Choose a reason for hiding this comment

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

would it be possible to modify these functions in place without first adding new versions and then deleting original ones? E.g. in patch 1 modify function prototypes and add wrappers:

-void x(int y)
+void x_new(struct mod *p, int y)
 {
...
-    m = rmalloc(...);
+    if (p)
+        m = mod_alloc(p, ...);
+    else
+        m = rmalloc(...);
 ...
 }
+
+void x(int y)
+{
+    return x_new(NULL, y);
+}

then move all users one by one to use x_new() and then remove the original x() and (optionally) rename x_new() back to x()?

Copy link
Contributor Author

@jsarha jsarha Oct 31, 2025

Choose a reason for hiding this comment

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

Maybe, but I am not volunteering for the task. What whoud be the point?

Copy link
Collaborator

@lyakh lyakh Oct 31, 2025

Choose a reason for hiding this comment

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

Maybe, but I am not volunteering for the task. What whoud be the point?

@jsarha the point would be not to duplicate code even intermittently, keeping all commits small and easily reviewable

Copy link
Contributor Author

@jsarha jsarha Nov 1, 2025

Choose a reason for hiding this comment

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

I grant you the point obout being easier to review and to be sure the functions are still in essence the same. But I think our git repository can easily take the duplicated code.

This was a result of my conversion method. Making mod api versions of all that I needed for mfcc and then remove the obsolte functions and see what breaks. I really hope you do not put to redo also this part of this PR series. Good thing of course is that by now I know almost by heart all the allocs and frees in all the module code.


int mod_dct_free_16(struct processing_module *mod, struct dct_plan_16 *dct)
{
return mod_free(mod, dct->matrix);
}
59 changes: 59 additions & 0 deletions src/math/fft/fft_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// Author: Amery Song <chao.song@intel.com>
// Keyon Jie <yang.jie@linux.intel.com>

#include <sof/audio/module_adapter/module/generic.h>
#include <sof/audio/buffer.h>
#include <sof/audio/format.h>
#include <sof/common.h>
Expand Down Expand Up @@ -63,6 +64,55 @@ struct fft_plan *fft_plan_new(void *inb, void *outb, uint32_t size, int bits)
return plan;
}

struct fft_plan *mod_fft_plan_new(struct processing_module *mod, void *inb,
void *outb, uint32_t size, int bits)
{
struct fft_plan *plan;
int lim = 1;
int len = 0;
int i;

if (!inb || !outb)
return NULL;

plan = mod_zalloc(mod, sizeof(struct fft_plan));
if (!plan)
return NULL;

switch (bits) {
case 16:
plan->inb16 = inb;
plan->outb16 = outb;
break;
case 32:
plan->inb32 = inb;
plan->outb32 = outb;
break;
default:
return NULL;
}

/* calculate the exponent of 2 */
while (lim < size) {
lim <<= 1;
len++;
}
Copy link
Collaborator

@lyakh lyakh Oct 31, 2025

Choose a reason for hiding this comment

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

this can be done without a loop with clz() and possibly checks for 0 and a power of 2

Copy link
Contributor Author

@jsarha jsarha Oct 31, 2025

Choose a reason for hiding this comment

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

A good proposal, but the issue has nothing to do with this PR.


plan->size = lim;
plan->len = len;

plan->bit_reverse_idx = mod_zalloc(mod, plan->size * sizeof(uint16_t));
if (!plan->bit_reverse_idx)
return NULL;

/* set up the bit reverse index */
for (i = 1; i < plan->size; ++i)
plan->bit_reverse_idx[i] = (plan->bit_reverse_idx[i >> 1] >> 1) |
((i & 1) << (len - 1));

return plan;
}

void fft_plan_free(struct fft_plan *plan)
{
if (!plan)
Expand All @@ -71,3 +121,12 @@ void fft_plan_free(struct fft_plan *plan)
rfree(plan->bit_reverse_idx);
rfree(plan);
}

void mod_fft_plan_free(struct processing_module *mod, struct fft_plan *plan)
{
if (!plan)
return;

mod_free(mod, plan->bit_reverse_idx);
mod_free(mod, plan);
}
2 changes: 1 addition & 1 deletion test/cmocka/src/audio/module_adapter_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ void module_adapter_test_free(struct processing_module_test_data *test_data)
}

for (i = 0; i < test_data->num_sources; i++) {
free_test_sink(test_data->sources[i]);
free_test_source(test_data->sources[i]);
test_free(test_data->input_buffers[i]);
}

Expand Down
7 changes: 6 additions & 1 deletion test/cmocka/src/audio/mux/demux_copy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ static int teardown_test_case(void **state)
struct test_data *td = *((struct test_data **)state);
int i;

rfree(td->mod->input_buffers);
rfree(td->mod->output_buffers);

free_test_source(td->source);

for (i = 0; i < MUX_MAX_STREAMS; ++i)
Expand Down Expand Up @@ -339,8 +342,10 @@ int main(void)
cmocka_set_message_output(CM_OUTPUT_TAP);
ret = cmocka_run_group_tests(tests, setup_group, NULL);

for (ti = 0; ti < ARRAY_SIZE(valid_formats) * ARRAY_SIZE(masks); ti++)
for (ti = 0; ti < ARRAY_SIZE(valid_formats) * ARRAY_SIZE(masks); ti++) {
free(tests[ti].initial_state);
free((void *)tests[ti].name);
}

return ret;
}
Loading
Loading