Audio: MDRC: Restructure MDRC for effective memory allocation

ShriramShastry · ShriramShastry · commit f770cd2d345a · 2024-06-14T10:25:05.000+05:30
This checkin consolidates the memory allocations for the crossover,
emphasis, and deemphasis filter coefficients of the multiband DRC
(MDRC) component into a single contiguous block. This change reduces
the memory allocation overhead, improves data locality to enhance
cache efficiency, and mitigates heap fragmentation. The refactoring
of memory management adds robustness to the component by reducing
the chances of memory leaks and simplifying the initialization and
cleanup process.

By streamlining memory management, the update lowers the possibility
of memory leaks.

Signed-off-by: Shriram Shastry &lt;malladi.sastry@intel.com&gt;
diff --git a/src/audio/multiband_drc/multiband_drc.c b/src/audio/multiband_drc/multiband_drc.c
@@ -28,6 +28,7 @@
 #include <sof/ut.h>
 #include <user/eq.h>
 #include <user/trace.h>
+#include <stdbool.h>
 #include <errno.h>
 #include <stddef.h>
 #include <stdint.h>
@@ -94,17 +95,45 @@ static int multiband_drc_eq_init_coef_ch(struct sof_eq_iir_biquad *coef,
 	return 0;
 }
 
-static int multiband_drc_init_coef(struct processing_module *mod, int16_t nch, uint32_t rate)
+/**
+ * @Description Initialize coefficients for multiband DRC processing.
+ *
+ * Allocates and initializes filter coefficients for the multiband DRC module. Memory
+ * for the crossover, emphasis, and de-emphasis filter coefficients is allocated within
+ * a contiguous block if not previously done. The function checks for configuration
+ * validity and adheres to predefined channel and band count limits.
+ *
+ * The memory layout for the coefficients_block is as follows:
+ * @code
+ * +-----------------------------------+
+ * |        coefficients_block         |
+ * +-----------------------------------+
+ * |       crossover_coef              | num_bands * nch * sizeof(struct sof_eq_iir_biquad)
+ * +-----------------------------------+ <-- offset for emp_coef (crossover_coef + num_bands * nch)
+ * |       emp_coef                    | num_bands * nch * sizeof(struct sof_eq_iir_biquad)
+ * +-----------------------------------+ <-- offset for deemp_coef (emp_coef + num_bands * nch)
+ * |       deemp_coef                  | num_bands * nch * sizeof(struct sof_eq_iir_biquad)
+ * +-----------------------------------+
+ * @endcode
+ *
+ * @parameters mod Pointer to the processing module containing multiband DRC data.
+ * @parameters nch Number of channels to process, up to PLATFORM_MAX_CHANNELS.
+ * @parameters rate Sampling rate, not used in current implementation.
+ *
+ * @return 0 on success or a negative error code on failure (e.g., invalid configuration,
+ *         memory allocation failure).
+ */
+static int multiband_drc_init_coef(struct processing_module *mod,
+				   int16_t nch, uint32_t rate)
 {
 	struct comp_dev *dev = mod->dev;
 	struct multiband_drc_comp_data *cd = module_get_private_data(mod);
-	struct sof_eq_iir_biquad *crossover;
-	struct sof_eq_iir_biquad *emphasis;
-	struct sof_eq_iir_biquad *deemphasis;
 	struct sof_multiband_drc_config *config = cd->config;
 	struct multiband_drc_state *state = &cd->state;
 	uint32_t sample_bytes = get_sample_bytes(cd->source_format);
-	int i, ch, ret, num_bands;
+	struct sof_eq_iir_biquad *crossover, *emphasis, *deemphasis;
+	int i, ch, ret;
+	int num_bands = config ? config->num_bands : 0;
 
 	if (!config) {
 		comp_err(dev, "multiband_drc_init_coef(), no config is set");
@@ -113,58 +142,37 @@ static int multiband_drc_init_coef(struct processing_module *mod, int16_t nch, u
 
 	num_bands = config->num_bands;
 
-	/* Sanity checks */
-	if (nch > PLATFORM_MAX_CHANNELS) {
-		comp_err(dev,
-			 "multiband_drc_init_coef(), invalid channels count(%i)", nch);
-		return -EINVAL;
-	}
-	if (config->num_bands > SOF_MULTIBAND_DRC_MAX_BANDS) {
-		comp_err(dev, "multiband_drc_init_coef(), invalid bands count(%i)",
-			 config->num_bands);
+	if (nch > PLATFORM_MAX_CHANNELS || num_bands > SOF_MULTIBAND_DRC_MAX_BANDS) {
+		comp_err(dev, "Invalid ch count(%i) or band count(%i)", nch, num_bands);
 		return -EINVAL;
 	}
 
 	comp_info(dev, "multiband_drc_init_coef(), initializing %i-way crossover",
-		  config->num_bands);
+		  num_bands);
 
-	/* Crossover: collect the coef array and assign it to every channel */
-	crossover = config->crossover_coef;
+	struct multiband_drc_coefficients *coefficients_block = cd->coefficients_block;
+
+	/* Crossover, Emphasis, and Deemphasis EQ initialization for each channel */
 	for (ch = 0; ch < nch; ch++) {
-		ret = crossover_init_coef_ch(crossover, &state->crossover[ch],
-					     config->num_bands);
-		/* Free all previously allocated blocks in case of an error */
+		crossover = coefficients_block->crossover + ch * num_bands;
+		emphasis = coefficients_block->emphasis + ch * num_bands;
+		deemphasis = coefficients_block->deemphasis + ch * num_bands;
+
+		ret = crossover_init_coef_ch(crossover, &state->crossover[ch], num_bands);
 		if (ret < 0) {
-			comp_err(dev,
-				 "multiband_drc_init_coef(), could not assign coeffs to ch %d", ch);
+			comp_err(dev, "Can't assign xover coeffs to ch %d", ch);
 			goto err;
 		}
-	}
 
-	comp_info(dev, "multiband_drc_init_coef(), initializing emphasis_eq");
-
-	/* Emphasis: collect the coef array and assign it to every channel */
-	emphasis = config->emp_coef;
-	for (ch = 0; ch < nch; ch++) {
 		ret = multiband_drc_eq_init_coef_ch(emphasis, &state->emphasis[ch]);
-		/* Free all previously allocated blocks in case of an error */
 		if (ret < 0) {
-			comp_err(dev, "multiband_drc_init_coef(), could not assign coeffs to ch %d",
-				 ch);
+			comp_err(dev, "Can't assign emp coeffs to ch %d", ch);
 			goto err;
 		}
-	}
 
-	comp_info(dev, "multiband_drc_init_coef(), initializing deemphasis_eq");
-
-	/* Deemphasis: collect the coef array and assign it to every channel */
-	deemphasis = config->deemp_coef;
-	for (ch = 0; ch < nch; ch++) {
 		ret = multiband_drc_eq_init_coef_ch(deemphasis, &state->deemphasis[ch]);
-		/* Free all previously allocated blocks in case of an error */
 		if (ret < 0) {
-			comp_err(dev, "multiband_drc_init_coef(), could not assign coeffs to ch %d",
-				 ch);
+			comp_err(dev, "Can't assign deemp coeffs to ch %d", ch);
 			goto err;
 		}
 	}
@@ -173,15 +181,14 @@ static int multiband_drc_init_coef(struct processing_module *mod, int16_t nch, u
 	for (i = 0; i < num_bands; i++) {
 		comp_info(dev, "multiband_drc_init_coef(), initializing drc band %d", i);
 
-		ret = drc_init_pre_delay_buffers(&state->drc[i], (size_t)sample_bytes, (int)nch);
+		ret = drc_init_pre_delay_buffers(&state->drc[i], sample_bytes, nch);
 		if (ret < 0) {
-			comp_err(dev,
-				 "multiband_drc_init_coef(), could not init pre delay buffers");
+			comp_err(dev, "multiband_drc_init_coef(), could not init pre delay buffers");
 			goto err;
 		}
 
 		ret = drc_set_pre_delay_time(&state->drc[i],
-					     cd->config->drc_coef[i].pre_delay_time, rate);
+					     config->drc_coef[i].pre_delay_time, rate);
 		if (ret < 0) {
 			comp_err(dev, "multiband_drc_init_coef(), could not set pre delay time");
 			goto err;
@@ -215,6 +222,22 @@ static int multiband_drc_setup(struct processing_module *mod, int16_t channels,
  * End of Multiband DRC setup code. Next the standard component methods.
  */
 
+/**
+ * Initialize Multiband Dynamic Range Control (DRC) component.
+ *
+ * Allocates and initializes memory for the multiband DRC component data,
+ * including state structure, coefficient blocks, and module interface.
+ * The multiband DRC coefficient block is also allocated here for efficient
+ * access during processing. The function checks and ensures that the
+ * provided configuration blob size is within expected limits. If successful,
+ * the component state is reset and multiband DRC processing is enabled by
+ * default.
+ *
+ * @parameters mod Pointer to the processing module to be initialized.
+ *
+ * @return 0 on success, -EINVAL if configuration blob size is too large,
+ *         -ENOMEM if memory allocation fails for component data.
+ */
 static int multiband_drc_init(struct processing_module *mod)
 {
 	struct module_data *md = &mod->priv;
@@ -236,52 +259,95 @@ static int multiband_drc_init(struct processing_module *mod)
 	}
 
 	cd = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, sizeof(*cd));
-	if (!cd)
+	if (!cd) {
+		comp_err(dev, "multiband_drc_init(), allocation for multiband_drc_comp_data failed");
 		return -ENOMEM;
+	}
 
 	md->private = cd;
 	cd->multiband_drc_func = NULL;
 	cd->crossover_split = NULL;
-	/* Initialize to enabled is a workaround for IPC4 kernel version 6.6 and
-	 * before where the processing is never enabled via switch control. New
-	 * kernel sends the IPC4 switch control and sets this to desired state
-	 * before prepare.
-	 */
-	multiband_drc_process_enable(&cd->process_enabled);
 
-	/* Handler for configuration data */
 	cd->model_handler = comp_data_blob_handler_new(dev);
 	if (!cd->model_handler) {
 		comp_err(dev, "multiband_drc_init(): comp_data_blob_handler_new() failed.");
 		ret = -ENOMEM;
 		goto cd_fail;
 	}
 
-	/* Get configuration data and reset DRC state */
 	ret = comp_init_data_blob(cd->model_handler, bs, cfg->data);
 	if (ret < 0) {
 		comp_err(dev, "multiband_drc_init(): comp_init_data_blob() failed.");
-		goto cd_fail;
+		goto cd_model_fail;
+	}
+
+	cd->coefficients_block = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM,
+					 sizeof(struct multiband_drc_coefficients));
+	if (!cd->coefficients_block) {
+		comp_err(dev, "Failed to allocate coeff block during initialization");
+		ret = -ENOMEM;
+		goto cd_init_coef_fail;
 	}
+
 	multiband_drc_reset_state(&cd->state);
+	/* Initialize to enabled is a workaround for IPC4 kernel version 6.6 and
+	 * before where the processing is never enabled via switch control. New
+	 * kernel sends the IPC4 switch control and sets this to desired state
+	 * before prepare.
+	 */
+	multiband_drc_process_enable(&cd->process_enabled);
 
 	return 0;
 
-cd_fail:
+cd_init_coef_fail:
 	comp_data_blob_handler_free(cd->model_handler);
+cd_model_fail:
 	rfree(cd);
+cd_fail:
 	return ret;
 }
 
+/**
+ * Free resources allocated by Multiband DRC processing component.
+ *
+ * This function releases all memory and resources associated with the
+ * multiband DRC component's operation. This includes dynamically allocated
+ * filter state instances and the coefficient block as well as freeing up
+ * the model handler.
+ *
+ * @parameters "mod" Pointer to the processing module to be freed.
+ *
+ * @return 0 indicating success.
+ */
 static int multiband_drc_free(struct processing_module *mod)
 {
 	struct multiband_drc_comp_data *cd = module_get_private_data(mod);
 
 	comp_info(mod->dev, "multiband_drc_free()");
 
-	comp_data_blob_handler_free(cd->model_handler);
+	if (cd) {
+		struct multiband_drc_state *state = &cd->state;
+
+		/* Free emphasis/deemphasis IIR filter states for all channels */
+		for (int i = 0; i < PLATFORM_MAX_CHANNELS; i++) {
+			multiband_drc_iir_reset_state_ch(&state->emphasis[i]);
+			multiband_drc_iir_reset_state_ch(&state->deemphasis[i]);
+		}
+
+		/* Freeing other resources as part of the component data */
+		comp_data_blob_handler_free(cd->model_handler);
+
+		if (cd->coefficients_block) {
+			rfree(cd->coefficients_block);
+			cd->coefficients_block = NULL;
+		}
+
+		/* Reset the rest of the component data */
+		multiband_drc_reset_state(state);
+		rfree(cd);
+		module_set_private_data(mod, NULL);
+	}
 
-	rfree(cd);
 	return 0;
 }
 
diff --git a/src/audio/multiband_drc/multiband_drc.h b/src/audio/multiband_drc/multiband_drc.h
@@ -21,6 +21,12 @@
 /**
  * Stores the state of the sub-components in Multiband DRC
  */
+struct multiband_drc_coefficients {
+	struct sof_eq_iir_biquad crossover[SOF_MULTIBAND_DRC_MAX_BANDS * PLATFORM_MAX_CHANNELS];
+	struct sof_eq_iir_biquad emphasis[SOF_MULTIBAND_DRC_MAX_BANDS * PLATFORM_MAX_CHANNELS];
+	struct sof_eq_iir_biquad deemphasis[SOF_MULTIBAND_DRC_MAX_BANDS * PLATFORM_MAX_CHANNELS];
+};
+
 struct multiband_drc_state {
 	struct iir_state_df2t emphasis[PLATFORM_MAX_CHANNELS];
 	struct crossover_state crossover[PLATFORM_MAX_CHANNELS];
@@ -38,6 +44,14 @@ struct multiband_drc_comp_data {
 	struct multiband_drc_state state;        /**< compressor state */
 	struct comp_data_blob_handler *model_handler;
 	struct sof_multiband_drc_config *config; /**< pointer to setup blob */
+	/**
+	 * Pointer to the coefficients of the filters used in multiband DRC processing
+	 * which includes crossover, emphasis, and deemphasis filters. These coefficients
+	 * are used by the processing functions and are allocated during the initialization
+	 * phase. This allows efficient access to filter parameters required for audio
+	 * processing across all channels and bands.
+	 */
+	struct multiband_drc_coefficients *coefficients_block; /**< Filter coefficients */
 	bool config_ready;                       /**< set when fully received */
 	enum sof_ipc_frame source_format;        /**< source frame format */
 	bool process_enabled;                    /**< true if component is enabled */
