Skip to content

module: waves: rework module to use sink/source api#10790

Open
softwarecki wants to merge 1 commit into
thesofproject:mainfrom
softwarecki:p20-waves
Open

module: waves: rework module to use sink/source api#10790
softwarecki wants to merge 1 commit into
thesofproject:mainfrom
softwarecki:p20-waves

Conversation

@softwarecki
Copy link
Copy Markdown
Collaborator

Rework the waves module to only use the sink/source api to prepare sof for the full transition to pipeline 2.0.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Reworks the WAVES module adapter processing path to use the sink/source API (ring-buffer based) instead of the legacy raw input/output buffer API, aligning the module with the pipeline 2.0 transition.

Changes:

  • Switch waves_codec_process() to the sink/source process callback signature.
  • Read input via source_get_data() (with wrap handling) into the module’s internal input buffer.
  • Write output via sink_get_buffer() + sink_commit_buffer() (with wrap handling) from the module’s internal output buffer.
Comments suppressed due to low confidence (3)

src/audio/module_adapter/module/waves/waves.c:766

  • source_get_data() expects void const **buffer_start, but src_buf_start is declared as void * and passed by address, which is an incompatible pointer type and can trigger build warnings/errors. Declare src_buf_start as const void * (or uint8_t const *) and keep the pointer types consistent with source_api.h.
	const void *src_ptr;
	void *src_buf_start;
	size_t src_buf_size;

src/audio/module_adapter/module/waves/waves.c:796

  • Input is released from the source (source_release_data(..., n_bytes)) immediately after copying, before MaxxEffect_Process() and before ensuring the output can be written. If processing fails or sink_get_buffer() returns an error, the input has already been consumed and will be lost (previous legacy path kept consumed=0 on errors). Delay releasing the source until after successful processing/output commit, and on failure paths release 0 bytes to drop the reservation without consuming.
	ret = source_get_data(sources[0], n_bytes, &src_ptr, &src_buf_start, &src_buf_size);
	if (ret)
		return ret;

	/* src_buf_size is the total ring buffer size; handle wrap when copying to in_buff */
	size_to_wrap = (const uint8_t *)src_buf_start + src_buf_size - (const uint8_t *)src_ptr;
	if (n_bytes <= size_to_wrap) {
		memcpy_s(codec->mpd.in_buff, n_bytes, src_ptr, n_bytes);
	} else {
		memcpy_s(codec->mpd.in_buff, n_bytes, src_ptr, size_to_wrap);
		memcpy_s((uint8_t *)codec->mpd.in_buff + size_to_wrap, n_bytes - size_to_wrap,
			 src_buf_start, n_bytes - size_to_wrap);
	}
	source_release_data(sources[0], n_bytes);
	codec->mpd.avail = n_bytes;

src/audio/module_adapter/module/waves/waves.c:836

  • There is no check that the sink has enough free space before running the effect, and sink_get_buffer() errors are handled after the input has already been released. If the sink is full, this can cause dropped samples or re-processing on retry (depending on how source release is fixed). Add an early sink_get_free_size() check (e.g., against codec->mpd.out_buff_size/expected produced bytes) and return -ENOSPC without consuming input when there isn’t enough space.
		codec->mpd.produced = waves_codec->o_stream.numAvailableSamples *
			waves_codec->o_format.numChannels * waves_codec->sample_size_in_bytes;
		codec->mpd.consumed = codec->mpd.produced;
		ret = sink_get_buffer(sinks[0], codec->mpd.produced,
				      &snk_ptr, &snk_buf_start, &snk_buf_size);
		if (!ret) {

Comment on lines +756 to +758
static int waves_codec_process(struct processing_module *mod,
struct sof_source **sources, int num_of_sources,
struct sof_sink **sinks, int num_of_sinks)
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.

@softwarecki do we have an init check for this when buffers are setup ? Seems valid.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@lgirdwood Number of sources and sinks is checked in module_adapter_prepare(). It returns an error if any of these values is zero. So process() cannot be called without at least one source and sink.

/* compute number of input buffers */
mod->num_of_sources = 0;
list_for_item(blist, &dev->bsource_list)
mod->num_of_sources++;
/* compute number of output buffers */
mod->num_of_sinks = 0;
list_for_item(blist, &dev->bsink_list)
mod->num_of_sinks++;
if (!mod->num_of_sources && !mod->num_of_sinks) {
comp_err(dev, "no source and sink buffers connected!");
return -EINVAL;
}
if (mod->num_of_sources > CONFIG_MODULE_MAX_CONNECTIONS ||
mod->num_of_sinks > CONFIG_MODULE_MAX_CONNECTIONS) {
comp_err(dev, "too many connected sinks %d or sources %d!",
mod->num_of_sinks, mod->num_of_sources);
return -EINVAL;
}
if (mod->num_of_sources > mod->max_sources ||
mod->num_of_sinks > mod->max_sinks) {
comp_err(dev, "connected sources %d (max %d) or sinks %d (max %d) exceed module capacity",
mod->num_of_sources, mod->max_sources,
mod->num_of_sinks, mod->max_sinks);
return -EINVAL;
}

Rework the waves module to only use the sink/source api to prepare sof for
the full transition to pipeline 2.0.

Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants